diff --git a/docs/requirements/Component-InboundAPI.md b/docs/requirements/Component-InboundAPI.md index bd968f18..59c37ba9 100644 --- a/docs/requirements/Component-InboundAPI.md +++ b/docs/requirements/Component-InboundAPI.md @@ -118,7 +118,7 @@ The `sbk__` design already supports zero-downtime rotation — a |--------|-------------|---------| | `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. | -| `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. | | `VALIDATION_FAILED` | 400 | Parameter validation failed (missing required field, wrong type, undeclared field). | | `BODY_TOO_LARGE` | 413 | Request body exceeded `MaxRequestBodyBytes`. | diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs index 85e5237c..0f753ef5 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs @@ -179,12 +179,22 @@ public static class EndpointExtensions httpContext.Items[AuditWriteMiddleware.AuditActorItemKey] = 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 // 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 // leniently as JSON below (preserving existing callers). The sniff is - // ordinal-ignore-case so "application/JSON" etc. are treated as JSON. - if (httpContext.Request.ContentLength > 0 + // ordinal-ignore-case so "application/JSON" etc. are treated as JSON. N5: the + // 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) && httpContext.Request.ContentType.Contains("json", StringComparison.OrdinalIgnoreCase) == false) { @@ -203,7 +213,7 @@ public static class EndpointExtensions // `Contains("json")` silently skipped JSON deserialization for any // capitalised value, leaving `body = null` and surfacing required // 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) { using var doc = await JsonDocument.ParseAsync( diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs index 2e5c1cc2..fbf8db84 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs @@ -226,4 +226,90 @@ public sealed class EndpointContentTypeTests : IDisposable // 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(); + repo.GetMethodByNameAsync(methodName, Arg.Any()).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(); + repo.GetMethodByNameAsync(methodName, Arg.Any()).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); + } + + /// + /// Content whose length is never computable, forcing HttpClient to transmit it + /// chunked (no Content-Length header ever reaches the server). + /// + 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; + } + } }