From a4c5ebc1d0c9f53f7dd2786e3c22b99944abefd0 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Thu, 17 Dec 2020 13:56:38 -0800 Subject: [PATCH 01/12] Add AzureSasCredential --- sdk/core/Azure.Core/src/Azure.Core.csproj | 1 + sdk/core/Azure.Core/src/AzureSasCredential.cs | 60 ++++++++++++++ .../src/Shared/AzureSasCredentialPolicy.cs | 40 +++++++++ .../tests/AzureSasCredentialPolicyTests.cs | 83 +++++++++++++++++++ sdk/core/Azure.Core/tests/PolicyTestBase.cs | 3 +- 5 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 sdk/core/Azure.Core/src/AzureSasCredential.cs create mode 100644 sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs create mode 100644 sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs diff --git a/sdk/core/Azure.Core/src/Azure.Core.csproj b/sdk/core/Azure.Core/src/Azure.Core.csproj index 144984d7964c..05f1a1756d3c 100644 --- a/sdk/core/Azure.Core/src/Azure.Core.csproj +++ b/sdk/core/Azure.Core/src/Azure.Core.csproj @@ -26,6 +26,7 @@ + diff --git a/sdk/core/Azure.Core/src/AzureSasCredential.cs b/sdk/core/Azure.Core/src/AzureSasCredential.cs new file mode 100644 index 000000000000..bfdc985cca07 --- /dev/null +++ b/sdk/core/Azure.Core/src/AzureSasCredential.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.ComponentModel; +using System.Threading; +using Azure.Core; + +namespace Azure +{ + /// + /// Shared access signature credential used to authenticate to an Azure Service. + /// It provides the ability to update the signature without creating a new client. + /// + public class AzureSasCredential + { + private string _signature; + + /// + /// Signature used to authenticate to an Azure service. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public string Signature + { + get => Volatile.Read(ref _signature); + private set => Volatile.Write(ref _signature, value); + } + + /// + /// Initializes a new instance of the class. + /// + /// Signature to use to authenticate with the Azure service. + /// + /// Thrown when the is null. + /// + /// + /// Thrown when the is empty. + /// +#pragma warning disable CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. + public AzureSasCredential(string signature) => Update(signature); +#pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. + + /// + /// Updates the signature. + /// This is intended to be used when you've regenerated your signature + /// and want to update long lived clients. + /// + /// Signature to authenticate the service against. + /// + /// Thrown when the is null. + /// + /// + /// Thrown when the is empty. + /// + public void Update(string signature) + { + Argument.AssertNotNullOrEmpty(signature, nameof(signature)); + Signature = signature; + } + } +} diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs new file mode 100644 index 000000000000..566f7ba8cbf8 --- /dev/null +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using Azure.Core.Pipeline; + +namespace Azure.Core +{ + internal class AzureSasCredentialPolicy : HttpPipelineSynchronousPolicy + { + private readonly AzureSasCredential _credential; + + /// + /// Initializes a new instance of the class. + /// + /// The used to authenticate requests. + public AzureSasCredentialPolicy(AzureSasCredential credential) + { + Argument.AssertNotNull(credential, nameof(credential)); + _credential = credential; + } + + /// + public override void OnSendingRequest(HttpMessage message) + { + base.OnSendingRequest(message); + string query = message.Request.Uri.Query; + if (!query.Contains(_credential.Signature)) + { + string signature = _credential.Signature; + if (signature.StartsWith("?", StringComparison.InvariantCulture)) + { + signature = signature.Substring(1); + } + query = string.IsNullOrEmpty(query) ? '?' + signature : query + '&' + signature; + message.Request.Uri.Query = query; + } + } + } +} diff --git a/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs b/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs new file mode 100644 index 000000000000..7f990665a530 --- /dev/null +++ b/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Threading; +using System.Threading.Tasks; +using Azure.Core.Pipeline; +using Azure.Core.TestFramework; +using NUnit.Framework; +using NUnit.Framework.Internal; + +namespace Azure.Core.Tests +{ + public class AzureSasCredentialPolicyTests : PolicyTestBase + { + [Test] + public async Task SetsSignatureEmptyQuery() + { + string signatureValue = "sig=test_signature_value"; + var transport = new MockTransport(new MockResponse(200)); + var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + + await SendGetRequest(transport, sasPolicy); + + Assert.AreEqual($"?{signatureValue}", transport.SingleRequest.Uri.Query); + } + + [Test] + public async Task SetsSignatureNonEmptyQuery() + { + string signatureValue = "sig=test_signature_value"; + var transport = new MockTransport(new MockResponse(200)); + var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + string query = "?foo=bar"; + + await SendGetRequest(transport, sasPolicy, query: query); + + Assert.AreEqual($"{query}&{signatureValue}", transport.SingleRequest.Uri.Query); + } + + [Test] + public async Task SetsSignatureThatHasQuestionMarkEmptyQuery() + { + string signatureValue = "?sig=test_signature_value"; + var transport = new MockTransport(new MockResponse(200)); + var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + + await SendGetRequest(transport, sasPolicy); + + Assert.AreEqual(signatureValue, transport.SingleRequest.Uri.Query); + } + + [Test] + public async Task SetsSignatureThatHasQuestionMarkNonEmptyQuery() + { + string signatureValue = "?sig=test_signature_value"; + var transport = new MockTransport(new MockResponse(200)); + var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + string query = "?foo=bar"; + + await SendGetRequest(transport, sasPolicy, query: query); + + Assert.AreEqual("?foo=bar&sig=test_signature_value", transport.SingleRequest.Uri.Query); + } + + [Test] + public async Task VerifyRetry() + { + string signatureValue = "sig=test_signature_value"; + var transport = new MockTransport(new MockResponse(200), new MockResponse(200)); + var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + + using (Request request = transport.CreateRequest()) + { + request.Method = RequestMethod.Get; + var pipeline = new HttpPipeline(transport, new[] { sasPolicy }); + await pipeline.SendRequestAsync(request, CancellationToken.None); + await pipeline.SendRequestAsync(request, CancellationToken.None); + } + + Assert.AreEqual($"?{signatureValue}", transport.Requests[0].Uri.Query); + } + } +} diff --git a/sdk/core/Azure.Core/tests/PolicyTestBase.cs b/sdk/core/Azure.Core/tests/PolicyTestBase.cs index 890d577ee871..e4aebef96861 100644 --- a/sdk/core/Azure.Core/tests/PolicyTestBase.cs +++ b/sdk/core/Azure.Core/tests/PolicyTestBase.cs @@ -11,7 +11,7 @@ namespace Azure.Core.Tests { public abstract class PolicyTestBase { - protected static async Task SendGetRequest(HttpPipelineTransport transport, HttpPipelinePolicy policy, ResponseClassifier responseClassifier = null) + protected static async Task SendGetRequest(HttpPipelineTransport transport, HttpPipelinePolicy policy, ResponseClassifier responseClassifier = null, string query = null) { Assert.IsInstanceOf(policy, "Use SyncAsyncPolicyTestBase base type for non-sync policies"); @@ -19,6 +19,7 @@ protected static async Task SendGetRequest(HttpPipelineTransport trans { request.Method = RequestMethod.Get; request.Uri.Reset(new Uri("http://example.com")); + request.Uri.Query = query; var pipeline = new HttpPipeline(transport, new[] { policy }, responseClassifier); return await pipeline.SendRequestAsync(request, CancellationToken.None); } From 5fdb953a0361f812417b04a6b50e4f9cb81e03f2 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Thu, 17 Dec 2020 14:11:44 -0800 Subject: [PATCH 02/12] corner case. --- .../src/Shared/AzureSasCredentialPolicy.cs | 12 ++-- .../tests/AzureSasCredentialPolicyTests.cs | 58 +++++++++---------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs index 566f7ba8cbf8..2655230b38d1 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs @@ -25,13 +25,13 @@ public override void OnSendingRequest(HttpMessage message) { base.OnSendingRequest(message); string query = message.Request.Uri.Query; - if (!query.Contains(_credential.Signature)) + string signature = _credential.Signature; + if (signature.StartsWith("?", StringComparison.InvariantCulture)) + { + signature = signature.Substring(1); + } + if (!query.Contains(signature)) { - string signature = _credential.Signature; - if (signature.StartsWith("?", StringComparison.InvariantCulture)) - { - signature = signature.Substring(1); - } query = string.IsNullOrEmpty(query) ? '?' + signature : query + '&' + signature; message.Request.Uri.Query = query; } diff --git a/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs b/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs index 7f990665a530..06cb843d0e6d 100644 --- a/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs +++ b/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs @@ -6,78 +6,72 @@ using Azure.Core.Pipeline; using Azure.Core.TestFramework; using NUnit.Framework; -using NUnit.Framework.Internal; namespace Azure.Core.Tests { public class AzureSasCredentialPolicyTests : PolicyTestBase { - [Test] - public async Task SetsSignatureEmptyQuery() + [TestCase("sig=test_signature_value")] + [TestCase("?sig=test_signature_value")] + public async Task SetsSignatureEmptyQuery(string signatureValue) { - string signatureValue = "sig=test_signature_value"; var transport = new MockTransport(new MockResponse(200)); var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); await SendGetRequest(transport, sasPolicy); - Assert.AreEqual($"?{signatureValue}", transport.SingleRequest.Uri.Query); + Assert.AreEqual("?sig=test_signature_value", transport.SingleRequest.Uri.Query); } - [Test] - public async Task SetsSignatureNonEmptyQuery() + [TestCase("sig=test_signature_value")] + [TestCase("?sig=test_signature_value")] + public async Task SetsSignatureNonEmptyQuery(string signatureValue) { - string signatureValue = "sig=test_signature_value"; var transport = new MockTransport(new MockResponse(200)); var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); string query = "?foo=bar"; await SendGetRequest(transport, sasPolicy, query: query); - Assert.AreEqual($"{query}&{signatureValue}", transport.SingleRequest.Uri.Query); + Assert.AreEqual($"?foo=bar&sig=test_signature_value", transport.SingleRequest.Uri.Query); } - [Test] - public async Task SetsSignatureThatHasQuestionMarkEmptyQuery() + [TestCase("sig=test_signature_value")] + [TestCase("?sig=test_signature_value")] + public async Task VerifyRetryEmptyQuery(string signatureValue) { - string signatureValue = "?sig=test_signature_value"; - var transport = new MockTransport(new MockResponse(200)); - var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); - - await SendGetRequest(transport, sasPolicy); - - Assert.AreEqual(signatureValue, transport.SingleRequest.Uri.Query); - } - - [Test] - public async Task SetsSignatureThatHasQuestionMarkNonEmptyQuery() - { - string signatureValue = "?sig=test_signature_value"; - var transport = new MockTransport(new MockResponse(200)); + var transport = new MockTransport(new MockResponse(200), new MockResponse(200)); var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); - string query = "?foo=bar"; - await SendGetRequest(transport, sasPolicy, query: query); + using (Request request = transport.CreateRequest()) + { + request.Method = RequestMethod.Get; + var pipeline = new HttpPipeline(transport, new[] { sasPolicy }); + await pipeline.SendRequestAsync(request, CancellationToken.None); + await pipeline.SendRequestAsync(request, CancellationToken.None); + } - Assert.AreEqual("?foo=bar&sig=test_signature_value", transport.SingleRequest.Uri.Query); + Assert.AreEqual("?sig=test_signature_value", transport.Requests[0].Uri.Query); } - [Test] - public async Task VerifyRetry() + [TestCase("sig=test_signature_value")] + [TestCase("?sig=test_signature_value")] + public async Task VerifyRetryNonEmptyQuery(string signatureValue) { - string signatureValue = "sig=test_signature_value"; var transport = new MockTransport(new MockResponse(200), new MockResponse(200)); var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + string query = "?foo=bar"; using (Request request = transport.CreateRequest()) { request.Method = RequestMethod.Get; + request.Uri.Query = query; var pipeline = new HttpPipeline(transport, new[] { sasPolicy }); await pipeline.SendRequestAsync(request, CancellationToken.None); await pipeline.SendRequestAsync(request, CancellationToken.None); } - Assert.AreEqual($"?{signatureValue}", transport.Requests[0].Uri.Query); + Assert.AreEqual("?foo=bar&sig=test_signature_value", transport.Requests[0].Uri.Query); } } } From 61c3839c8912bd939ccc7fd6febc63132bf38dda Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Thu, 17 Dec 2020 15:37:18 -0800 Subject: [PATCH 03/12] api. --- sdk/core/Azure.Core/api/Azure.Core.net461.cs | 7 +++++++ sdk/core/Azure.Core/api/Azure.Core.net5.0.cs | 7 +++++++ sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs | 7 +++++++ 3 files changed, 21 insertions(+) diff --git a/sdk/core/Azure.Core/api/Azure.Core.net461.cs b/sdk/core/Azure.Core/api/Azure.Core.net461.cs index f22d63a15b29..5690e26096cc 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.net461.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.net461.cs @@ -22,6 +22,13 @@ public AzureKeyCredential(string key) { } public string Key { get { throw null; } } public void Update(string key) { } } + public partial class AzureSasCredential + { + public AzureSasCredential(string signature) { } + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + public string Signature { get { throw null; } } + public void Update(string signature) { } + } [System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)] public readonly partial struct ETag : System.IEquatable { diff --git a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs index bcabbc77fb10..e7a6b0df30ff 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs @@ -22,6 +22,13 @@ public AzureKeyCredential(string key) { } public string Key { get { throw null; } } public void Update(string key) { } } + public partial class AzureSasCredential + { + public AzureSasCredential(string signature) { } + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + public string Signature { get { throw null; } } + public void Update(string signature) { } + } [System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)] public readonly partial struct ETag : System.IEquatable { diff --git a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs index f22d63a15b29..5690e26096cc 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs @@ -22,6 +22,13 @@ public AzureKeyCredential(string key) { } public string Key { get { throw null; } } public void Update(string key) { } } + public partial class AzureSasCredential + { + public AzureSasCredential(string signature) { } + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + public string Signature { get { throw null; } } + public void Update(string signature) { } + } [System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)] public readonly partial struct ETag : System.IEquatable { From 2d9fb26ad99e572e19da3b1c48f5950f90d13732 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Sat, 26 Dec 2020 21:37:56 -0800 Subject: [PATCH 04/12] doc update. --- sdk/core/Azure.Core/src/AzureSasCredential.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/core/Azure.Core/src/AzureSasCredential.cs b/sdk/core/Azure.Core/src/AzureSasCredential.cs index bfdc985cca07..51f679a84e76 100644 --- a/sdk/core/Azure.Core/src/AzureSasCredential.cs +++ b/sdk/core/Azure.Core/src/AzureSasCredential.cs @@ -9,14 +9,14 @@ namespace Azure { /// /// Shared access signature credential used to authenticate to an Azure Service. - /// It provides the ability to update the signature without creating a new client. + /// It provides the ability to update the shared access signature without creating a new client. /// public class AzureSasCredential { private string _signature; /// - /// Signature used to authenticate to an Azure service. + /// Shared access signature used to authenticate to an Azure service. /// [EditorBrowsable(EditorBrowsableState.Never)] public string Signature @@ -28,7 +28,7 @@ public string Signature /// /// Initializes a new instance of the class. /// - /// Signature to use to authenticate with the Azure service. + /// Shared access signature to use to authenticate with the Azure service. /// /// Thrown when the is null. /// @@ -40,11 +40,11 @@ public string Signature #pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. /// - /// Updates the signature. - /// This is intended to be used when you've regenerated your signature + /// Updates the shared access signature. + /// This is intended to be used when you've regenerated your shared access signature /// and want to update long lived clients. /// - /// Signature to authenticate the service against. + /// Shared access signature to authenticate the service against. /// /// Thrown when the is null. /// From 637560692b20fff151e8a6316df4946be1b286f3 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Sat, 26 Dec 2020 22:19:23 -0800 Subject: [PATCH 05/12] less allocations. --- .../Azure.Core/src/Shared/AzureSasCredentialPolicy.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs index 2655230b38d1..dbfa515ed0fe 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Text; using Azure.Core.Pipeline; namespace Azure.Core @@ -28,12 +29,14 @@ public override void OnSendingRequest(HttpMessage message) string signature = _credential.Signature; if (signature.StartsWith("?", StringComparison.InvariantCulture)) { - signature = signature.Substring(1); + signature = signature.AsSpan().Slice(1).ToString(); } if (!query.Contains(signature)) { - query = string.IsNullOrEmpty(query) ? '?' + signature : query + '&' + signature; - message.Request.Uri.Query = query; + var newQuery = new StringBuilder(query, query.Length + signature.Length + 1); + newQuery.Append(string.IsNullOrEmpty(query) ? '?' : '&'); + newQuery.Append(signature); + message.Request.Uri.Query = newQuery.ToString(); } } } From 2cbe659d4388aa9a87191b4797883dbea5bbb9ae Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Sat, 26 Dec 2020 22:22:57 -0800 Subject: [PATCH 06/12] call base last. --- sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs index dbfa515ed0fe..4b30f1217dac 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs @@ -24,7 +24,6 @@ public AzureSasCredentialPolicy(AzureSasCredential credential) /// public override void OnSendingRequest(HttpMessage message) { - base.OnSendingRequest(message); string query = message.Request.Uri.Query; string signature = _credential.Signature; if (signature.StartsWith("?", StringComparison.InvariantCulture)) @@ -38,6 +37,8 @@ public override void OnSendingRequest(HttpMessage message) newQuery.Append(signature); message.Request.Uri.Query = newQuery.ToString(); } + + base.OnSendingRequest(message); } } } From 212e464107eaf3d103fb54cfb7d7cfc2ef7ba96d Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Mon, 4 Jan 2021 09:18:28 -0800 Subject: [PATCH 07/12] Revert "less allocations." This reverts commit 637560692b20fff151e8a6316df4946be1b286f3. --- .../Azure.Core/src/Shared/AzureSasCredentialPolicy.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs index 4b30f1217dac..2eb1962fdfbc 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System; -using System.Text; using Azure.Core.Pipeline; namespace Azure.Core @@ -28,14 +27,12 @@ public override void OnSendingRequest(HttpMessage message) string signature = _credential.Signature; if (signature.StartsWith("?", StringComparison.InvariantCulture)) { - signature = signature.AsSpan().Slice(1).ToString(); + signature = signature.Substring(1); } if (!query.Contains(signature)) { - var newQuery = new StringBuilder(query, query.Length + signature.Length + 1); - newQuery.Append(string.IsNullOrEmpty(query) ? '?' : '&'); - newQuery.Append(signature); - message.Request.Uri.Query = newQuery.ToString(); + query = string.IsNullOrEmpty(query) ? '?' + signature : query + '&' + signature; + message.Request.Uri.Query = query; } base.OnSendingRequest(message); From b24612582cec8d83f20df91bd7ec22badd74b8d1 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Mon, 4 Jan 2021 14:47:46 -0800 Subject: [PATCH 08/12] validation feedback. --- sdk/core/Azure.Core/src/AzureSasCredential.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/Azure.Core/src/AzureSasCredential.cs b/sdk/core/Azure.Core/src/AzureSasCredential.cs index 51f679a84e76..5248a7cb305c 100644 --- a/sdk/core/Azure.Core/src/AzureSasCredential.cs +++ b/sdk/core/Azure.Core/src/AzureSasCredential.cs @@ -53,7 +53,7 @@ public string Signature /// public void Update(string signature) { - Argument.AssertNotNullOrEmpty(signature, nameof(signature)); + Argument.AssertNotNullOrWhiteSpace(signature, nameof(signature)); Signature = signature; } } From a1bffb48761e07c8e7262317c4468816bd5d9841 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Mon, 4 Jan 2021 14:47:56 -0800 Subject: [PATCH 09/12] policy rename. --- sdk/core/Azure.Core/src/Azure.Core.csproj | 2 +- ...olicy.cs => AzureSasCredentialSynchronousPolicy.cs} | 8 ++++---- ....cs => AzureSasCredentialSynchronousPolicyTests.cs} | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) rename sdk/core/Azure.Core/src/Shared/{AzureSasCredentialPolicy.cs => AzureSasCredentialSynchronousPolicy.cs} (81%) rename sdk/core/Azure.Core/tests/{AzureSasCredentialPolicyTests.cs => AzureSasCredentialSynchronousPolicyTests.cs} (84%) diff --git a/sdk/core/Azure.Core/src/Azure.Core.csproj b/sdk/core/Azure.Core/src/Azure.Core.csproj index 05f1a1756d3c..264d5baa6954 100644 --- a/sdk/core/Azure.Core/src/Azure.Core.csproj +++ b/sdk/core/Azure.Core/src/Azure.Core.csproj @@ -26,7 +26,7 @@ - + diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs similarity index 81% rename from sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs rename to sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs index 2eb1962fdfbc..8f45cc770f1c 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs @@ -6,15 +6,15 @@ namespace Azure.Core { - internal class AzureSasCredentialPolicy : HttpPipelineSynchronousPolicy + internal class AzureSasCredentialSynchronousPolicy : HttpPipelineSynchronousPolicy { private readonly AzureSasCredential _credential; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - /// The used to authenticate requests. - public AzureSasCredentialPolicy(AzureSasCredential credential) + /// The used to authenticate requests. + public AzureSasCredentialSynchronousPolicy(AzureSasCredential credential) { Argument.AssertNotNull(credential, nameof(credential)); _credential = credential; diff --git a/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs b/sdk/core/Azure.Core/tests/AzureSasCredentialSynchronousPolicyTests.cs similarity index 84% rename from sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs rename to sdk/core/Azure.Core/tests/AzureSasCredentialSynchronousPolicyTests.cs index 06cb843d0e6d..546b9cd339f3 100644 --- a/sdk/core/Azure.Core/tests/AzureSasCredentialPolicyTests.cs +++ b/sdk/core/Azure.Core/tests/AzureSasCredentialSynchronousPolicyTests.cs @@ -9,14 +9,14 @@ namespace Azure.Core.Tests { - public class AzureSasCredentialPolicyTests : PolicyTestBase + public class AzureSasCredentialSynchronousPolicyTests : PolicyTestBase { [TestCase("sig=test_signature_value")] [TestCase("?sig=test_signature_value")] public async Task SetsSignatureEmptyQuery(string signatureValue) { var transport = new MockTransport(new MockResponse(200)); - var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + var sasPolicy = new AzureSasCredentialSynchronousPolicy(new AzureSasCredential(signatureValue)); await SendGetRequest(transport, sasPolicy); @@ -28,7 +28,7 @@ public async Task SetsSignatureEmptyQuery(string signatureValue) public async Task SetsSignatureNonEmptyQuery(string signatureValue) { var transport = new MockTransport(new MockResponse(200)); - var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + var sasPolicy = new AzureSasCredentialSynchronousPolicy(new AzureSasCredential(signatureValue)); string query = "?foo=bar"; await SendGetRequest(transport, sasPolicy, query: query); @@ -41,7 +41,7 @@ public async Task SetsSignatureNonEmptyQuery(string signatureValue) public async Task VerifyRetryEmptyQuery(string signatureValue) { var transport = new MockTransport(new MockResponse(200), new MockResponse(200)); - var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + var sasPolicy = new AzureSasCredentialSynchronousPolicy(new AzureSasCredential(signatureValue)); using (Request request = transport.CreateRequest()) { @@ -59,7 +59,7 @@ public async Task VerifyRetryEmptyQuery(string signatureValue) public async Task VerifyRetryNonEmptyQuery(string signatureValue) { var transport = new MockTransport(new MockResponse(200), new MockResponse(200)); - var sasPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(signatureValue)); + var sasPolicy = new AzureSasCredentialSynchronousPolicy(new AzureSasCredential(signatureValue)); string query = "?foo=bar"; using (Request request = transport.CreateRequest()) From 89cc8672be2796a62e61f4c198f1605b7eb57b3f Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Mon, 4 Jan 2021 15:12:43 -0800 Subject: [PATCH 10/12] another attempt to reduce allocations. --- .../AzureSasCredentialSynchronousPolicy.cs | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs index 8f45cc770f1c..ad59a7e70b84 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Text; using Azure.Core.Pipeline; namespace Azure.Core @@ -24,15 +25,24 @@ public AzureSasCredentialSynchronousPolicy(AzureSasCredential credential) public override void OnSendingRequest(HttpMessage message) { string query = message.Request.Uri.Query; + var querySpan = query.AsSpan(); string signature = _credential.Signature; - if (signature.StartsWith("?", StringComparison.InvariantCulture)) + bool hasLeadingQuestionMark = signature.StartsWith("?", StringComparison.InvariantCulture); + var signatureSpan = signature.AsSpan(hasLeadingQuestionMark ? 1 : 0); + if (!querySpan.Contains(signatureSpan, StringComparison.InvariantCulture)) { - signature = signature.Substring(1); - } - if (!query.Contains(signature)) - { - query = string.IsNullOrEmpty(query) ? '?' + signature : query + '&' + signature; - message.Request.Uri.Query = query; + var finalQuery = new StringBuilder(query.Length + signatureSpan.Length + 1); + if (string.IsNullOrEmpty(query)) + { + finalQuery.Append('?'); + } + else + { + finalQuery.Append(query); + finalQuery.Append('&'); + } + finalQuery.Append(signature, hasLeadingQuestionMark ? 1 : 0, hasLeadingQuestionMark ? signature.Length - 1 : signature.Length); + message.Request.Uri.Query = finalQuery.ToString(); } base.OnSendingRequest(message); From 3d76464b0d97d5d726d8d6b64e56d99dc6f7c489 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Mon, 4 Jan 2021 15:24:30 -0800 Subject: [PATCH 11/12] Revert "another attempt to reduce allocations." This reverts commit 89cc8672be2796a62e61f4c198f1605b7eb57b3f. --- .../AzureSasCredentialSynchronousPolicy.cs | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs index ad59a7e70b84..8f45cc770f1c 100644 --- a/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs +++ b/sdk/core/Azure.Core/src/Shared/AzureSasCredentialSynchronousPolicy.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System; -using System.Text; using Azure.Core.Pipeline; namespace Azure.Core @@ -25,24 +24,15 @@ public AzureSasCredentialSynchronousPolicy(AzureSasCredential credential) public override void OnSendingRequest(HttpMessage message) { string query = message.Request.Uri.Query; - var querySpan = query.AsSpan(); string signature = _credential.Signature; - bool hasLeadingQuestionMark = signature.StartsWith("?", StringComparison.InvariantCulture); - var signatureSpan = signature.AsSpan(hasLeadingQuestionMark ? 1 : 0); - if (!querySpan.Contains(signatureSpan, StringComparison.InvariantCulture)) + if (signature.StartsWith("?", StringComparison.InvariantCulture)) { - var finalQuery = new StringBuilder(query.Length + signatureSpan.Length + 1); - if (string.IsNullOrEmpty(query)) - { - finalQuery.Append('?'); - } - else - { - finalQuery.Append(query); - finalQuery.Append('&'); - } - finalQuery.Append(signature, hasLeadingQuestionMark ? 1 : 0, hasLeadingQuestionMark ? signature.Length - 1 : signature.Length); - message.Request.Uri.Query = finalQuery.ToString(); + signature = signature.Substring(1); + } + if (!query.Contains(signature)) + { + query = string.IsNullOrEmpty(query) ? '?' + signature : query + '&' + signature; + message.Request.Uri.Query = query; } base.OnSendingRequest(message); From 8944925abdbd9bc6f4a4df3c534a8e86e5fb48c8 Mon Sep 17 00:00:00 2001 From: Kamil Sobol Date: Mon, 4 Jan 2021 15:36:11 -0800 Subject: [PATCH 12/12] changelog. --- sdk/core/Azure.Core/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/core/Azure.Core/CHANGELOG.md b/sdk/core/Azure.Core/CHANGELOG.md index 776d4f0d69e5..c45a67d3054e 100644 --- a/sdk/core/Azure.Core/CHANGELOG.md +++ b/sdk/core/Azure.Core/CHANGELOG.md @@ -2,6 +2,9 @@ ## 1.8.0-beta.1 (Unreleased) +### Added +- `AzureSasCredential` and its respective policy. + ## 1.7.0 (2020-12-14) ### New Features