Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.32k stars 4.66k forks source link

[BUG] SasQueryParameters unable to parse multiple DateTime query parameters from a Uri authenticated with a Sas Token #15798

Closed hikarunoryoma closed 3 years ago

hikarunoryoma commented 3 years ago

Describe the bug GetBlobClient is unable to parse my Uri if I have multiple date query parameters. Note that I am using a SAS token generated from an azure storage using version 2017-04-17.

Expected behavior What is the expected behavior? I would expect that the query string would be able to be parsed and if it was invalid I would hit a 403 web request error.

Actual behavior (include Exception or Stack Trace) What is the actual behavior? I hit a Format Exception at SasQueryParameters..ctor(IDictionary`2 values)

See exception detail here:

Exception Type: System.FormatException
Error message: String was not recognized as a valid DateTime. 
at System.DateTimeParse.ParseExact(String s, String format, DateTimeFormatInfo dtfi, DateTimeStyles style, TimeSpan& offset)
at System.DateTimeOffset.ParseExact(String input, String format, IFormatProvider formatProvider, DateTimeStyles styles)
at Azure.Storage.Sas.SasQueryParameters..ctor(IDictionary`2 values)
at Azure.Storage.Blobs.BlobUriBuilder..ctor(Uri uri)
at Azure.Storage.Blobs.BlobContainerClient.GetBlobClient(String blobName)

To Reproduce Steps to reproduce the behavior (include a code snippet, screenshot, or any additional information that might help us reproduce the issue)

I am able to reproduce using Linqpad that while I am able to use the containerService, the blob service fails. I've redacted identifying information, but the below code will still error out since we fail before issuing the web request.

void Main()
{
    var baseUrl1 = "https://serviceAccountName.blob.core.windows.net/";
    var queryString1 = "sv=2017-04-17&sp=rl&sr=c&sig={redacted}&se=2020-12-25";

    var uri1 = new UriBuilder(baseUrl1)
    {
        Query = queryString1
    }.Uri;

    var serviceClient = new BlobServiceClient(uri1);
    var container = serviceClient.GetBlobContainerClient("aptdata");
    var blobClient = container.GetBlobClient("randomBlob"); // fails here
}

The same information works using the Azure CLI

Environment:

Runtime Environment: OS Name: Windows OS Version: 10.0.18363 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.200\

Host (useful for support): Version: 3.1.2 Commit: 916b5cba26

.NET Core SDKs installed: 2.1.4 [C:\Program Files\dotnet\sdk] 2.1.202 [C:\Program Files\dotnet\sdk] 2.1.509 [C:\Program Files\dotnet\sdk] 3.1.200 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]



 - IDE and version : Visual Studio v. 16.5.0
jsquire commented 3 years ago

Thank you for your feedback. Tagging and routing to the team best able to assist.

ghost commented 3 years ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

seanmcc-msft commented 3 years ago

Hi @hikarunoryoma, I can reproduce this issue and am investigating.

seanmcc-msft commented 3 years ago

@hikarunoryoma, I believe the Storage Service expects the SAS DateTime format to be yyyy-MM-ddTHH:mm:ssZ. How did you go about generating a SAS for the 2017-04-17 SAS version?

hikarunoryoma commented 3 years ago

A client team provided the Sas token for us so I'll have to get back to you on how these were constructed.

I will note that if you remove the se or sv param the query string is parseable. I have another Uri structured as https://accountName.blob.core.windows.net/?sv=2018-11-09&si={si}&sr=c&sig={sig} that works fine for us.

seanmcc-msft commented 3 years ago

That makes sense, the sv parameter format is yyyy-MM-dd. I think the ambiguity is in whether the Storage service accepts st and se with the format yyyy-MM-dd. I suspect it doesn't, but I may be wrong.

hikarunoryoma commented 3 years ago

The SAS token is definitely valid (according to my understanding) as I am still able to invoke BlobContainerClient methods such as GetBlobsAsync. Unsure if that helps.

seanmcc-msft commented 3 years ago

That is actually really helpful, I will investigate further.

hikarunoryoma commented 3 years ago

I also just tried editing my variable in LinqPad script to var queryString1 = "sv=2017-04-17&sp=rl&sr=c&sig={redacted}&se=2016-06-23T09:07:21.20-07:00" and I hit the same FormatException

I'm not too great at date formats so just grabbed the date from here: https://numenta.com/machine-intelligence-technology/htm-studio/date-time-formats/

seanmcc-msft commented 3 years ago

Right now, we are throwing if the format is not yyyy-MM-ddTHH:mm:ssZ. https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Common/src/Shared/Constants.cs#L73. The question is if the service can take other DateTime formats, which I need to look into.

hikarunoryoma commented 3 years ago

@seanmcc-msft any updates on this? On our end we'll still have to wait on how this token was generated, but we've found that editing the SasToken in anyway renders it invalid on authentication

ghost commented 3 years ago

@seankane-msft - is there an update here? I am hitting a similar issue with a very old token which works fine in powershell.

seanmcc-msft commented 3 years ago

We know about the issue, I'm going to look into trying to fix it later this week.

danielwhyte-smudge commented 3 years ago

We are also facing a similar problem using connection strings to instantiate our BlobServiceClient

For our deployments we use ARM to generate a scoped connection string, necessary to prevent a single resource from accessing things outside its allowed scope:

BlobEndpoint=https://{{truncated}}.blob.core.windows.net/;SharedAccessSignature=sv=2015-04-05&sr=c&spr=https&se=3121-01-01T00%3A31%3A21.0000000Z&sp=rwdl&sig={{truncated}}

On start up we read this connection string from the configuration, and then call the following code:

BlobServiceClient serviceClient = new BlobServiceClient(connectionString);

This results in the following error:

Exception while executing function: NodeMetricsHasUpdates String '3121-01-01T00:31:21.0000000Z' was not recognized as a valid DateTime. 

Thought I should comment here instead of opening a new issue, however I'm happy to do so.

Our current work around is to continue using the V11 of the SDK as the connection string works fine.

seanmcc-msft commented 3 years ago

I have merged a PR with a fix, it will go out with next next release.