fix(core-amqp): ignore ASCII case in the entity path match - #7343
Draft
Johnathan W (j7nw4r) wants to merge 3 commits into
Draft
fix(core-amqp): ignore ASCII case in the entity path match#7343Johnathan W (j7nw4r) wants to merge 3 commits into
Johnathan W (j7nw4r) wants to merge 3 commits into
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes case-sensitive Event Hub entity-path validation by using the existing ASCII case-insensitive comparison.
Changes:
- Accepts entity paths differing only by ASCII letter case.
- Removes an unused private member.
- Adds tests and updates API documentation, README, and changelogs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
sdk/core/azure-core-amqp/inc/azure/core/amqp/internal/connection_string_credential.hpp |
Implements case-insensitive validation. |
sdk/core/azure-core-amqp/test/ut/connection_string_tests.cpp |
Tests matching and edge cases. |
sdk/core/azure-core-amqp/CHANGELOG.md |
Records core behavior change. |
sdk/eventhubs/azure-messaging-eventhubs/test/ut/connection_string_test.cpp |
Tests producer and consumer behavior. |
sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp |
Documents producer validation. |
sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp |
Documents consumer validation. |
sdk/eventhubs/azure-messaging-eventhubs/README.md |
Explains connection-string matching. |
sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md |
Records client behavior change. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
The ServiceBusSasConnectionStringCredential constructor compares the entity path argument with the connection string EntityPath element byte for byte. A caller that gives a different case gets an exception. These tests pin the case-insensitive comparison that issue 7261 asks for. Three tests fail against the unchanged source: ServiceBusSasEntityPathCaseInsensitive, ProducerAcceptsCaseInsensitiveEntityPath, and ConsumerAcceptsCaseInsensitiveEntityPath. Four more items pass today, and they guard the behavior that must not change: an empty entity path argument, a connection string without an EntityPath element, a non-ASCII byte pair that stays a mismatch, and an explicit event hub name that keeps its case.
The ServiceBusSasConnectionStringCredential constructor compared the connection string EntityPath element and the entityPath argument byte for byte. A caller that spelled the name with different ASCII letter case got a std::invalid_argument. Issue 7261 asks for a match that ignores that case. The constructor now calls StringExtensions::LocaleInvariantCaseInsensitiveEqual. That helper folds ASCII A-Z only. It is not the culture-aware comparison that .NET uses. An argument that differs by more than ASCII letter case still throws std::invalid_argument with the same message. An empty argument still keeps the connection string value. GetEntityPath() returns the connection string spelling. ProducerClient and ConsumerClient read that value through EventHubsUtilities::CreateConnectionStringDetails, so GetEventHubName() returns the connection string spelling too. .NET returns the explicit argument instead, so this behavior deviates from .NET. The change also deletes the dead private member m_entityPath from ServiceBusSasConnectionStringCredential. Only the deleted branch wrote to that member, and no code read it. The identically named member of ConnectionStringParser stays. The header comments, the Event Hubs README, and both changelogs record the new rule.
The branch diff carried 16 comment lines against 78 added lines, which is 20.5 percent. The budget is 10 percent. Delete the comment above ServiceBusSasEntityPathAbsentFromConnectionString, because the test name states it. Cut the two comments above ServiceBusSasEntityPathArgumentEmpty and ServiceBusSasEntityPathNonAsciiMismatch to one line each, and keep the reason a reader cannot get from the code. No test name, no test body, and no assertion changes.
Johnathan W (j7nw4r)
force-pushed
the
fix/7261-case-insensitive-entity-path
branch
from
August 18, 2026 17:49
9baad0b to
dab0599
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ServiceBusSasConnectionStringCredentialcompared the entity path argument against the connection stringEntityPathelement byte for byte. A caller who passedMyHubagainst a connection string that carriesEntityPath=myhubgotstd::invalid_argumentbefore any network call. The comparison now ignores ASCII letter case.Motivation
The Microsoft resource naming rules say to always do a case-insensitive comparison of names, and the
namespaces/eventhubsrow lists no case exception. .NET compares the same two names withStringComparison.InvariantCultureIgnoreCaseinEventHubsConnectionStringProperties.Validate. The byte comparison in C++ rejected a pair of names that the service accepts, soProducerClientandConsumerClientthrew on a valid input. This change aligns with .NET. A legal Event Hubs entity name contains ASCII alphanumerics plus period, hyphen, and underscore. An ASCII fold and the .NET culture-aware fold thus agree on every name that the service accepts.Changes
ServiceBusSasConnectionStringCredentialmatches the two names through the existingAzure::Core::_internal::StringExtensions::LocaleInvariantCaseInsensitiveEqualhelper.std::invalid_argumentwith the same message.m_entityPath, which the constructor wrote and no code read.azure-core-amqpandazure-messaging-eventhubschangelogs: the fold covers ASCII A-Z only, andGetEntityPath()returns the connection string spelling.ProducerClientandConsumerClientconstructor documentation and the Event Hubs README.ApiViewSettings.jsonsetsincludePrivateto false.Test plan
ConnectionStringTest.ServiceBusSasEntityPathCaseInsensitive,ConnectionStringClientTest.ProducerAcceptsCaseInsensitiveEntityPath, andConnectionStringClientTest.ConsumerAcceptsCaseInsensitiveEntityPath.EntityPath, a non-ASCII byte pair that must still throw, and a mixed-case caller name against a connection string with noEntityPath.Fixes #7261