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

[wghilliard]

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[bot]]

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

[jsquire]

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.

[github-actions[bot]]

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]

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);

[wghilliard]

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]

@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

[amnguye]

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]

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

[xperiandri]

Must validate!!! MUST VALIDATE!!!

[nickliu-msft]

Reading through the comments here, it seems like we can leverage BlobUriBuilder to validate if the BlobContainerName is set in the Uri passed into BlobContainerClient's ctor. If not we can throw an argument exception. This seems like relatively straightforward fix and would be useful, though the existing mocks could be an issue.

Bumping @amnguye question from above, what are yalls thoughts on this fix? @seanmcc-msft @jaschrep-msft