Skip to content

Commit 1f37b45

Browse files
bart-vmwarebart-vmware
committed
Bugfix: preserve custom query string parameters in local RabbitMQ URI; relax other parsers to allow unknown parameters to support third-party brokers
1 parent 467c823 commit 1f37b45

8 files changed

Lines changed: 81 additions & 89 deletions

‎src/Connectors/src/Connectors/MongoDb/MongoDbConnectionStringBuilder.cs‎

Lines changed: 15 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal sealed class MongoDbConnectionStringBuilder : IConnectionStringBuilder
2222
public string ConnectionString
2323
{
2424
get => ToConnectionString();
25-
set => FromConnectionString(value, false);
25+
set => FromConnectionString(value);
2626
}
2727

2828
/// <inheritdoc />
@@ -37,22 +37,16 @@ public object? this[string keyword]
3737
return ConnectionString;
3838
}
3939

40-
// Allow getting unknown keyword, if it was set earlier. We don't pretend to know all valid query string parameters.
41-
if (_settings.TryGetValue(keyword, out string? value))
42-
{
43-
return value;
44-
}
45-
46-
AssertIsKnownKeyword(keyword);
47-
return null;
40+
// Allow getting unknown keyword. We don't pretend to know all valid query string parameters.
41+
return _settings.GetValueOrDefault(keyword);
4842
}
4943
set
5044
{
5145
ArgumentException.ThrowIfNullOrWhiteSpace(keyword);
5246

5347
if (string.Equals(keyword, KnownKeywords.Url, StringComparison.OrdinalIgnoreCase))
5448
{
55-
FromConnectionString(value?.ToString(), true);
49+
ConnectionString = value?.ToString();
5650
}
5751
else
5852
{
@@ -106,29 +100,19 @@ private string ToConnectionString()
106100

107101
var queryString = default(QueryString);
108102

109-
foreach ((string keyword, string value) in _settings.Where(pair => !KnownKeywords.Exists(pair.Key)))
103+
foreach ((string name, string value) in _settings.Where(pair => !KnownKeywords.Exists(pair.Key)))
110104
{
111-
queryString = queryString.Add(keyword, value);
105+
queryString = queryString.Add(name, value);
112106
}
113107

114108
builder.Query = queryString.Value;
115109

116110
return builder.Uri.AbsoluteUri;
117111
}
118112

119-
private void FromConnectionString(string? connectionString, bool preserveUnknownSettings)
113+
private void FromConnectionString(string? connectionString)
120114
{
121-
if (preserveUnknownSettings)
122-
{
123-
foreach (string keywordToRemove in _settings.Keys.Where(KnownKeywords.Exists).ToArray())
124-
{
125-
_settings.Remove(keywordToRemove);
126-
}
127-
}
128-
else
129-
{
130-
_settings.Clear();
131-
}
115+
_settings.Clear();
132116

133117
if (!string.IsNullOrEmpty(connectionString))
134118
{
@@ -140,7 +124,7 @@ private void FromConnectionString(string? connectionString, bool preserveUnknown
140124
#pragma warning restore S3717 // Track use of "NotImplementedException"
141125
}
142126

143-
// MongoDB allows semicolon as separator for query string parameters, to provide backwards compatibility.
127+
// MongoDB allows semicolon as separator between query string parameters for backward compatibility.
144128
connectionString = connectionString.Replace(';', '&');
145129

146130
var uri = new Uri(connectionString);
@@ -169,31 +153,20 @@ private void FromConnectionString(string? connectionString, bool preserveUnknown
169153
_settings[KnownKeywords.AuthenticationDatabase] = Uri.UnescapeDataString(uri.AbsolutePath[1..]);
170154
}
171155

172-
NameValueCollection queryCollection = HttpUtility.ParseQueryString(uri.Query);
156+
NameValueCollection queryString = HttpUtility.ParseQueryString(uri.Query);
173157

174-
foreach (string? key in queryCollection.AllKeys)
158+
foreach (string remainingKeyword in queryString.AllKeys.Where(key => key != null && !KnownKeywords.Exists(key)).Cast<string>())
175159
{
176-
if (key != null)
177-
{
178-
string? value = queryCollection.Get(key);
160+
string? value = queryString.Get(remainingKeyword);
179161

180-
if (value != null)
181-
{
182-
_settings[key] = value;
183-
}
162+
if (value != null)
163+
{
164+
_settings[remainingKeyword] = value;
184165
}
185166
}
186167
}
187168
}
188169

189-
private static void AssertIsKnownKeyword(string keyword)
190-
{
191-
if (!KnownKeywords.Exists(keyword))
192-
{
193-
throw new ArgumentException($"Keyword not supported: '{keyword}'.", nameof(keyword));
194-
}
195-
}
196-
197170
private static class KnownKeywords
198171
{
199172
public const string Url = "url";

‎src/Connectors/src/Connectors/RabbitMQ/RabbitMQConnectionStringBuilder.cs‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
// The .NET Foundation licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Collections.Specialized;
56
using System.Diagnostics.CodeAnalysis;
67
using System.Globalization;
8+
using System.Web;
9+
using Microsoft.AspNetCore.Http;
710

811
namespace Steeltoe.Connectors.RabbitMQ;
912

@@ -28,19 +31,18 @@ public object? this[string keyword]
2831
get
2932
{
3033
ArgumentException.ThrowIfNullOrWhiteSpace(keyword);
31-
AssertIsKnownKeyword(keyword);
3234

3335
if (string.Equals(keyword, KnownKeywords.Url, StringComparison.OrdinalIgnoreCase))
3436
{
3537
return ConnectionString;
3638
}
3739

40+
// Allow getting unknown keyword. We don't pretend to know all valid query string parameters.
3841
return _settings.GetValueOrDefault(keyword);
3942
}
4043
set
4144
{
4245
ArgumentException.ThrowIfNullOrWhiteSpace(keyword);
43-
AssertIsKnownKeyword(keyword);
4446

4547
if (string.Equals(keyword, KnownKeywords.Url, StringComparison.OrdinalIgnoreCase))
4648
{
@@ -56,6 +58,7 @@ public object? this[string keyword]
5658
}
5759
else
5860
{
61+
// Allow setting unknown keyword. We don't pretend to know all valid query string parameters.
5962
_settings[keyword] = stringValue;
6063
}
6164
}
@@ -97,6 +100,15 @@ private string ToConnectionString()
97100
builder.Path = Uri.EscapeDataString(virtualHost);
98101
}
99102

103+
var queryString = default(QueryString);
104+
105+
foreach ((string name, string value) in _settings.Where(pair => !KnownKeywords.Exists(pair.Key)))
106+
{
107+
queryString = queryString.Add(name, value);
108+
}
109+
110+
builder.Query = queryString.Value;
111+
100112
return builder.Uri.AbsoluteUri;
101113
}
102114

@@ -132,14 +144,18 @@ private void FromConnectionString(string? connectionString)
132144
{
133145
_settings[KnownKeywords.VirtualHost] = Uri.UnescapeDataString(uri.AbsolutePath[1..]);
134146
}
135-
}
136-
}
137147

138-
private static void AssertIsKnownKeyword(string keyword)
139-
{
140-
if (!KnownKeywords.Exists(keyword))
141-
{
142-
throw new ArgumentException($"Keyword not supported: '{keyword}'.", nameof(keyword));
148+
NameValueCollection queryString = HttpUtility.ParseQueryString(uri.Query);
149+
150+
foreach (string remainingKeyword in queryString.AllKeys.Where(key => key != null && !KnownKeywords.Exists(key)).Cast<string>())
151+
{
152+
string? value = queryString.Get(remainingKeyword);
153+
154+
if (value != null)
155+
{
156+
_settings[remainingKeyword] = value;
157+
}
158+
}
143159
}
144160
}
145161

‎src/Connectors/src/Connectors/Redis/RedisConnectionStringBuilder.cs‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,8 @@ public object? this[string keyword]
2929
{
3030
ArgumentException.ThrowIfNullOrWhiteSpace(keyword);
3131

32-
// Allow getting unknown keyword, if it was set earlier. We don't pretend to know all valid keywords.
33-
if (_settings.TryGetValue(keyword, out string? value))
34-
{
35-
return value;
36-
}
37-
38-
AssertIsKnownKeyword(keyword);
39-
return null;
32+
// Allow getting unknown keyword. We don't pretend to know all valid query string parameters.
33+
return _settings.GetValueOrDefault(keyword);
4034
}
4135
set
4236
{
@@ -50,7 +44,7 @@ public object? this[string keyword]
5044
}
5145
else
5246
{
53-
// Allow setting unknown keyword. We don't pretend to know all valid keywords.
47+
// Allow setting unknown keyword. We don't pretend to know all valid query string parameters.
5448
_settings[keyword] = stringValue;
5549
}
5650
}
@@ -120,14 +114,6 @@ private void FromConnectionString(string? connectionString)
120114
}
121115
}
122116

123-
private static void AssertIsKnownKeyword(string keyword)
124-
{
125-
if (!KnownKeywords.Exists(keyword))
126-
{
127-
throw new ArgumentException($"Keyword not supported: '{keyword}'.", nameof(keyword));
128-
}
129-
}
130-
131117
private static class KnownKeywords
132118
{
133119
public const string Host = "host";

‎src/Connectors/test/Connectors.Test/MongoDb/MongoDbConnectionStringBuilderTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@ public void Returns_null_when_getting_known_keyword()
8282
}
8383

8484
[Fact]
85-
public void Throws_when_getting_unknown_keyword()
85+
public void Returns_null_when_getting_unknown_keyword()
8686
{
8787
var builder = new MongoDbConnectionStringBuilder();
8888

89-
Action action = () => _ = builder["bad"];
89+
object? some = builder["some"];
9090

91-
action.Should().ThrowExactly<ArgumentException>().WithMessage("Keyword not supported: 'bad'.*");
91+
some.Should().BeNull();
9292
}
9393

9494
[Fact]

‎src/Connectors/test/Connectors.Test/MongoDb/MongoDbConnectorTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public async Task Binds_options_with_CloudFoundry_service_bindings()
158158
MongoDbOptions optionsOne = optionsMonitor.Get("myMongoDbServiceOne");
159159

160160
optionsOne.ConnectionString.Should().Be(
161-
"mongodb://csb0230eada-2354-4c73-b3e4-8a1aaa996894:AiNtEyASbdXR5neJmTStMzKGItX2xvKuyEkcy65rviKD0ggZR19E1iVFIJ5ZAIY1xvvAiS5tOXsmACDbKDJIhQ%3D%3D@csb0230eada-2354-4c73-b3e4-8a1aaa996894.mongo.cosmos.cloud-hostname.com:10255/csb-db0230eada-2354-4c73-b3e4-8a1aaa996894?connectTimeoutMS=5000&ssl=true&replicaSet=globaldb&retrywrites=false&maxIdleTimeMS=120000&appName=@csb0230eada-2354-4c73-b3e4-8a1aaa996894@");
161+
"mongodb://csb0230eada-2354-4c73-b3e4-8a1aaa996894:AiNtEyASbdXR5neJmTStMzKGItX2xvKuyEkcy65rviKD0ggZR19E1iVFIJ5ZAIY1xvvAiS5tOXsmACDbKDJIhQ%3D%3D@csb0230eada-2354-4c73-b3e4-8a1aaa996894.mongo.cosmos.cloud-hostname.com:10255/csb-db0230eada-2354-4c73-b3e4-8a1aaa996894?ssl=true&replicaSet=globaldb&retrywrites=false&maxIdleTimeMS=120000&appName=@csb0230eada-2354-4c73-b3e4-8a1aaa996894@");
162162

163163
optionsOne.Database.Should().Be("csb-db0230eada-2354-4c73-b3e4-8a1aaa996894");
164164

‎src/Connectors/test/Connectors.Test/RabbitMQ/RabbitMQConnectionStringBuilderTest.cs‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,21 @@ public void Decodes_properties_with_special_characters()
4141
builder["virtualHost"].Should().Be("my virtual= host");
4242
}
4343

44+
[Fact]
45+
public void Preserves_query_string_parameters_when_setting_URL()
46+
{
47+
var builder = new RabbitMQConnectionStringBuilder
48+
{
49+
ConnectionString = "amqps://localhost:999/virtual-host-1?first=one&second=two"
50+
};
51+
52+
const string url = "amqps://localhost:999/virtual-host-1?first=one&second=number2";
53+
builder["url"] = url;
54+
55+
builder.ConnectionString.Should().Be("amqps://localhost:999/virtual-host-1?first=one&second=number2");
56+
builder["url"].Should().Be(builder.ConnectionString);
57+
}
58+
4459
[Fact]
4560
public void Returns_null_when_getting_known_keyword()
4661
{
@@ -52,22 +67,24 @@ public void Returns_null_when_getting_known_keyword()
5267
}
5368

5469
[Fact]
55-
public void Throws_when_getting_unknown_keyword()
70+
public void Returns_null_when_getting_unknown_keyword()
5671
{
5772
var builder = new RabbitMQConnectionStringBuilder();
5873

59-
Action action = () => _ = builder["bad"];
74+
object? some = builder["some"];
6075

61-
action.Should().ThrowExactly<ArgumentException>().WithMessage("Keyword not supported: 'bad'.*");
76+
some.Should().BeNull();
6277
}
6378

6479
[Fact]
65-
public void Throws_when_setting_unknown_keyword()
80+
public void Can_get_unknown_keyword_that_was_set_earlier()
6681
{
67-
var builder = new RabbitMQConnectionStringBuilder();
68-
69-
Action action = () => builder["bad"] = "some";
82+
var builder = new RabbitMQConnectionStringBuilder
83+
{
84+
["some"] = "other"
85+
};
7086

71-
action.Should().ThrowExactly<ArgumentException>().WithMessage("Keyword not supported: 'bad'.*");
87+
object? value = builder["some"];
88+
value.Should().Be("other");
7289
}
7390
}

‎src/Connectors/test/Connectors.Test/RabbitMQ/RabbitMQConnectorTest.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ public async Task Binds_options_without_service_bindings()
190190
{
191191
var appSettings = new Dictionary<string, string?>
192192
{
193-
["Steeltoe:Client:RabbitMQ:myRabbitMQServiceOne:ConnectionString"] = "amqp://user1:pass1@host1:5672/virtual-host-1",
194-
["Steeltoe:Client:RabbitMQ:myRabbitMQServiceTwo:ConnectionString"] = "amqps://user2:pass2@host2:5672/virtual-host-2"
193+
["Steeltoe:Client:RabbitMQ:myRabbitMQServiceOne:ConnectionString"] = "amqp://user1:pass1@host1:5672/virtual-host-1?heartbeat=5",
194+
["Steeltoe:Client:RabbitMQ:myRabbitMQServiceTwo:ConnectionString"] = "amqps://user2:pass2@host2:5672/virtual-host-2?connection_timeout=5000"
195195
};
196196

197197
WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create();
@@ -203,18 +203,18 @@ public async Task Binds_options_without_service_bindings()
203203
var optionsSnapshot = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<RabbitMQOptions>>();
204204

205205
RabbitMQOptions optionsOne = optionsSnapshot.Get("myRabbitMQServiceOne");
206-
optionsOne.ConnectionString.Should().Be("amqp://user1:pass1@host1:5672/virtual-host-1");
206+
optionsOne.ConnectionString.Should().Be("amqp://user1:pass1@host1:5672/virtual-host-1?heartbeat=5");
207207

208208
RabbitMQOptions optionsTwo = optionsSnapshot.Get("myRabbitMQServiceTwo");
209-
optionsTwo.ConnectionString.Should().Be("amqps://user2:pass2@host2:5672/virtual-host-2");
209+
optionsTwo.ConnectionString.Should().Be("amqps://user2:pass2@host2:5672/virtual-host-2?connection_timeout=5000");
210210
}
211211

212212
[Fact]
213213
public async Task Binds_options_with_CloudFoundry_service_bindings()
214214
{
215215
var appSettings = new Dictionary<string, string?>
216216
{
217-
["Steeltoe:Client:RabbitMQ:myRabbitMQServiceOne:ConnectionString"] = "amqps://user:pass@localhost:5672"
217+
["Steeltoe:Client:RabbitMQ:myRabbitMQServiceOne:ConnectionString"] = "amqps://user:pass@localhost:5672?connection_timeout=5000&heartbeat=5"
218218
};
219219

220220
WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create();
@@ -228,7 +228,7 @@ public async Task Binds_options_with_CloudFoundry_service_bindings()
228228
RabbitMQOptions optionsOne = optionsMonitor.Get("myRabbitMQServiceOne");
229229

230230
optionsOne.ConnectionString.Should().Be(
231-
"amqp://d2fd2c9d-ef84-406b-8401-f2ffacaafda6:AqntL6IwehKOGssE51psrJYd@q-s0.rabbitmq-server.benicia-services-subnet.service-instance-377d9d72-e951-4a1c-82e8-99c3c4933368.bosh:5672/377d9d72-e951-4a1c-82e8-99c3c4933368");
231+
"amqp://d2fd2c9d-ef84-406b-8401-f2ffacaafda6:AqntL6IwehKOGssE51psrJYd@q-s0.rabbitmq-server.benicia-services-subnet.service-instance-377d9d72-e951-4a1c-82e8-99c3c4933368.bosh:5672/377d9d72-e951-4a1c-82e8-99c3c4933368?connection_timeout=5000&heartbeat=5");
232232

233233
RabbitMQOptions optionsTwo = optionsMonitor.Get("myRabbitMQServiceTwo");
234234

‎src/Connectors/test/Connectors.Test/Redis/RedisConnectionStringBuilderTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ public void Returns_null_when_getting_known_keyword()
4646
}
4747

4848
[Fact]
49-
public void Throws_when_getting_unknown_keyword()
49+
public void Returns_null_when_getting_unknown_keyword()
5050
{
5151
var builder = new RedisConnectionStringBuilder();
5252

53-
Action action = () => _ = builder["bad"];
53+
object? some = builder["some"];
5454

55-
action.Should().ThrowExactly<ArgumentException>().WithMessage("Keyword not supported: 'bad'.*");
55+
some.Should().BeNull();
5656
}
5757

5858
[Fact]

0 commit comments

Comments
 (0)