Skip to content

Commit 183ae8d

Browse files
niemyjskiniemyjski
committed
Stabilize provider parsing and request handling
Root cause: several provider code paths still relied on culture-sensitive protocol normalization and ambiguous deserialization or disposal patterns, which left review threads open and behavior dependent on locale/runtime details.
1 parent 31e932a commit 183ae8d

9 files changed

Lines changed: 163 additions & 18 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ samples/
6868
- Use modern C# features where the target frameworks support them
6969
- Follow SOLID, DRY principles; remove unused code and parameters
7070
- Clear, descriptive naming; prefer explicit over clever
71+
- Use ordinal or invariant string operations for protocol-level values such as HTTP verbs, OAuth parameter sorting, provider identifiers, and other locale-independent tokens
7172
- For existing public value-like types, prefer additive equality fixes over record conversions unless an API shape change is explicitly intended
7273
- Handle cancellation tokens properly: pass through call chains
7374
- Always dispose resources: use `using` statements

‎src/Geocoding.Google/GoogleGeocoder.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ private async Task<IEnumerable<GoogleAddress>> ProcessRequest(HttpRequestMessage
199199
{
200200
try
201201
{
202-
using var requestToDispose = request;
203202
using var client = BuildClient();
204203
using var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false);
205204

@@ -215,6 +214,10 @@ private async Task<IEnumerable<GoogleAddress>> ProcessRequest(HttpRequestMessage
215214
{
216215
throw new GoogleGeocodingException(ex);
217216
}
217+
finally
218+
{
219+
request.Dispose();
220+
}
218221
}
219222

220223
/// <summary>

‎src/Geocoding.MapQuest/BaseRequest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public virtual Uri RequestUri
124124
public virtual string RequestVerb
125125
{
126126
get { return _verb; }
127-
protected set { _verb = String.IsNullOrWhiteSpace(value) ? "POST" : value.Trim().ToUpper(); }
127+
protected set { _verb = String.IsNullOrWhiteSpace(value) ? "POST" : value.Trim().ToUpperInvariant(); }
128128
}
129129

130130
/// <summary>

‎src/Geocoding.MapQuest/MapQuestGeocoder.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ where l.Quality < Quality.COUNTRY
151151
if (!String.IsNullOrWhiteSpace(l.FormattedAddress) || o.ProvidedLocation is null)
152152
continue;
153153

154-
if (string.Compare(o.ProvidedLocation.FormattedAddress, "unknown", true) != 0)
154+
if (!String.Equals(o.ProvidedLocation.FormattedAddress, "unknown", StringComparison.OrdinalIgnoreCase))
155155
l.FormattedAddress = o.ProvidedLocation.FormattedAddress;
156156
else
157157
l.FormattedAddress = o.ProvidedLocation.ToString();

‎src/Geocoding.MapQuest/MapQuestLocation.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ public class MapQuestLocation : ParsedAddress
1616
/// Initializes a new instance of the <see cref="MapQuestLocation"/> class for deserialization.
1717
/// </summary>
1818
[JsonConstructor]
19-
protected MapQuestLocation() { }
19+
public MapQuestLocation()
20+
: this(Unknown, new Location(0, 0)) { }
2021

2122
/// <summary>
2223
/// Initializes a new instance of the <see cref="MapQuestLocation"/> class.

‎src/Geocoding.Yahoo/OAuthBase.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ public int Compare(QueryParameter x, QueryParameter y)
7979
{
8080
if (x.Name == y.Name)
8181
{
82-
return String.Compare(x.Value, y.Value);
82+
return StringComparer.Ordinal.Compare(x.Value, y.Value);
8383
}
8484
else
8585
{
86-
return String.Compare(x.Name, y.Name);
86+
return StringComparer.Ordinal.Compare(x.Name, y.Name);
8787
}
8888
}
8989

@@ -338,7 +338,7 @@ public string GenerateSignatureBase(Uri url, string consumerKey, string token, s
338338
normalizedRequestParameters = NormalizeRequestParameters(parameters);
339339

340340
StringBuilder signatureBase = new StringBuilder();
341-
signatureBase.AppendFormat("{0}&", httpMethod.ToUpper());
341+
signatureBase.AppendFormat("{0}&", httpMethod.ToUpperInvariant());
342342
signatureBase.AppendFormat("{0}&", UrlEncode(normalizedUrl));
343343
signatureBase.AppendFormat("{0}", UrlEncode(normalizedRequestParameters));
344344

@@ -400,13 +400,15 @@ public string GenerateSignature(Uri url, string consumerKey, string consumerSecr
400400
case SignatureTypes.PLAINTEXT:
401401
return WebUtility.UrlEncode($"{consumerSecret}&{tokenSecret}")!;
402402
case SignatureTypes.HMACSHA1:
403+
{
403404
string signatureBase = GenerateSignatureBase(url, consumerKey, token, tokenSecret, httpMethod, timeStamp, nonce, HMACSHA1SignatureType, out normalizedUrl, out normalizedRequestParameters);
404405

405-
HMACSHA1 hmacsha1 = new HMACSHA1();
406+
using HMACSHA1 hmacsha1 = new HMACSHA1();
406407
hmacsha1.Key = Encoding.ASCII.GetBytes(
407408
$"{UrlEncode(consumerSecret)}&{(String.IsNullOrEmpty(tokenSecret) ? "" : UrlEncode(tokenSecret))}");
408409

409410
return GenerateSignatureUsingHash(signatureBase, hmacsha1);
411+
}
410412
case SignatureTypes.RSASHA1:
411413
throw new NotImplementedException();
412414
default:

‎src/Geocoding.Yahoo/YahooGeocoder.cs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ private async Task<IEnumerable<YahooAddress>> ProcessRequest(HttpRequestMessage
111111
{
112112
try
113113
{
114-
using var requestToDispose = request;
115114
using var client = BuildClient();
116115
using var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false);
117116

@@ -120,14 +119,7 @@ private async Task<IEnumerable<YahooAddress>> ProcessRequest(HttpRequestMessage
120119
var preview = await BuildResponsePreviewAsync(response.Content).ConfigureAwait(false);
121120
var message = $"Yahoo request failed ({(int)response.StatusCode} {response.ReasonPhrase}).{preview}";
122121

123-
try
124-
{
125-
response.EnsureSuccessStatusCode();
126-
}
127-
catch (HttpRequestException ex)
128-
{
129-
throw new YahooGeocodingException(message, ex);
130-
}
122+
throw new YahooGeocodingException(message, new HttpRequestException(message));
131123
}
132124

133125
using (var stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false))
@@ -146,6 +138,10 @@ private async Task<IEnumerable<YahooAddress>> ProcessRequest(HttpRequestMessage
146138
//wrap in yahoo exception
147139
throw new YahooGeocodingException(ex);
148140
}
141+
finally
142+
{
143+
request.Dispose();
144+
}
149145
}
150146

151147
async Task<IEnumerable<Address>> IGeocoder.GeocodeAsync(string address, CancellationToken cancellationToken)

‎test/Geocoding.Tests/MapQuestGeocoderTest.cs‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
using System.Net;
1+
using System.Globalization;
2+
using System.Net;
23
using System.Net.Http;
34
using System.Reflection;
5+
using System.Text.Json;
6+
using Geocoding.Extensions;
47
using Geocoding.MapQuest;
58
using Geocoding.Tests.Utility;
69
using Xunit;
@@ -46,6 +49,59 @@ public void UseOSM_SetTrue_ThrowsNotSupportedException()
4649
Assert.Contains("no longer supported", exception.Message, StringComparison.OrdinalIgnoreCase);
4750
}
4851

52+
[Fact]
53+
public void RequestVerb_Normalization_IsCultureInvariant()
54+
{
55+
// Arrange
56+
var originalCulture = CultureInfo.CurrentCulture;
57+
var originalUICulture = CultureInfo.CurrentUICulture;
58+
59+
try
60+
{
61+
CultureInfo.CurrentCulture = new CultureInfo("tr-TR");
62+
CultureInfo.CurrentUICulture = new CultureInfo("tr-TR");
63+
var request = new TestRequest("mapquest-key");
64+
65+
// Act
66+
request.SetVerb("mixid");
67+
68+
// Assert
69+
Assert.Equal("MIXID", request.RequestVerb);
70+
}
71+
finally
72+
{
73+
CultureInfo.CurrentCulture = originalCulture;
74+
CultureInfo.CurrentUICulture = originalUICulture;
75+
}
76+
}
77+
78+
[Fact]
79+
public void MapQuestLocation_Deserialization_PreservesProviderDefaults()
80+
{
81+
// Arrange
82+
const string json = """
83+
{
84+
"location": "1600 Pennsylvania Ave NW, Washington, DC 20500, US",
85+
"latLng": { "lat": 38.8977, "lng": -77.0365 },
86+
"displayLatLng": { "lat": 38.8977, "lng": -77.0365 },
87+
"street": "1600 Pennsylvania Ave NW",
88+
"adminArea5": "Washington",
89+
"adminArea3": "DC",
90+
"adminArea1": "US",
91+
"postalCode": "20500"
92+
}
93+
""";
94+
95+
// Act
96+
var location = JsonSerializer.Deserialize<MapQuestLocation>(json, JsonExtensions.JsonOptions);
97+
98+
// Assert
99+
Assert.NotNull(location);
100+
Assert.Equal("MapQuest", location.Provider);
101+
Assert.Equal("1600 Pennsylvania Ave NW, Washington, DC 20500, US", location.FormattedAddress);
102+
Assert.Equal(new Location(38.8977, -77.0365), location.Coordinates);
103+
}
104+
49105
[Fact]
50106
public async Task CreateRequest_GeocodeRequest_CreatesJsonPost()
51107
{
@@ -111,4 +167,17 @@ protected override HttpClient BuildClient()
111167
return new HttpClient(_handler, disposeHandler: false);
112168
}
113169
}
170+
171+
private sealed class TestRequest : BaseRequest
172+
{
173+
public TestRequest(string key)
174+
: base(key) { }
175+
176+
public override string RequestAction => "address";
177+
178+
public void SetVerb(string verb)
179+
{
180+
RequestVerb = verb;
181+
}
182+
}
114183
}

‎test/Geocoding.Tests/YahooGeocoderTest.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#pragma warning disable CS0618
2+
using System.Globalization;
23
using System.Net;
34
using System.Net.Http;
45
using System.Reflection;
@@ -133,6 +134,68 @@ public async Task Geocode_TransportFailure_WrapsTransportException()
133134
Assert.IsType<HttpRequestException>(exception.InnerException);
134135
}
135136

137+
[Fact]
138+
public void QueryParameterComparer_UsesOrdinalComparison()
139+
{
140+
// Arrange
141+
var originalCulture = CultureInfo.CurrentCulture;
142+
var originalUICulture = CultureInfo.CurrentUICulture;
143+
144+
try
145+
{
146+
CultureInfo.CurrentCulture = new CultureInfo("tr-TR");
147+
CultureInfo.CurrentUICulture = new CultureInfo("tr-TR");
148+
var helper = new TestOAuthBase();
149+
150+
// Act
151+
var comparison = helper.CompareParameters("I", "first", "ı", "second");
152+
153+
// Assert
154+
Assert.Equal(StringComparer.Ordinal.Compare("I", "ı"), comparison);
155+
}
156+
finally
157+
{
158+
CultureInfo.CurrentCulture = originalCulture;
159+
CultureInfo.CurrentUICulture = originalUICulture;
160+
}
161+
}
162+
163+
[Fact]
164+
public void GenerateSignatureBase_HttpMethodNormalization_IsCultureInvariant()
165+
{
166+
// Arrange
167+
var originalCulture = CultureInfo.CurrentCulture;
168+
var originalUICulture = CultureInfo.CurrentUICulture;
169+
170+
try
171+
{
172+
CultureInfo.CurrentCulture = new CultureInfo("tr-TR");
173+
CultureInfo.CurrentUICulture = new CultureInfo("tr-TR");
174+
var helper = new TestOAuthBase();
175+
176+
// Act
177+
var signatureBase = helper.GenerateSignatureBase(
178+
new Uri("https://example.com/resource?a=1"),
179+
"consumer-key",
180+
String.Empty,
181+
String.Empty,
182+
"mixid",
183+
"1234567890",
184+
"nonce",
185+
"HMAC-SHA1",
186+
out _,
187+
out _);
188+
189+
// Assert
190+
Assert.StartsWith("MIXID&", signatureBase, StringComparison.Ordinal);
191+
}
192+
finally
193+
{
194+
CultureInfo.CurrentCulture = originalCulture;
195+
CultureInfo.CurrentUICulture = originalUICulture;
196+
}
197+
}
198+
136199
private sealed class TestableYahooGeocoder : YahooGeocoder
137200
{
138201
private readonly HttpMessageHandler _handler;
@@ -148,5 +211,15 @@ protected override HttpClient BuildClient()
148211
return new HttpClient(_handler, disposeHandler: false);
149212
}
150213
}
214+
215+
private sealed class TestOAuthBase : OAuthBase
216+
{
217+
public int CompareParameters(string leftName, string leftValue, string rightName, string rightValue)
218+
{
219+
return new QueryParameterComparer().Compare(
220+
new QueryParameter(leftName, leftValue),
221+
new QueryParameter(rightName, rightValue));
222+
}
223+
}
151224
}
152225
#pragma warning restore CS0618

0 commit comments

Comments
 (0)