Repository navigation
fix: keep REST URLs intact in per-page safe mode - #596
Open
faisalahammad wants to merge 1 commit into
Open
faisalahammad wants to merge 1 commit into
faisalahammad wants to merge 1 commit into
Conversation
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
Per-page safe mode (
?snippets-safe-mode=1) added the query var to everyhome_url(). REST URLs are built on top of the home URL, so with pretty permalinks they came out as/wp-json?snippets-safe-mode=1/code-snippets/v1/snippets/5. The route ended up in the query string and every request hit the REST index. So in safe mode you could not deactivate or delete the snippet that was crashing the site, which is the whole point of safe mode.Now
rest_url()URLs are left alone, and the admin scripts add the flag to their REST requests themselves, so the deactivate or delete request also runs in safe mode.Fixes #470
Changes
Evaluate_Functions.phpBefore:
After:
Why:
get_rest_url()always passes therestscheme toget_home_url(), so this one check covers plain, pretty and/index.php/permalinks. Matching on the path would miss some cases and catch normal pages that havewp-jsonin the path. All other site and admin links still carry the flag. I did not touchadd_safe_mode_query_var()itself, so this should not conflict with #570.restAPI.ts/useRestAPI.tsxNew
applySafeModerequest interceptor on the shared axios instance. When the page URL hassnippets-safe-mode, it adds it to the request URL with the existingaddQueryArg()helper, which handles the?rest_route=form too.Why: without it the REST request that deactivates or deletes a broken snippet would still run that snippet and fail with it.
admin-bar.tsbuildRequestUrl()now copies the flag from the page URL to the admin bar snippet list request, since that one uses its ownfetchand not the axios instance.Tests
test_rest_urls_are_unchanged_in_safe_modechecksrest_url()is unchanged with plain, pretty and index permalinks. It fails for all three on the old code.test_site_and_admin_urls_carry_the_safe_mode_query_varchecks normal home and admin links still get the flag, including a page path that containswp-json.Testing
Test 1: Remove a crashing snippet with per-page safe mode
add_action( 'init', function () { undefined_function_for_test(); } );. The site shows a critical error.wp-admin/admin.php?page=snippets&snippets-safe-mode=1.Result: it works, and the request URL is
/wp-json/code-snippets/v1/snippets/<id>?snippets-safe-mode=1. The site loads again without the query var.Test 2: Plain and index permalinks
/index.php/%postname%/.Result: works. Plain gives
/index.php?rest_route=%2Fcode-snippets%2Fv1%2Fsnippets%2F<id>&snippets-safe-mode=1.Test 3: No regressions
snippets-safe-mode=1.snippets-safe-mode.CODE_SNIPPETS_SAFE_MODEconstant still works as before.Result: works as expected.
PHPUnit (359 tests),
lint:phpandlint:jsall pass.