fix(transport): answer 413 for an over-cap body on both JSON APIs

What

internal/managementapi answered 400 bad_request for a request body over server.max_body_size when the request carried no Content-Length. It now answers 413, like the identical payload that declares its length.

The strict JSON decode both API surfaces run (decoder with DisallowUnknownFields, the single-object check, the 400 message classification, and the size-cap detection) moves into internal/transport.DecodeJSONBody. internal/gitlabapi and internal/managementapi both call it and keep their own way of writing a rejection. The 400 messages each surface produces are unchanged.

How the bug works

Two guards enforce server.max_body_size:

  1. BodySizeMiddleware rejects a request whose Content-Length is over the cap before reading anything: 413.
  2. A body with no Content-Length (chunked) is wrapped in http.MaxBytesReader instead, so the overflow surfaces inside the handler as a read error during decode.

The management decode path had no *http.MaxBytesError branch at all, so path 2 fell through to the generic "request body contains malformed JSON" 400. The same payload got 413 or 400 depending only on how the client framed it.

There is a second layer. The decoder usually wraps the MaxBytesReader read error into its own *json.SyntaxError (jsontext: read error: ...), and whether the typed error reaches the handler tracks the decoder's internal buffer state at the cutoff, not any property of the request. Measured on go1.26.5, for one fixture the type survives at caps 1-7 and 71 but not at 9. So even internal/gitlabapi, which did have the typed check, classified the overflow by accident of byte position. A tripped MaxBytesReader replays the typed error on every later Read without touching the connection, so a one-byte probe recovers the condition either way.

Contract

api/openapi/v1.yaml never documented 413, although the middleware already returned it on a declared over-cap length. This adds the response to the two body-carrying routes (POST, PATCH) and request_entity_too_large to the error-code enum. The new handler tests validate the 413 against the document.

Tests

  • internal/managementapi: 413 on POST and PATCH, at two caps that straddle the decoder's error handling. Both rows fail on main with 400.
  • internal/gitlabapi: the same two cap positions on the provisioning route, so its route-level net is non-vacuous too.
  • internal/transport: the 400 classification table (including a malformed body under a cap it never reaches, so a "wrapped means 413" misclassification fails), both 413 caps, and the accepting edge (a body sized exactly at the cap must not 413).
  • request_internal_test.go shrinks to what it still owns: the adapter carrying status, code, and message onto requestError. Its message table moved to internal/transport with the code.

Non-vacuousness: replacing the probe with the bare type assertion fails exactly the "typed error wrapped" rows in all three packages and passes the others. The typed assertion itself is a fast path the probe subsumes; that is documented rather than pinned, because no test can distinguish it.

Not here

The 500-writing helpers are still duplicated across both surfaces and their cancellation semantics differ, so unifying them changes behavior for one of them. decodeErrorMessage also still names a Go type (must be a JSON int for a hypothetical numeric field); no surface has a non-string body field today.

The npm surface carries the same fragility this MR removes: unpublish_version.go classifies the capped-body condition with the bare typed check, and deprecate.go has no 413 branch under its envelope-sniff cap. npm's own envelope and code types make transport.DecodeJSONBody non-reusable there, so they are scoped out; #400 owns them.

Testing

No e2e scenario is affected: the management API has no e2e catalog under docs/testing/, and this change alters no format-client-visible behavior (the 413/400 split is exercised by the unit and contract tests above).

Related to #394 (closed)

Edited by João Pereira

Merge request reports

Loading
Loading