Skip to content
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

[Feature Request] BlobContainerClient should validate the Uri include a container name #43509

Open
wghilliard opened this issue Apr 18, 2024 · 8 comments
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. needs-team-attention This issue needs attention from Azure service team or SDK team Service Attention This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)
Milestone

Comments

@wghilliard
Copy link

wghilliard commented Apr 18, 2024

Azure Storage Team: Please see the last two comments in the discussion thread for the feature request and rationale behind it.

Library name and version

Azure.Messaging.EventHubs 5.11.2

Describe the bug

When using a BlobContainerClient with a TokenCredential for checkpointing with the EventProccessorClient, the processor fails on startup because of a malformed query being sent via the BlobContainerClient. This issue is not reproducible when using a Connection String for the BlobContainerClient.

Expected behavior

The EventProcessorClient should successfully check the ownership of the of the partition using the BlobContainerClient, however it seems like it's adding an extra slash to the query which is causing a 400.

Actual behavior

The EventProcessorClient fails to start because ownership of the partition cannot be read.

Reproduction Steps

  1. Configure a BlobContainerClient with a TokenCredential.
  2. Configure a EventProcesserClient with the previously created BlobContainerClient.
  3. Start the processor.

Below is the error message I encountered.

      Azure.Messaging.EventHubs.EventHubsException(GeneralError): Retrieving list of ownership from the storage service..  For troubleshooting information, see https://aka.ms/azsdk/net/eventhubs/exceptions/troubleshoot
         at Azure.Messaging.EventHubs.Primitives.PartitionLoadBalancer.RunLoadBalancingAsync(String[] partitionIds, CancellationToken cancellationToken)
         at Azure.Messaging.EventHubs.Primitives.EventProcessor`1.PerformLoadBalancingAsync(ValueStopwatch cycleDuration, String[] partitionIds, CancellationToken cancellationToken)
      Azure.RequestFailedException: Value for one of the query parameters specified in the request URI is invalid.
RequestId: [redacted]
Time :[redacted]
      Status: 400 (Value for one of the query parameters specified in the request URI is invalid.)
      ErrorCode: InvalidQueryParameterValue

      Additional Information:
      QueryParameterName: prefix
      QueryParameterValue: nyancat.servicebus.windows.net/mynamespace/myconsumergroup/ownership/
      Reason: Cannot contain slash.

      Content:
      ?<?xml version="1.0" encoding="utf-8"?><Error><Code>InvalidQueryParameterValue</Code><Message>Value for one of the query parameters specified in the request URI is invalid.
RequestId:[redacted]
Time:[redacted]</Message><QueryParameterName>prefix</QueryParameterName><QueryParameterValue>nyancat.servicebus.windows.net/mynamespace/myconsumergroup/ownership/</QueryParameterValue><Reason>Cannot contain slash.</Reason></Error>

      Headers:
      Server: Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0
      x-ms-request-id: [redacted]
      x-ms-client-request-id: [redacted]
      x-ms-version: 2023-11-03
      x-ms-error-code: InvalidQueryParameterValue
      Date: [redacted]
      Content-Length: 481
      Content-Type: application/xml

         at Azure.Storage.Blobs.ContainerRestClient.ListBlobFlatSegmentAsync(String prefix, String marker, Nullable`1 maxresults, IEnumerable`1 include, Nullable`1 timeout, CancellationToken cancellationToken)
         at Azure.Storage.Blobs.BlobContainerClient.GetBlobsInternal(String marker, BlobTraits traits, BlobStates states, String prefix, Nullable`1 pageSizeHint, Boolean async, CancellationToken cancellationToken)
         at Azure.Storage.Blobs.Models.GetBlobsAsyncCollection.GetNextPageAsync(String continuationToken, Nullable`1 pageSizeHint, Boolean async, CancellationToken cancellationToken)
         at Azure.Storage.StorageCollectionEnumerator`1.StorageAsyncPageable.GetAsyncEnumerator(CancellationToken cancellationToken)+MoveNext()
         at Azure.Storage.StorageCollectionEnumerator`1.StorageAsyncPageable.GetAsyncEnumerator(CancellationToken cancellationToken)+System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult()
         at Azure.Messaging.EventHubs.Primitives.BlobCheckpointStoreInternal.ListOwnershipAsync(String fullyQualifiedNamespace, String eventHubName, String consumerGroup, CancellationToken cancellationToken)
         at Azure.Messaging.EventHubs.Primitives.BlobCheckpointStoreInternal.ListOwnershipAsync(String fullyQualifiedNamespace, String eventHubName, String consumerGroup, CancellationToken cancellationToken)
         at Azure.Messaging.EventHubs.Primitives.EventProcessor`1.DelegatingCheckpointStore.ListOwnershipAsync(String fullyQualifiedNamespace, String eventHubName, String consumerGroup, CancellationToken cancellationToken)
         at Azure.Messaging.EventHubs.Primitives.PartitionLoadBalancer.RunLoadBalancingAsync(String[] partitionIds, CancellationToken cancellationToken)

Environment

> dotnet --info
.NET SDK:
 Version:           8.0.204
 Commit:            c338c7548c
 Workload version:  8.0.200-manifests.7d36c14f

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.204\

.NET workloads installed:
There are no installed workloads to display.

Host:
  Version:      8.0.4
  Architecture: x64
  Commit:       2d7eea2529

.NET SDKs installed:
  6.0.421 [C:\Program Files\dotnet\sdk]
  8.0.202 [C:\Program Files\dotnet\sdk]
  8.0.204 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.28 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.29 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.28 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.29 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.28 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.29 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.18 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files) labels Apr 18, 2024
Copy link

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

@jsquire
Copy link
Member

jsquire commented Apr 19, 2024

Hi @wghilliard: Thanks for reaching out and we regret that you're experiencing difficulties. The Event Hubs library does not directly interact with storage; it makes use of the Azure Storage library for all storage operations, using the BlobContainerClient that you pass to it as part of construction. As this lies outside of Event Hubs, the Azure Storage team has been looped in to offer assistance.

I don't believe this is related to token credential use, as this scenario has a comprehensive suite of tests that cover it and run nightly. I see no failures, I am not able to reproduce locally, and we have no other reports of this failure when using credentials. More likely, I suspect that the failure that you're seeing may have something to do with the BlobContainerClient configuration.

Please provide a code snippet that demonstrates how you're constructing your BlobContainerClient, using the exact container name that you're using, if possible, or if that contains sensitive information, a container name that uses the exact same pattern.

@jsquire jsquire added needs-author-feedback More information is needed from author to address the issue. and removed needs-team-attention This issue needs attention from Azure service team or SDK team labels Apr 19, 2024
Copy link

Hi @wghilliard. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@wghilliard
Copy link
Author

wghilliard commented Apr 19, 2024

I think the error could be in how the path is being constructed when performing the partition ownership check, note the trailing slash in the query

QueryParameterName: prefix
QueryParameterValue: nyancat.servicebus.windows.net/mynamespace/myconsumergroup/ownership/

Here is the snippet demonstrating how I'm constructing the BlobContainerClient

new BlobContainerClient(new Uri("https://nyancat.blob.core.windows.net/"), tokenCredential);

@github-actions github-actions bot added needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-author-feedback More information is needed from author to address the issue. labels Apr 19, 2024
@wghilliard
Copy link
Author

It seems like perhaps the exception might be caused by not providing a suffixed container name in the URI. After appending it, I think the SDK is working as expected (currently attempting to get role assignments sorted out such that I can test).

Given this was perhaps caused by user error, could we add validation to the BlobContainerClient constructor to ensure the URI is the shape it is expecting?

@jsquire
Copy link
Member

jsquire commented Apr 19, 2024

@wghilliard: Yes, that's what it appears to be. You're creating a BlobContainerClient without a container name. To do so, you'd either need to form the URL with the container name:

var containerClient = new BlobContainerClient(new Uri("https://nyancat.blob.core.windows.net/mycontainer"), tokenCredential);

or use a BlobServiceClient to build the container client:

var containerClient = new BlobServiceClient(new Uri("https://nyancat.blob.core.windows.net"), tokenCredential)
    .GetBlobContainerClient("mycontainer");

Given this was perhaps caused by user error, could we add validation to the BlobContainerClient constructor to ensure the URI is the shape it is expecting?

I agree that would be a good idea. I'll update the title and leave this open for the Azure Storage team to review/consider, as the owners of the Storage SDK packages.

//cc: @amnguye

@jsquire jsquire changed the title [BUG] Azure EventHub SDK creates bad requests when using TokenCredentials [Feature Request] BlobContainerClient should validate the Uri includes a container name Apr 19, 2024
@jsquire jsquire added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-team-attention This issue needs attention from Azure service team or SDK team labels Apr 19, 2024
@jsquire jsquire added this to the Backlog milestone Apr 19, 2024
@github-actions github-actions bot added the needs-team-attention This issue needs attention from Azure service team or SDK team label Apr 19, 2024
@jsquire jsquire changed the title [Feature Request] BlobContainerClient should validate the Uri includes a container name [Feature Request] BlobContainerClient should validate the Uri include a container name Apr 19, 2024
@amnguye
Copy link
Member

amnguye commented Apr 19, 2024

I could see the SDK providing a way to validate blob container Uri's, to validate the name is in the URL.

This does require us to make sure we cover all our bases with every single type of storage Uri out there (e.g. host IP style endpoints).

Today we don't have checks on the Uri passed to any storage client constructor. I believe adding these checks wouldn't break any happy path behavior (since people would know they passed an invalid Uri eventually, when they go to make an API call).

I wonder though if it does break those who are mocking the storage clients (e.g. BlobContainerClient), handing it a bad Uri and just expecting it to work. Just a thought.

@seanmcc-msft @jaschrep-msft Are we good to just throw an ArgumentException in this case? We can easily reuse the BlobUriBuilder to validate for us.

@wghilliard
Copy link
Author

That's a good point about mocks, perhaps the builder or an option during construction to run / suppress validation??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. needs-team-attention This issue needs attention from Azure service team or SDK team Service Attention This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

3 participants