Skip to content

Commit 736b825

Browse files
jsquirejsquire
authored
[Event Hubs Client] Enhanced Connection String Validation (Azure#13288)
The focus of these changes is to enhance connection string validation for all clients to ensure that issues are detected where reasonable messages can be displayed. Previously, validation was delegated to the EventHubConnection which left a gap when the connection was not eagerly initialized, such as with the event processor types.
1 parent 2a64a67 commit 736b825

File tree

10 files changed

+247
-49
lines changed

10 files changed

+247
-49
lines changed

‎sdk/eventhub/Azure.Messaging.EventHubs.Shared/src/Core/ConnectionStringProperties.cs

100755100644
+35
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,40 @@ public ConnectionStringProperties(Uri endpoint,
5959
SharedAccessKeyName = sharedAccessKeyName;
6060
SharedAccessKey = sharedAccessKey;
6161
}
62+
63+
/// <summary>
64+
/// Performs the actions needed to validate the set of connection string properties for connecting to the
65+
/// Event Hubs service.
66+
/// </summary>
67+
///
68+
/// <param name="explicitEventHubName">The name of the Event Hub that was explicitly passed independent of the connection string, allowing easier use of a namespace-level connection string.</param>
69+
/// <param name="connectionStringArgumentName">The name of the argument associated with the connection string; to be used when raising <see cref="ArgumentException" /> variants.</param>
70+
///
71+
/// <exception cref="ArgumentException">In the case that the properties violate an invariant or otherwise represent a combination that is not permissible, an appropriate exception will be thrown.</exception>
72+
///
73+
public void Validate(string explicitEventHubName,
74+
string connectionStringArgumentName)
75+
{
76+
// The Event Hub name may only be specified in one of the possible forms, either as part of the
77+
// connection string or as a stand-alone parameter, but not both. If specified in both to the same
78+
// value, then do not consider this a failure.
79+
80+
if ((!string.IsNullOrEmpty(explicitEventHubName))
81+
&& (!string.IsNullOrEmpty(EventHubName))
82+
&& (!string.Equals(explicitEventHubName, EventHubName, StringComparison.InvariantCultureIgnoreCase)))
83+
{
84+
throw new ArgumentException(Resources.OnlyOneEventHubNameMayBeSpecified, connectionStringArgumentName);
85+
}
86+
87+
// Ensure that each of the needed components are present for connecting.
88+
89+
if ((string.IsNullOrEmpty(explicitEventHubName)) && (string.IsNullOrEmpty(EventHubName))
90+
|| (string.IsNullOrEmpty(Endpoint?.Host))
91+
|| (string.IsNullOrEmpty(SharedAccessKeyName))
92+
|| (string.IsNullOrEmpty(SharedAccessKey)))
93+
{
94+
throw new ArgumentException(Resources.MissingConnectionInformation, connectionStringArgumentName);
95+
}
96+
}
6297
}
6398
}

‎sdk/eventhub/Azure.Messaging.EventHubs.Shared/src/Resources.resx

+3-3
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,10 @@
151151
<value>The connection string could not be parsed; either it was malformed or contains no well-known tokens.</value>
152152
</data>
153153
<data name="MissingConnectionInformation" xml:space="preserve">
154-
<value>The connection string used for an Event Hub client must specify the Event Hubs namespace host, and a Shared Access Signature (both the name and value) to be valid. The path to an Event Hub must be included in the connection string or specified separately.</value>
154+
<value>The connection string used for an Event Hub client must specify the Event Hubs namespace host, and a Shared Access Key (both the name and value) to be valid. The path to an Event Hub must be included in the connection string or specified separately.</value>
155155
</data>
156156
<data name="OnlyOneEventHubNameMayBeSpecified" xml:space="preserve">
157-
<value>The path to an Event Hub may be specified as part of the connection string or as a separate value, but not both.</value>
157+
<value>The path to an Event Hub may be specified as part of the connection string or as a separate value, but not both. Please verify that your connection string does not have the `EntityPath` token if you are passing an explicit Event Hub name.</value>
158158
</data>
159159
<data name="UnknownConnectionType" xml:space="preserve">
160160
<value>The specified connection type, "{0}", is not recognized as valid in this context.</value>
@@ -288,4 +288,4 @@
288288
<data name="AggregateEventProcessingExceptionMessage" xml:space="preserve">
289289
<value>One or more exceptions occured during event processing. Please see the inner exceptions for more detail.</value>
290290
</data>
291-
</root>
291+
</root>

‎sdk/eventhub/Azure.Messaging.EventHubs.Shared/tests/Core/ConnectionStringParserTests.cs

100755100644
File mode changed.

‎sdk/eventhub/Azure.Messaging.EventHubs.Shared/tests/Core/ConnectionStringPropertiesTests.cs

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Azure.Messaging.EventHubs.Core;
5+
using NUnit.Framework;
6+
7+
namespace Azure.Messaging.EventHubs.Tests
8+
{
9+
/// <summary>
10+
/// The suite of tests for the <see cref="ConnectionStringProperties" />
11+
/// class.
12+
/// </summary>
13+
///
14+
[TestFixture]
15+
public class ConnectionStringPropertiesTests
16+
{
17+
/// <summary>
18+
/// Verifies functionality of the <see cref="ConnectionStringProperties.Validate" />
19+
/// method.
20+
/// </summary>
21+
///
22+
[Test]
23+
[TestCase("SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
24+
[TestCase("Endpoint=value.com;SharedAccessKey=[value];EntityPath=[value]")]
25+
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];EntityPath=[value]")]
26+
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
27+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
28+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
29+
public void ValidateDetectsAnInvalidConnectionString(string connectionString)
30+
{
31+
var properties = ConnectionStringParser.Parse(connectionString);
32+
Assert.That(() => properties.Validate(null, "Dummy"), Throws.ArgumentException.And.Message.StartsWith(Resources.MissingConnectionInformation));
33+
}
34+
35+
/// <summary>
36+
/// Verifies functionality of the <see cref="ConnectionStringProperties.Validate" />
37+
/// method.
38+
/// </summary>
39+
///
40+
[Test]
41+
public void ValidateDetectsMultipleEventNames()
42+
{
43+
var eventHubName = "myHub";
44+
var fakeConnection = $"Endpoint=sb://not-real.servicebus.windows.net/;SharedAccessKeyName=DummyKey;SharedAccessKey=[not_real];EntityPath=[unique_fake]";
45+
var properties = ConnectionStringParser.Parse(fakeConnection);
46+
47+
Assert.That(() => properties.Validate(eventHubName, "Dummy"), Throws.ArgumentException.And.Message.StartsWith(Resources.OnlyOneEventHubNameMayBeSpecified));
48+
}
49+
50+
/// <summary>
51+
/// Verifies functionality of the <see cref="ConnectionStringProperties.Validate" />
52+
/// method.
53+
/// </summary>
54+
///
55+
[Test]
56+
public void ValidateAllowsMultipleEventHubNamesIfEqual()
57+
{
58+
var eventHubName = "myHub";
59+
var fakeConnection = $"Endpoint=sb://not-real.servicebus.windows.net/;SharedAccessKeyName=DummyKey;SharedAccessKey=[not_real];EntityPath={ eventHubName }";
60+
var properties = ConnectionStringParser.Parse(fakeConnection);
61+
62+
Assert.That(() => properties.Validate(eventHubName, "dummy"), Throws.Nothing, "Validation should accept the same Event Hub in multiple places");
63+
}
64+
}
65+
}

‎sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubConnection.cs

100755100644
+2-42
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ public EventHubConnection(string connectionString,
149149
connectionOptions = connectionOptions?.Clone() ?? new EventHubConnectionOptions();
150150
ValidateConnectionOptions(connectionOptions);
151151

152-
ConnectionStringProperties connectionStringProperties = ConnectionStringParser.Parse(connectionString);
153-
ValidateConnectionProperties(connectionStringProperties, eventHubName, nameof(connectionString));
152+
var connectionStringProperties = ConnectionStringParser.Parse(connectionString);
153+
connectionStringProperties.Validate(eventHubName, nameof(connectionString));
154154

155155
var fullyQualifiedNamespace = connectionStringProperties.Endpoint.Host;
156156

@@ -470,46 +470,6 @@ private static string BuildAudienceResource(EventHubsTransportType transportType
470470
return builder.Uri.AbsoluteUri.ToLowerInvariant();
471471
}
472472

473-
/// <summary>
474-
/// Performs the actions needed to validate the set of properties for connecting to the
475-
/// Event Hubs service, as passed to this client during creation.
476-
/// </summary>
477-
///
478-
/// <param name="properties">The set of properties parsed from the connection string associated this client.</param>
479-
/// <param name="eventHubName">The name of the Event Hub passed independent of the connection string, allowing easier use of a namespace-level connection string.</param>
480-
/// <param name="connectionStringArgumentName">The name of the argument associated with the connection string; to be used when raising <see cref="ArgumentException" /> variants.</param>
481-
///
482-
/// <remarks>
483-
/// In the case that the properties violate an invariant or otherwise represent a combination that
484-
/// is not permissible, an appropriate exception will be thrown.
485-
/// </remarks>
486-
///
487-
private static void ValidateConnectionProperties(ConnectionStringProperties properties,
488-
string eventHubName,
489-
string connectionStringArgumentName)
490-
{
491-
// The Event Hub name may only be specified in one of the possible forms, either as part of the
492-
// connection string or as a stand-alone parameter, but not both. If specified in both to the same
493-
// value, then do not consider this a failure.
494-
495-
if ((!string.IsNullOrEmpty(eventHubName))
496-
&& (!string.IsNullOrEmpty(properties.EventHubName))
497-
&& (!string.Equals(eventHubName, properties.EventHubName, StringComparison.InvariantCultureIgnoreCase)))
498-
{
499-
throw new ArgumentException(Resources.OnlyOneEventHubNameMayBeSpecified, connectionStringArgumentName);
500-
}
501-
502-
// Ensure that each of the needed components are present for connecting.
503-
504-
if ((string.IsNullOrEmpty(eventHubName)) && (string.IsNullOrEmpty(properties.EventHubName))
505-
|| (string.IsNullOrEmpty(properties.Endpoint?.Host))
506-
|| (string.IsNullOrEmpty(properties.SharedAccessKeyName))
507-
|| (string.IsNullOrEmpty(properties.SharedAccessKey)))
508-
{
509-
throw new ArgumentException(Resources.MissingConnectionInformation, connectionStringArgumentName);
510-
}
511-
}
512-
513473
/// <summary>
514474
/// Performs the actions needed to validate the <see cref="EventHubConnectionOptions" /> associated
515475
/// with this client.

‎sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/EventProcessor{TPartition}.cs

+1
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ protected EventProcessor(int eventBatchMaximumCount,
300300
options = options?.Clone() ?? new EventProcessorOptions();
301301

302302
var connectionStringProperties = ConnectionStringParser.Parse(connectionString);
303+
connectionStringProperties.Validate(eventHubName, nameof(connectionString));
303304

304305
ConnectionFactory = () => new EventHubConnection(connectionString, eventHubName, options.ConnectionOptions);
305306
FullyQualifiedNamespace = connectionStringProperties.Endpoint.Host;

‎sdk/eventhub/Azure.Messaging.EventHubs/tests/Connection/EventHubConnectionTests.cs

100755100644
+3-1
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,11 @@ public void ConstructorDoesNotRequireEventHubInConnectionStringWhenPassedSeparat
136136
[TestCase("Endpoint=value.com;SharedAccessKey=[value];EntityPath=[value]")]
137137
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];EntityPath=[value]")]
138138
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
139+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
140+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
139141
public void ConstructorValidatesConnectionString(string connectionString)
140142
{
141-
Assert.That(() => new EventHubConnection(connectionString), Throws.ArgumentException);
143+
Assert.That(() => new EventHubConnection(connectionString), Throws.ArgumentException.And.Message.StartsWith(Resources.MissingConnectionInformation));
142144
}
143145

144146
/// <summary>

‎sdk/eventhub/Azure.Messaging.EventHubs/tests/Consumer/EventHubConsumerClientTests.cs

+46-1
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,57 @@ public void ConstructorValidatesTheConsumerGroup(string consumerGroup)
7474
[Test]
7575
[TestCase(null)]
7676
[TestCase("")]
77-
public void ConstructorValidatesTheConnectionString(string connectionString)
77+
public void ConstructorValidatesTheConnectionStringIsPopulated(string connectionString)
7878
{
7979
Assert.That(() => new EventHubConsumerClient(EventHubConsumerClient.DefaultConsumerGroupName, connectionString), Throws.InstanceOf<ArgumentException>(), "The constructor without options should ensure a connection string.");
8080
Assert.That(() => new EventHubConsumerClient(EventHubConsumerClient.DefaultConsumerGroupName, connectionString, "dummy", new EventHubConsumerClientOptions()), Throws.InstanceOf<ArgumentException>(), "The constructor with options should ensure a connection string.");
8181
}
8282

83+
/// <summary>
84+
/// Verifies functionality of the <see cref="EventHubConnection" />
85+
/// constructor.
86+
/// </summary>
87+
///
88+
[Test]
89+
[TestCase("SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
90+
[TestCase("Endpoint=value.com;SharedAccessKey=[value];EntityPath=[value]")]
91+
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];EntityPath=[value]")]
92+
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
93+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
94+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
95+
public void ConstructorValidatesConnectionString(string connectionString)
96+
{
97+
Assert.That(() => new EventHubConsumerClient(EventHubConsumerClient.DefaultConsumerGroupName, connectionString), Throws.ArgumentException.And.Message.StartsWith(Resources.MissingConnectionInformation));
98+
}
99+
100+
/// <summary>
101+
/// Verifies functionality of the <see cref="EventHubConnection" />
102+
/// constructor.
103+
/// </summary>
104+
///
105+
[Test]
106+
public void ConstructorDetectsMultipleEventHubNamesFromTheConnectionString()
107+
{
108+
var eventHubName = "myHub";
109+
var connectionString = $"Endpoint=sb://not-real.servicebus.windows.net/;SharedAccessKeyName=DummyKey;SharedAccessKey=[not_real];EntityPath=[unique_fake]";
110+
111+
Assert.That(() => new EventHubConsumerClient(EventHubConsumerClient.DefaultConsumerGroupName, connectionString, eventHubName), Throws.ArgumentException.And.Message.StartsWith(Resources.OnlyOneEventHubNameMayBeSpecified));
112+
}
113+
114+
/// <summary>
115+
/// Verifies functionality of the <see cref="EventHubConnection" />
116+
/// constructor.
117+
/// </summary>
118+
///
119+
[Test]
120+
public void ConstructorAllowsMultipleEventHubNamesFromTheConnectionStringIfEqual()
121+
{
122+
var eventHubName = "myHub";
123+
var connectionString = $"Endpoint=sb://not-real.servicebus.windows.net/;SharedAccessKeyName=DummyKey;SharedAccessKey=[not_real];EntityPath={ eventHubName }";
124+
125+
Assert.That(() => new EventHubConsumerClient(EventHubConsumerClient.DefaultConsumerGroupName, connectionString, eventHubName), Throws.Nothing);
126+
}
127+
83128
/// <summary>
84129
/// Verifies functionality of the constructor.
85130
/// </summary>

‎sdk/eventhub/Azure.Messaging.EventHubs/tests/Primitives/EventProcessorTests.Constructor.cs

100755100644
+46-1
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,57 @@ public void ConstructorValidatesTheConsumerGroup(string constructorArgument)
7676
[Test]
7777
[TestCase(null)]
7878
[TestCase("")]
79-
public void ConstructorValidatesTheConnectionString(string connectionString)
79+
public void ConstructorValidatesTheConnectionStringIsPopulated(string connectionString)
8080
{
8181
Assert.That(() => new MinimalProcessorMock(1, EventHubConsumerClient.DefaultConsumerGroupName, connectionString), Throws.InstanceOf<ArgumentException>(), "The constructor without options should ensure a connection string.");
8282
Assert.That(() => new MinimalProcessorMock(1, EventHubConsumerClient.DefaultConsumerGroupName, connectionString, "dummy", new EventProcessorOptions()), Throws.InstanceOf<ArgumentException>(), "The constructor with options should ensure a connection string.");
8383
}
8484

85+
/// <summary>
86+
/// Verifies functionality of the <see cref="EventHubConnection" />
87+
/// constructor.
88+
/// </summary>
89+
///
90+
[Test]
91+
[TestCase("SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
92+
[TestCase("Endpoint=value.com;SharedAccessKey=[value];EntityPath=[value]")]
93+
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];EntityPath=[value]")]
94+
[TestCase("Endpoint=value.com;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
95+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value]")]
96+
[TestCase("HostName=value.azure-devices.net;SharedAccessKeyName=[value];SharedAccessKey=[value];EntityPath=[value]")]
97+
public void ConstructorValidatesConnectionString(string connectionString)
98+
{
99+
Assert.That(() => new MinimalProcessorMock(1, EventHubConsumerClient.DefaultConsumerGroupName, connectionString), Throws.ArgumentException.And.Message.StartsWith(Resources.MissingConnectionInformation));
100+
}
101+
102+
/// <summary>
103+
/// Verifies functionality of the <see cref="EventHubConnection" />
104+
/// constructor.
105+
/// </summary>
106+
///
107+
[Test]
108+
public void ConstructorDetectsMultipleEventHubNamesFromTheConnectionString()
109+
{
110+
var eventHubName = "myHub";
111+
var connectionString = $"Endpoint=sb://not-real.servicebus.windows.net/;SharedAccessKeyName=DummyKey;SharedAccessKey=[not_real];EntityPath=[unique_fake]";
112+
113+
Assert.That(() => new MinimalProcessorMock(1, EventHubConsumerClient.DefaultConsumerGroupName, connectionString, eventHubName), Throws.ArgumentException.And.Message.StartsWith(Resources.OnlyOneEventHubNameMayBeSpecified));
114+
}
115+
116+
/// <summary>
117+
/// Verifies functionality of the <see cref="EventHubConnection" />
118+
/// constructor.
119+
/// </summary>
120+
///
121+
[Test]
122+
public void ConstructorAllowsMultipleEventHubNamesFromTheConnectionStringIfEqual()
123+
{
124+
var eventHubName = "myHub";
125+
var connectionString = $"Endpoint=sb://not-real.servicebus.windows.net/;SharedAccessKeyName=DummyKey;SharedAccessKey=[not_real];EntityPath={ eventHubName }";
126+
127+
Assert.That(() => new MinimalProcessorMock(1, EventHubConsumerClient.DefaultConsumerGroupName, connectionString, eventHubName), Throws.Nothing);
128+
}
129+
85130
/// <summary>
86131
/// Verifies functionality of the <see cref="EventProcessor{TPartition}" />
87132
/// constructor.

0 commit comments

Comments
 (0)