fix: Ignore Content-Type header parameters when checking media type - #934
Open
zoliszabo wants to merge 3 commits into
Open
fix: Ignore Content-Type header parameters when checking media type#934zoliszabo wants to merge 3 commits into
zoliszabo wants to merge 3 commits into
Conversation
Both HTTP retrievers (`FileGetContents` and `Curl`) captured the raw `Content-Type` header value verbatim (regex `/Content-Type:(\V*)/ims`), charset included. CDNs serve schemas with e.g. `application/json; charset=utf-8`, which failed the exact media type check against `application/json` in confirmMediaType(). RFC 8259 (JSON Data Interchange Format, https://www.rfc-editor.org/info/rfc8259/) does not define a charset parameter for the application/json media type: section 11 says "No 'charset' parameter is defined for this registration. Adding one really has no effect on compliant recipients.". The parameter therefore must not affect the comparison. The retrievers now capture only up to the first `;` (`/Content-Type:([^;\v]*)/ims`), so the `Content-Type` is normalized to its base type before the check. CDN example: https://cdn.jsdelivr.net/gh/WordPress/gutenberg@trunk/schemas/json/theme.json.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the URI retrievers’ Content-Type parsing so media-type validation treats application/json; charset=utf-8 as application/json, preventing false failures when CDNs include parameters on JSON schema responses.
Changes:
- Update
Content-Typeextraction regexes inFileGetContentsandCurlto capture only the base media type (up to;). - Add PHPUnit coverage to ensure
Content-Typeparameters are ignored for both retrievers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/JsonSchema/Uri/Retrievers/FileGetContents.php |
Adjusts Content-Type header parsing to ignore parameters before media-type validation. |
src/JsonSchema/Uri/Retrievers/Curl.php |
Adjusts Content-Type parsing from raw cURL response to ignore parameters before media-type validation. |
tests/Uri/Retrievers/FileGetContentsTest.php |
Adds data-driven tests verifying Content-Type parameter stripping for FileGetContents. |
tests/Uri/Retrievers/CurlTest.php |
Adds data-driven tests verifying Content-Type parameter stripping for Curl. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The parameter-stripping capture `[^;\v]*` was introduced inside a double-quoted string, where PHP interprets `\v` as a vertical-tab character instead of passing it through to PCRE as the vertical-whitespace escape. For a Content-Type without parameters, the match could therefore run past the CRLF and capture subsequent headers and body. Both retrievers now use a single-quoted, line-anchored pattern '/^Content-Type:([^;\v]*)/im': single-quoting lets the escape reach PCRE verbatim, and anchoring prevents headers like `X-Content-Type` from matching as a content type. The unused `s` modifier is dropped. Tests are consolidated into data-provider driven cases covering the no-parameter form, charset and multiple parameters, and the `X-Content-Type` negative, so both the escaping and anchoring regressions are guarded.
Author
|
Pushed a new commit with the regex fixes and consolidated tests. |
Collaborator
|
@zoliszabo thanks for you contributions. I'm planning to review them in the upcoming week. |
DannyvdSluijs
approved these changes
Aug 14, 2026
Collaborator
|
Seems all good. Will wait with the merge depending on #931 to avoid conflicts. Expect to merge this within a week. |
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.
Description
Both HTTP retrievers (
FileGetContentsandCurl) capture the rawContent-Typeheader value verbatim (regex/Content-Type:(\V*)/ims), charset included. CDNs serve schemas with e.g.application/json; charset=utf-8, which fails the exact media type check againstapplication/jsoninconfirmMediaType().RFC 8259 (JSON Data Interchange Format, https://www.rfc-editor.org/info/rfc8259/) does not define a charset parameter for the application/json media type: section 11 says "No 'charset' parameter is defined for this registration. Adding one really has no effect on compliant recipients.". The parameter therefore must not affect the comparison.
In the updated version, the retrievers capture only up to the first
;(/Content-Type:([^;\v]*)/ims), so theContent-Typeis normalized to its base type before the check, ignoring any parameters.CDN example: https://cdn.jsdelivr.net/gh/WordPress/gutenberg@trunk/schemas/json/theme.json.
Related Issue
N/A
Type of Change
Checklist
Additional Notes
N/A