fix(adapters): align adapter JSON content-type detection with ContentTypeMatcher#253
Merged
Merged
Conversation
…TypeMatcher The three adapter body-extraction methods (Laravel `extractRequestBody` / `extractJsonBody`, Symfony `extractSymfonyJsonBody`) decided whether to JSON-decode a body with a loose `str_contains(strtolower($contentType), 'json')` check. That treats borderline media types such as `application/jsonsomethingweird` as JSON, so the adapter would attempt a `json_decode()` on a non-JSON body and fail with a misleading "could not be parsed as JSON" parse error. Switch the three methods to `ContentTypeMatcher::isJsonContentType()` (fed through `normalizeMediaType()`) — the same strict check the body validators already use. The adapter and validator now agree on exactly which media types count as JSON: a non-JSON body is left undecoded and reported absent, and the validator's content negotiation surfaces its clean "Content-Type is not defined" diagnostic instead. Standard types (`application/json`, `application/*+json`, `application/json; charset=utf-8`) are unaffected. Closes #251
…ct extractRequestBody docblock Review follow-ups for #251: - ContentTypeMatcherTest: add an explicit negative case pinning that media types which merely contain the substring "json" (text/json, application/jsonp, application/jsonsomethingweird) are not JSON. This is the contract the adapters now delegate to, and text/json / application/jsonp are behavior-change cases relative to the old loose check. - extractRequestBody() docblock: the previous wording promised a "loud" verdict unconditionally. The request-side validator only reports an undeclared media type when the operation declares a requestBody content map, and a declared non-JSON media type is accepted without body-schema validation (the schema engine is JSON-only). Reworded to match.
Collaborator
Author
Multi-agent review (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The three adapter body-extraction methods (Laravel
extractRequestBody/extractJsonBody, SymfonyextractSymfonyJsonBody) decided whether to JSON-decode a body using a loosestr_contains(strtolower($contentType), 'json')check. They now use the strictContentTypeMatcher::isJsonContentType()— the same check the body validators already use — so the adapter and validator agree on which media types count as JSON.Why
Fixes #251.
The loose check treats borderline media types such as
application/jsonsomethingweirdas JSON, so the adapter would attemptjson_decode()on a non-JSON body and fail with a misleading "could not be parsed as JSON" parse error, when the real issue is the Content-Type. After the change the non-JSON body is left undecoded and reported absent, and the validator's content negotiation surfaces its clean "Content-Type is not defined" diagnostic instead.Scope note (investigation finding): Issue #251 describes a "silent pass" for non-empty non-JSON bodies. On close inspection that scenario no longer reproduces — the content negotiation introduced in #246 already makes the body validators decide non-JSON handling from the separately-forwarded
$contentType(loud "Content-Type is not defined" when the spec does not declare the type; correctly accepted when it does). The body envelope's absent-vs-present bit is never consulted for non-JSON types, so extendingDecodedBodywith a third state was deliberately not done — it would be a dead state nothing reads. The remaining, actionable gap was the adapter/validator JSON-detection inconsistency addressed here.Behavior change (minor, intentional): non-standard JSON-ish content types (
application/jsonsomethingweird,text/json,application/jsonp) are now treated as non-JSON by the adapters, matching the validators. Standard types (application/json,application/*+json,application/json; charset=utf-8) are unaffected.Verification
Failing tests were added first (TDD), confirmed red, then made green by the fix. Each asserts the failure message is the clean
Content-Type ... is not defineddiagnostic rather thancould not be parsed as JSON:ValidatesOpenApiSchemaTest::content_type_containing_json_substring_is_not_decoded_as_json(Laravel response)ValidatesOpenApiSchemaAutoValidateRequestTest::content_type_containing_json_substring_is_not_decoded_as_json(Laravel request)OpenApiAssertionsTest::content_type_containing_json_substring_is_not_decoded_as_json(Symfony)composer testpasses (1788 tests, 4201 assertions)composer stanpasses (no errors)composer cs-checkpasses (no violations)Notes for reviewers
ContentTypeMatcheris@internal; intra-package use from the adapters mirrors howRequestBodyValidator/ResponseBodyValidatoralready consume it.