JSON API: serializable_error() valid-int-only status hardening#50077
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
|
There was a problem hiding this comment.
Pull request overview
This PR hardens WPCOM_JSON_API::serializable_error() so WP_Error responses always serialize a sane HTTP error status code (integer and >= 400), preventing array-shaped error data from being (int)-cast to 1 and avoiding accidental 2xx/3xx statuses that clients could interpret as success.
Changes:
- Coerce error
status_codeto an integer and default/coerce any< 400(or non-numeric) value to400. - Add PHPUnit coverage for the regression cases (array-without-
status_code→400, non-error statuses →400, unknown>= 400statuses pass through, error shape preserved). - Add a Jetpack plugin changelog entry for the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| projects/plugins/jetpack/class.json-api.php | Hardens status extraction/coercion in serializable_error() to ensure int >= 400. |
| projects/plugins/jetpack/tests/php/json-api/WPCOM_JSON_API_Serializable_Error_Test.php | Adds targeted unit tests covering the regression and intended status coercion behavior. |
| projects/plugins/jetpack/changelog/update-json-api-serializable-error-valid-int | Adds a changelog entry describing the JSON API error-status hardening. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Related to CONNECT-267 (problem 1)
This is a second attempt after #50047 failed deployment CI in WPCOM. More info in p1782802283094959-slack-C01U2KGS2PQ
It is less ambitious than the previous PR, but we will handle problem 2 separately.
Proposed changes
Harden
WPCOM_JSON_API::serializable_error()so an error always serializes a valid HTTP status (>= 400) — never1, a non-integer, or a2xxan app could read as success.The previous extraction let array-shaped error data without a
status_codekey fall through as a truthy array, which downstream(int)-cast to1(and, withouthttp_envelope, a200 OK) — the root of the app-crash incident. It now coerces non-numeric / array data and any< 400value to400.Does this pull request change what data or activity we track or use?
No
Testing instructions
jetpack docker phpunit jetpack -- --filter=WPCOM_JSON_API_Serializable_Error_Test— 11 tests green.>= 400left unchanged), array-without-status_code->400(not1),2xx->400, and the error body shape (code/message/additional_data) preserved.