Skip to content

Commit 6197ab8

Browse files
authored
Move response Content-Type validation onto ProductRegistration; broaden vendor MIME match (#209)
## Move response validation onto `ProductRegistration` (the actual fix) `ResponseFactory.ValidateResponseContentType` carried a hardcoded `application/vnd.elasticsearch+json → application/json` fallback that: - was product-specific (Elasticsearch-shaped) but lived in generic transport code; and - never generalised. ES routinely strips the `;compatible-with=N` annotation and the `vnd.elasticsearch+` wrapper in responses (404, EQL, MVT, …). The hardcoded branch only covered the JSON case; `+x-ndjson` and the canonical `+vnd.mapbox-vector-tile → application/vnd.mapbox-vector-tile` translation were left out, so `IsValidResponse` was false even for legitimate `_mvt` responses. Move the rule to a new `ProductRegistration.IsExpectedResponseContentType` virtual: - Base: exact match or response Content-Type that starts with the Accept (covers `application/json` ↔ `application/json;charset=utf-8`). - `ElasticsearchProductRegistration` overrides: for any `application/vnd.elasticsearch+SUBTYPE[;…]` Accept entry (incl. multi-value lists), additionally accept the bare vendor form (`application/vnd.elasticsearch+SUBTYPE`) and the bare form (`application/SUBTYPE`). `ResponseFactory.InitializeApiCallDetails` and `DefaultResponseFactory.CreateCoreAsync` now delegate to the registration. The static helper and its hardcoded fallback are gone. ## Broaden the vendor MIME regex (small forward-compat tweak) The pattern introduced in #207 enumerated the three known vendor subtypes: application/vnd\.elasticsearch\+(json|x-ndjson|vnd\.mapbox-vector-tile) Replace with: application/vnd\.elasticsearch\+([A-Za-z0-9.\-+]+) Same coverage today; forward-compatible with any new `application/vnd.elasticsearch+SUBTYPE` ES might add. Plain MIMEs and third-party vendor MIMEs (`application/vnd.mapbox-vector-tile` etc.) stay untouched — those are the caller's to set. ## Tests - `Products/DefaultProductRegistrationContentTypeTests` pins the base contract. - `Products/Elasticsearch/IsExpectedResponseContentTypeTests` covers the vendor matrix: exact, more-verbose response, vendor compat-stripped, bare form for `+json` / `+x-ndjson` / `+vnd.mapbox-vector-tile`, multi-value Accept, case-insensitive matching, plain Accept, and third-party vendor MIMEs that must not trigger the bare-form rule.
1 parent 0ef60d5 commit 6197ab8

6 files changed

Lines changed: 190 additions & 32 deletions

File tree

src/Elastic.Transport/Products/Elasticsearch/ElasticsearchProductRegistration.cs

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ public partial class ElasticsearchProductRegistration : ProductRegistration
2828
internal const string XFoundHandlingInstanceHeader = "X-Found-Handling-Instance";
2929

3030
#if NET7_0_OR_GREATER
31-
[GeneratedRegex(@"application/vnd\.elasticsearch\+(json|x-ndjson|vnd\.mapbox-vector-tile)", RegexOptions.IgnoreCase)]
31+
[GeneratedRegex(@"application/vnd\.elasticsearch\+([A-Za-z0-9.\-+]+)", RegexOptions.IgnoreCase)]
3232
private static partial Regex VendorMimeRegex();
3333

3434
private static readonly Regex _vendorMimeRegex = VendorMimeRegex();
3535
#else
3636
private static readonly Regex _vendorMimeRegex = new(
37-
@"application/vnd\.elasticsearch\+(json|x-ndjson|vnd\.mapbox-vector-tile)",
37+
@"application/vnd\.elasticsearch\+([A-Za-z0-9.\-+]+)",
3838
RegexOptions.Compiled | RegexOptions.IgnoreCase);
3939
#endif
4040

@@ -112,11 +112,12 @@ public ElasticsearchProductRegistration(Type markerType) : this()
112112

113113
/// <inheritdoc cref="ProductRegistration.TransformContentType"/>
114114
/// <remarks>
115-
/// Appends <c>;compatible-with=N</c> to a supported vendor MIME type
116-
/// (<c>application/vnd.elasticsearch+json</c>, <c>+x-ndjson</c>, or
117-
/// <c>+vnd.mapbox-vector-tile</c>) when the parameter is not already present.
118-
/// Plain MIME types like <c>application/json</c> are returned unchanged so the
119-
/// caller stays in control of the value they explicitly provided.
115+
/// Appends <c>;compatible-with=N</c> to any <c>application/vnd.elasticsearch+SUBTYPE</c>
116+
/// MIME type (e.g. <c>+json</c>, <c>+x-ndjson</c>, <c>+vnd.mapbox-vector-tile</c>)
117+
/// when the parameter is not already present. Plain or third-party vendor MIME
118+
/// types (<c>application/json</c>, <c>application/vnd.mapbox-vector-tile</c>, …)
119+
/// are returned unchanged — appending a compatibility annotation to them is the
120+
/// caller's responsibility.
120121
/// </remarks>
121122
public override string? TransformContentType(string? contentType)
122123
{
@@ -140,6 +141,70 @@ private string AppendCompatibleWithAnnotation(string input)
140141
return _vendorMimeRegex.Replace(input, $"$0;compatible-with={_clientMajorVersion!.Value}");
141142
}
142143

144+
/// <inheritdoc cref="ProductRegistration.IsExpectedResponseContentType"/>
145+
/// <remarks>
146+
/// Extends the default rule with vendor-MIME equivalence: when the request's
147+
/// <c>Accept</c> is <c>application/vnd.elasticsearch+SUBTYPE[;…]</c>, a response
148+
/// is also accepted if its <c>Content-Type</c> matches the bare vendor form
149+
/// (<c>application/vnd.elasticsearch+SUBTYPE</c>) or the bare form
150+
/// (<c>application/SUBTYPE</c>) — Elasticsearch frequently omits the
151+
/// <c>;compatible-with=N</c> annotation in responses (e.g. on 404, EQL, MVT)
152+
/// and may strip the <c>vnd.elasticsearch+</c> wrapper too. Multi-value
153+
/// <c>Accept</c> lists are checked entry-by-entry.
154+
/// </remarks>
155+
public override bool IsExpectedResponseContentType(string accept, string? responseContentType)
156+
{
157+
if (base.IsExpectedResponseContentType(accept, responseContentType))
158+
return true;
159+
160+
if (string.IsNullOrEmpty(responseContentType))
161+
return false;
162+
163+
var normalized = responseContentType!.Replace(" ", "");
164+
165+
foreach (var entry in accept.Split(','))
166+
{
167+
var trimmedEntry = entry.Replace(" ", "");
168+
if (!TryParseElasticsearchVendorMime(trimmedEntry, out var bareVendor, out var bareForm))
169+
continue;
170+
171+
if (normalized.Equals(bareVendor, StringComparison.OrdinalIgnoreCase)
172+
|| normalized.StartsWith(bareVendor, StringComparison.OrdinalIgnoreCase)
173+
|| normalized.Equals(bareForm, StringComparison.OrdinalIgnoreCase)
174+
|| normalized.StartsWith(bareForm, StringComparison.OrdinalIgnoreCase))
175+
return true;
176+
}
177+
178+
return false;
179+
}
180+
181+
private static bool TryParseElasticsearchVendorMime(string mime, out string bareVendor, out string bareForm)
182+
{
183+
const string prefix = "application/vnd.elasticsearch+";
184+
if (!mime.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
185+
{
186+
bareVendor = string.Empty;
187+
bareForm = string.Empty;
188+
return false;
189+
}
190+
191+
var afterPlus = mime.AsSpan(prefix.Length);
192+
var endIdx = afterPlus.IndexOfAny(';', ',');
193+
var subtypeSpan = endIdx < 0 ? afterPlus : afterPlus[..endIdx];
194+
195+
if (subtypeSpan.Length == 0)
196+
{
197+
bareVendor = string.Empty;
198+
bareForm = string.Empty;
199+
return false;
200+
}
201+
202+
var subtype = subtypeSpan.ToString();
203+
bareVendor = prefix + subtype;
204+
bareForm = "application/" + subtype;
205+
return true;
206+
}
207+
143208
/// <summary> Exposes the path used for sniffing in Elasticsearch </summary>
144209
public const string SniffPath = "_nodes/http,settings";
145210

src/Elastic.Transport/Products/ProductRegistration.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,31 @@ public abstract class ProductRegistration
3838
/// </summary>
3939
public virtual string? TransformContentType(string? contentType) => contentType;
4040

41+
/// <summary>
42+
/// Decides whether a response <c>Content-Type</c> is acceptable given the
43+
/// request's <c>Accept</c>. The default accepts an exact (case-insensitive)
44+
/// match or a response Content-Type that starts with the Accept (so
45+
/// <c>application/json</c> matches <c>application/json;charset=utf-8</c>).
46+
/// Products override this to add their own equivalence rules — for example,
47+
/// translating between a vendor MIME type and its bare form.
48+
/// </summary>
49+
/// <param name="accept">The <c>Accept</c> header sent on the request.</param>
50+
/// <param name="responseContentType">The <c>Content-Type</c> returned by the server, if any.</param>
51+
public virtual bool IsExpectedResponseContentType(string accept, string? responseContentType)
52+
{
53+
if (string.IsNullOrEmpty(responseContentType))
54+
return false;
55+
56+
if (accept == responseContentType)
57+
return true;
58+
59+
var trimmedAccept = accept.Replace(" ", "");
60+
var normalized = responseContentType!.Replace(" ", "");
61+
62+
return normalized.Equals(trimmedAccept, StringComparison.OrdinalIgnoreCase)
63+
|| normalized.StartsWith(trimmedAccept, StringComparison.OrdinalIgnoreCase);
64+
}
65+
4166
/// <summary>
4267
/// The name of the current product utilizing <see cref="ITransport{TConfiguration}"/>
4368
/// <para>This name makes its way into the transport diagnostics sources and the default user agent string</para>

src/Elastic.Transport/Responses/DefaultResponseFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ private async ValueTask<TResponse> CreateCoreAsync<TResponse>(
129129
}
130130

131131
// We only attempt to build a response when the Content-Type matches the accepted type.
132-
if (builder is not null && ValidateResponseContentType(boundConfiguration.Accept, contentType) && contentType is not null)
132+
if (builder is not null && productRegistration.IsExpectedResponseContentType(boundConfiguration.Accept, contentType) && contentType is not null)
133133
{
134134
response = isAsync
135135
? await builder.BuildAsync<TResponse>(details, boundConfiguration, responseStream, contentType, contentLength, cancellationToken).ConfigureAwait(false)

src/Elastic.Transport/Responses/ResponseFactory.cs

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ internal static ApiCallDetails InitializeApiCallDetails(
7777

7878
// We don't validate the content-type (MIME type) for HEAD requests or responses that have no content (204 status code).
7979
// Elastic Cloud responses to HEAD requests strip the content-type header so we want to avoid validation in that case.
80-
var hasExpectedContentType = !MayHaveBody(statusCode, endpoint.Method, contentLength) || ValidateResponseContentType(boundConfiguration.Accept, contentType);
80+
var hasExpectedContentType = !MayHaveBody(statusCode, endpoint.Method, contentLength)
81+
|| boundConfiguration.ConnectionSettings.ProductRegistration.IsExpectedResponseContentType(boundConfiguration.Accept, contentType);
8182

8283
var details = new ApiCallDetails
8384
{
@@ -108,27 +109,4 @@ internal static ApiCallDetails InitializeApiCallDetails(
108109
protected static bool MayHaveBody(int? statusCode, HttpMethod httpMethod, long contentLength) =>
109110
contentLength != 0 && (!statusCode.HasValue || (statusCode.Value != 204 && httpMethod != HttpMethod.HEAD));
110111

111-
internal static bool ValidateResponseContentType(string accept, string? responseContentType)
112-
{
113-
if (string.IsNullOrEmpty(responseContentType))
114-
return false;
115-
116-
if (accept == responseContentType)
117-
return true;
118-
119-
// TODO - Performance: Review options to avoid the replace here and compare more efficiently.
120-
// At this point, responseContentType is guaranteed to be non-null due to the check at line 116
121-
var trimmedAccept = accept.Replace(" ", "");
122-
var normalizedResponseContentType = responseContentType!.Replace(" ", "");
123-
124-
return normalizedResponseContentType.Equals(trimmedAccept, StringComparison.OrdinalIgnoreCase)
125-
|| normalizedResponseContentType.StartsWith(trimmedAccept, StringComparison.OrdinalIgnoreCase)
126-
127-
// ES specific fallback required because:
128-
// - 404 responses from ES8 don't include the vendored header
129-
// - ES8 EQL responses don't include vendored type
130-
131-
|| (trimmedAccept.Contains("application/vnd.elasticsearch+json")
132-
&& normalizedResponseContentType.StartsWith(BoundConfiguration.DefaultContentType, StringComparison.OrdinalIgnoreCase));
133-
}
134112
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Licensed to Elasticsearch B.V under one or more agreements.
2+
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
3+
// See the LICENSE file in the project root for more information
4+
5+
#nullable enable
6+
7+
using Elastic.Transport.Products;
8+
using FluentAssertions;
9+
using Xunit;
10+
11+
namespace Elastic.Transport.Tests.Products;
12+
13+
public class DefaultProductRegistrationContentTypeTests
14+
{
15+
[Theory]
16+
[InlineData("application/json", "application/json")]
17+
[InlineData("application/json", "application/json;charset=utf-8")]
18+
[InlineData("application/json", "APPLICATION/JSON")]
19+
[InlineData("text/event-stream", "text/event-stream")]
20+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/vnd.elasticsearch+json;compatible-with=8")]
21+
public void ReturnsTrueForExactOrPrefixMatch(string accept, string responseContentType) =>
22+
DefaultProductRegistration.Default.IsExpectedResponseContentType(accept, responseContentType).Should().BeTrue();
23+
24+
[Theory]
25+
[InlineData("application/json", "text/plain")]
26+
[InlineData("application/json", "application/xml")]
27+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/json")]
28+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/vnd.elasticsearch+json")]
29+
[InlineData("application/json", null)]
30+
[InlineData("application/json", "")]
31+
public void ReturnsFalseForMismatchOrEmpty(string accept, string? responseContentType) =>
32+
DefaultProductRegistration.Default.IsExpectedResponseContentType(accept, responseContentType).Should().BeFalse();
33+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Licensed to Elasticsearch B.V under one or more agreements.
2+
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
3+
// See the LICENSE file in the project root for more information
4+
5+
#nullable enable
6+
7+
using Elastic.Transport.Products.Elasticsearch;
8+
using FluentAssertions;
9+
using Xunit;
10+
11+
namespace Elastic.Transport.Tests.Products.Elasticsearch;
12+
13+
public class IsExpectedResponseContentTypeTests
14+
{
15+
[Theory]
16+
// Exact match
17+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/vnd.elasticsearch+json;compatible-with=8")]
18+
// Response is more verbose (extra parameters)
19+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/vnd.elasticsearch+json;compatible-with=8;charset=utf-8")]
20+
// Vendor MIME with compat-with stripped from the response
21+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/vnd.elasticsearch+json")]
22+
// Bare form (the part after the +)
23+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/json")]
24+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "application/json;charset=utf-8")]
25+
// x-ndjson bare form
26+
[InlineData("application/vnd.elasticsearch+x-ndjson;compatible-with=8", "application/vnd.elasticsearch+x-ndjson")]
27+
[InlineData("application/vnd.elasticsearch+x-ndjson;compatible-with=8", "application/x-ndjson")]
28+
// Canonical mapbox bare form (the SearchMvt path)
29+
[InlineData("application/vnd.elasticsearch+vnd.mapbox-vector-tile;compatible-with=8", "application/vnd.mapbox-vector-tile")]
30+
[InlineData("application/vnd.elasticsearch+vnd.mapbox-vector-tile;compatible-with=8", "application/vnd.elasticsearch+vnd.mapbox-vector-tile")]
31+
// Multi-value Accept — any vendor entry's bare form is acceptable
32+
[InlineData("application/vnd.elasticsearch+json,application/vnd.elasticsearch+x-ndjson", "application/x-ndjson")]
33+
[InlineData("application/vnd.elasticsearch+json,application/vnd.elasticsearch+x-ndjson", "application/json")]
34+
// Case-insensitive
35+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "APPLICATION/JSON")]
36+
public void Accepts(string accept, string responseContentType) =>
37+
new ElasticsearchProductRegistration(typeof(ElasticsearchProductRegistration))
38+
.IsExpectedResponseContentType(accept, responseContentType).Should().BeTrue();
39+
40+
[Theory]
41+
// Plain Accept — bare-form rule must NOT trigger; only exact / prefix matches.
42+
[InlineData("application/json", "text/plain")]
43+
[InlineData("text/event-stream", "application/json")]
44+
// Non-elasticsearch vendor MIME as Accept — bare-form rule must NOT trigger
45+
// (third-party vendor MIMEs aren't owned by this product).
46+
[InlineData("application/vnd.mapbox-vector-tile", "application/json")]
47+
[InlineData("application/vnd.mapbox-vector-tile;compatible-with=8", "application/json")]
48+
// Vendor Accept — completely different MIME on response
49+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "text/html")]
50+
[InlineData("application/vnd.elasticsearch+x-ndjson;compatible-with=8", "application/json")]
51+
// Empty / null response
52+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", null)]
53+
[InlineData("application/vnd.elasticsearch+json;compatible-with=8", "")]
54+
public void Rejects(string accept, string? responseContentType) =>
55+
new ElasticsearchProductRegistration(typeof(ElasticsearchProductRegistration))
56+
.IsExpectedResponseContentType(accept, responseContentType).Should().BeFalse();
57+
}

0 commit comments

Comments
 (0)