Skip to content

Fix User-Agent duplication and support case-sensitive constant classes for feature IDs #3763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions generator/.DevConfigs/6ea8698f-8d5d-4a79-8648-42451415f12e.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

{
"core": {
"changeLogMessages": [
"fix: User-Agent header is growing when reusing the request object"
],
"type": "patch",
"updateMinimum": true
}
}
49 changes: 48 additions & 1 deletion sdk/src/Core/Amazon.Runtime/ConstantClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using System.Reflection;
using System.Text;

using Amazon.Util.Internal;
Expand All @@ -35,6 +36,7 @@ namespace Amazon.Runtime
/// Base class for constant class that holds the value that will be sent to AWS for the static constants.
/// </summary>
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)]
[ConstantClassComparer(ConstantClassComparerKind.OrdinalIgnoreCase)]
public class ConstantClass
{
static readonly object staticFieldsLock = new object();
Expand Down Expand Up @@ -119,7 +121,14 @@ private static void LoadFields([DynamicallyAccessedMembers(DynamicallyAccessedMe
{
if (staticFields.ContainsKey(type)) return;

var map = new Dictionary<string, ConstantClass>(StringComparer.OrdinalIgnoreCase);
var comparer = StringComparer.OrdinalIgnoreCase;
var comparerAttr = type.GetCustomAttribute<ConstantClassComparerAttribute>();
if (comparerAttr != null)
{
comparer = GetStringComparerFromKind(comparerAttr.ComparerKind);
}

var map = new Dictionary<string, ConstantClass>(comparer);

foreach (var fieldInfo in type.GetFields())
{
Expand All @@ -139,6 +148,19 @@ private static void LoadFields([DynamicallyAccessedMembers(DynamicallyAccessedMe
}
}

private static StringComparer GetStringComparerFromKind(ConstantClassComparerKind comparerKind)
{
switch (comparerKind)
{
case ConstantClassComparerKind.Ordinal:
return StringComparer.Ordinal;
case ConstantClassComparerKind.OrdinalIgnoreCase:
return StringComparer.OrdinalIgnoreCase;
default:
return StringComparer.OrdinalIgnoreCase;
}
}

public override int GetHashCode()
{
return this.Value.GetHashCode();
Expand Down Expand Up @@ -246,4 +268,29 @@ protected virtual bool Equals(string value)
return !(a == b);
}
}

/// <summary>
/// Indicates the type of <see cref="ConstantClassComparerKind"/> to use when indexing constants
/// in a <see cref="ConstantClass"/>-derived type.
/// </summary>
[AttributeUsage(AttributeTargets.Class)]
public sealed class ConstantClassComparerAttribute : Attribute
{
public ConstantClassComparerKind ComparerKind { get; }

public ConstantClassComparerAttribute(ConstantClassComparerKind comparerKind)
{
ComparerKind = comparerKind;
}
}

/// <summary>
/// Specifies the kind of string comparison to use when indexing constant values
/// in a <see cref="ConstantClass"/>-derived type.
/// </summary>
public enum ConstantClassComparerKind
{
Ordinal,
OrdinalIgnoreCase,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ public void AddFeature(UserAgentFeatureId featureId)
_trackedFeatureIds.Add(featureId.Value);
}

/// <summary>
/// Returns the user agent components that were explicitly added via AddUserAgentComponent,
/// </summary>
public string GetCustomUserAgentComponents()
{
return _userAgentBuilder.ToString().Trim();
}

/// <summary>
/// Appends the metrics user-agent to the existing user-agent and returns the full string.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace Amazon.Runtime.Internal.UserAgent
/// <summary>
/// Represents the unique metric identifiers for SDK features tracked under User Agent 2.1 (UA2.1).
/// </summary>
[ConstantClassComparer(ConstantClassComparerKind.Ordinal)]
public class UserAgentFeatureId : ConstantClass
{
/// <summary>
Expand Down
22 changes: 20 additions & 2 deletions sdk/src/Core/Amazon.Runtime/Pipeline/Contexts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ namespace Amazon.Runtime.Internal
public class RequestContext : IRequestContext
{
private IServiceMetadata _serviceMetadata;
IDictionary<string, object> _contextAttributes;
private IDictionary<string, object> _contextAttributes;
private UserAgentDetails _userAgentDetails;

public RequestContext(bool enableMetric)
: this(enableMetric, null)
Expand Down Expand Up @@ -104,7 +105,24 @@ public RequestContext(bool enableMetrics, ISigner clientSigner)
public InvokeOptionsBase Options { get; set; }
public ISigner Signer { get; set; }
public BaseIdentity Identity { get; set; }
public UserAgentDetails UserAgentDetails { get => ((IAmazonWebServiceRequest)OriginalRequest).UserAgentDetails; }
public UserAgentDetails UserAgentDetails
{
get
{
if (_userAgentDetails != null)
return _userAgentDetails;

_userAgentDetails = new UserAgentDetails();

_userAgentDetails.AddUserAgentComponent(((IAmazonWebServiceRequest)OriginalRequest).UserAgentDetails.GetCustomUserAgentComponents());
foreach (var featureId in ((IAmazonWebServiceRequest)OriginalRequest).UserAgentDetails.TrackedFeatureIds)
{
_userAgentDetails.AddFeature(featureId);
}

return _userAgentDetails;
}
}

public System.Threading.CancellationToken CancellationToken { get; set; }

Expand Down
21 changes: 21 additions & 0 deletions sdk/test/UnitTests/Custom/Runtime/UserAgentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
using System.IO;
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;

namespace AWSSDK.UnitTests
{
Expand Down Expand Up @@ -117,6 +118,26 @@ public void TestUserAgentAdditionForPaginators()
Assert.IsTrue(metricsSection.Contains(UserAgentFeatureId.PAGINATOR.Value));
}

[TestMethod]
public void RunningMultipleRequestsUsingTheSameRequestObject_ShouldntDuplicateTheUserAgentDetails()
{
var listObjectsV2Request = new ListObjectsV2Request
{
BucketName = "test"
};

RunMockRequest(listObjectsV2Request, new AmazonS3Config(), ListObjectsV2RequestMarshaller.Instance, ListObjectsV2ResponseUnmarshaller.Instance);
RunMockRequest(listObjectsV2Request, new AmazonS3Config(), ListObjectsV2RequestMarshaller.Instance, ListObjectsV2ResponseUnmarshaller.Instance);
var request = RunMockRequest(listObjectsV2Request, new AmazonS3Config(), ListObjectsV2RequestMarshaller.Instance, ListObjectsV2ResponseUnmarshaller.Instance);

request.Headers.TryGetValue(HeaderKeys.UserAgentHeader, out string userAgentHeader);
Assert.IsNotNull(userAgentHeader);

Assert.AreEqual(Regex.Matches(userAgentHeader, "md/ClientSync").Count, 1);
Assert.AreEqual(Regex.Matches(userAgentHeader, "cfg/init-coll#").Count, 1);
Assert.AreEqual(Regex.Matches(userAgentHeader, "aws-sdk-dotnet-framework/").Count, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This would fail if the test project didn't run on Windows or targeted .NET 8 for example. I know that's not the case today, but still good not to write Windows specific asserts (for new tests at least) if we can.

}

[DataTestMethod]
[DataRow(RequestRetryMode.Standard, "E")]
[DataRow(RequestRetryMode.Adaptive, "F")]
Expand Down