fix(inbound): 415 guard and lenient JSON parse cover chunked (no Content-Length) bodies (plan R2-06 T6)
This commit is contained in:
@@ -118,7 +118,7 @@ The `sbk_<keyId>_<secret>` design already supports zero-downtime rotation — a
|
|||||||
|--------|-------------|---------|
|
|--------|-------------|---------|
|
||||||
| `UNAUTHORIZED` | 401 | Missing/invalid API key (every auth-stage failure maps here — no stage leak). |
|
| `UNAUTHORIZED` | 401 | Missing/invalid API key (every auth-stage failure maps here — no stage leak). |
|
||||||
| `NOT_APPROVED` | 403 | Method not found **or** key not in scope — indistinguishable by design. |
|
| `NOT_APPROVED` | 403 | Method not found **or** key not in scope — indistinguishable by design. |
|
||||||
| `UNSUPPORTED_MEDIA_TYPE` | 415 | Request carried a body with a non-JSON `Content-Type`. A body with **no** `Content-Type` is still parsed leniently as JSON. |
|
| `UNSUPPORTED_MEDIA_TYPE` | 415 | Request carried a body — declared by `Content-Length` or `Transfer-Encoding` (chunked) — with a non-JSON `Content-Type`. A body with **no** `Content-Type` is still parsed leniently as JSON, and a chunked (no `Content-Length`) body is treated identically to a fixed-length one. |
|
||||||
| `INVALID_JSON` | 400 | Request body was not valid JSON. |
|
| `INVALID_JSON` | 400 | Request body was not valid JSON. |
|
||||||
| `VALIDATION_FAILED` | 400 | Parameter validation failed (missing required field, wrong type, undeclared field). |
|
| `VALIDATION_FAILED` | 400 | Parameter validation failed (missing required field, wrong type, undeclared field). |
|
||||||
| `BODY_TOO_LARGE` | 413 | Request body exceeded `MaxRequestBodyBytes`. |
|
| `BODY_TOO_LARGE` | 413 | Request body exceeded `MaxRequestBodyBytes`. |
|
||||||
|
|||||||
@@ -179,12 +179,22 @@ public static class EndpointExtensions
|
|||||||
httpContext.Items[AuditWriteMiddleware.AuditActorItemKey] =
|
httpContext.Items[AuditWriteMiddleware.AuditActorItemKey] =
|
||||||
identity.DisplayName;
|
identity.DisplayName;
|
||||||
|
|
||||||
|
// N5: a body is present when Content-Length says so, OR when the request is
|
||||||
|
// chunked — Transfer-Encoding present and Content-Length (necessarily) absent.
|
||||||
|
// Computed once and used by BOTH the 415 guard and the JSON parse condition so
|
||||||
|
// chunked and fixed-length bodies behave identically.
|
||||||
|
var requestHeaders = httpContext.Request.Headers;
|
||||||
|
var hasBody = httpContext.Request.ContentLength > 0
|
||||||
|
|| (httpContext.Request.ContentLength is null && requestHeaders.TransferEncoding.Count > 0);
|
||||||
|
|
||||||
// S9: a body sent with an explicit non-JSON Content-Type is a media-type
|
// S9: a body sent with an explicit non-JSON Content-Type is a media-type
|
||||||
// mismatch, not malformed JSON — reject it with 415 rather than a misleading
|
// mismatch, not malformed JSON — reject it with 415 rather than a misleading
|
||||||
// 400 "invalid JSON". A body with NO Content-Type header is still parsed
|
// 400 "invalid JSON". A body with NO Content-Type header is still parsed
|
||||||
// leniently as JSON below (preserving existing callers). The sniff is
|
// leniently as JSON below (preserving existing callers). The sniff is
|
||||||
// ordinal-ignore-case so "application/JSON" etc. are treated as JSON.
|
// ordinal-ignore-case so "application/JSON" etc. are treated as JSON. N5: the
|
||||||
if (httpContext.Request.ContentLength > 0
|
// body signal is `hasBody`, so a chunked (no Content-Length) non-JSON body is
|
||||||
|
// also caught here instead of slipping through to a misleading 400.
|
||||||
|
if (hasBody
|
||||||
&& !string.IsNullOrEmpty(httpContext.Request.ContentType)
|
&& !string.IsNullOrEmpty(httpContext.Request.ContentType)
|
||||||
&& httpContext.Request.ContentType.Contains("json", StringComparison.OrdinalIgnoreCase) == false)
|
&& httpContext.Request.ContentType.Contains("json", StringComparison.OrdinalIgnoreCase) == false)
|
||||||
{
|
{
|
||||||
@@ -203,7 +213,7 @@ public static class EndpointExtensions
|
|||||||
// `Contains("json")` silently skipped JSON deserialization for any
|
// `Contains("json")` silently skipped JSON deserialization for any
|
||||||
// capitalised value, leaving `body = null` and surfacing required
|
// capitalised value, leaving `body = null` and surfacing required
|
||||||
// parameters as 400 "missing" even though the caller sent a valid body.
|
// parameters as 400 "missing" even though the caller sent a valid body.
|
||||||
if (httpContext.Request.ContentLength > 0
|
if (hasBody
|
||||||
|| httpContext.Request.ContentType?.Contains("json", StringComparison.OrdinalIgnoreCase) == true)
|
|| httpContext.Request.ContentType?.Contains("json", StringComparison.OrdinalIgnoreCase) == true)
|
||||||
{
|
{
|
||||||
using var doc = await JsonDocument.ParseAsync(
|
using var doc = await JsonDocument.ParseAsync(
|
||||||
|
|||||||
@@ -226,4 +226,90 @@ public sealed class EndpointContentTypeTests : IDisposable
|
|||||||
// Best-effort cleanup.
|
// Best-effort cleanup.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ChunkedNonJsonBody_Returns415()
|
||||||
|
{
|
||||||
|
// Arch-review R2 N5: a chunked body has NULL ContentLength — it must not
|
||||||
|
// slip past the 415 guard into a misleading VALIDATION_FAILED.
|
||||||
|
const string methodName = "echoChunked";
|
||||||
|
var method = new ApiMethod(methodName, "return Parameters[\"value\"];")
|
||||||
|
{
|
||||||
|
Id = 1,
|
||||||
|
TimeoutSeconds = 10,
|
||||||
|
ParameterDefinitions = """[{"name":"value","type":"Integer","required":true}]""",
|
||||||
|
};
|
||||||
|
var repo = Substitute.For<IInboundApiRepository>();
|
||||||
|
repo.GetMethodByNameAsync(methodName, Arg.Any<CancellationToken>()).Returns(method);
|
||||||
|
|
||||||
|
using var host = await BuildHostAsync(repo);
|
||||||
|
var token = await SeedKeyAsync(host, methodName);
|
||||||
|
var client = host.GetTestClient();
|
||||||
|
|
||||||
|
var request = new HttpRequestMessage(HttpMethod.Post, "/api/" + methodName)
|
||||||
|
{
|
||||||
|
Content = new ChunkedByteContent(Encoding.UTF8.GetBytes("hello")),
|
||||||
|
};
|
||||||
|
request.Content.Headers.ContentType = MediaTypeHeaderValue.Parse("text/plain");
|
||||||
|
request.Headers.TransferEncodingChunked = true;
|
||||||
|
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
|
||||||
|
|
||||||
|
var response = await client.SendAsync(request);
|
||||||
|
var body = await response.Content.ReadAsStringAsync();
|
||||||
|
|
||||||
|
Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode);
|
||||||
|
Assert.Contains("UNSUPPORTED_MEDIA_TYPE", body);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ChunkedJsonBody_NoContentType_StillParsesLeniently()
|
||||||
|
{
|
||||||
|
// The parse condition must also learn about chunked bodies: a chunked JSON
|
||||||
|
// body with no Content-Type previously skipped parsing entirely (body=null →
|
||||||
|
// misleading "missing required parameter").
|
||||||
|
const string methodName = "echoChunkedJson";
|
||||||
|
var method = new ApiMethod(methodName, "return Parameters[\"value\"];")
|
||||||
|
{
|
||||||
|
Id = 1,
|
||||||
|
TimeoutSeconds = 10,
|
||||||
|
ParameterDefinitions = """[{"name":"value","type":"Integer","required":true}]""",
|
||||||
|
};
|
||||||
|
var repo = Substitute.For<IInboundApiRepository>();
|
||||||
|
repo.GetMethodByNameAsync(methodName, Arg.Any<CancellationToken>()).Returns(method);
|
||||||
|
|
||||||
|
using var host = await BuildHostAsync(repo);
|
||||||
|
var token = await SeedKeyAsync(host, methodName);
|
||||||
|
var client = host.GetTestClient();
|
||||||
|
|
||||||
|
var request = new HttpRequestMessage(HttpMethod.Post, "/api/" + methodName)
|
||||||
|
{
|
||||||
|
Content = new ChunkedByteContent(Encoding.UTF8.GetBytes("{\"value\":42}")),
|
||||||
|
};
|
||||||
|
request.Content.Headers.ContentType = null;
|
||||||
|
request.Headers.TransferEncodingChunked = true;
|
||||||
|
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
|
||||||
|
|
||||||
|
var response = await client.SendAsync(request);
|
||||||
|
var body = await response.Content.ReadAsStringAsync();
|
||||||
|
|
||||||
|
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||||
|
Assert.Contains("42", body);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Content whose length is never computable, forcing HttpClient to transmit it
|
||||||
|
/// chunked (no Content-Length header ever reaches the server).
|
||||||
|
/// </summary>
|
||||||
|
private sealed class ChunkedByteContent(byte[] bytes) : HttpContent
|
||||||
|
{
|
||||||
|
protected override Task SerializeToStreamAsync(
|
||||||
|
Stream stream, System.Net.TransportContext? context) =>
|
||||||
|
stream.WriteAsync(bytes, 0, bytes.Length);
|
||||||
|
|
||||||
|
protected override bool TryComputeLength(out long length)
|
||||||
|
{
|
||||||
|
length = -1;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user