Skip to content

fix: keep REST URLs intact in per-page safe mode - #596

Open
faisalahammad wants to merge 1 commit into
codesnippetspro:core-betafrom
faisalahammad:fix/470-safe-mode-rest-urls/core
Open

faisalahammad wants to merge 1 commit into
codesnippetspro:core-betafrom
faisalahammad:fix/470-safe-mode-rest-urls/core

Conversation

@faisalahammad

Copy link
Copy Markdown

Summary

Per-page safe mode (?snippets-safe-mode=1) added the query var to every home_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.php

Before:

add_filter( 'home_url', [ $this, 'add_safe_mode_query_var' ] );

After:

add_filter( 'home_url', [ $this, 'add_safe_mode_query_var_to_home_url' ], 10, 3 );

public function add_safe_mode_query_var_to_home_url( $url, $path = '', $orig_scheme = null ): string {
	if ( 'rest' === $orig_scheme ) {
		return is_string( $url ) ? $url : '';
	}

	return $this->add_safe_mode_query_var( $url );
}

Why: get_rest_url() always passes the rest scheme to get_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 have wp-json in the path. All other site and admin links still carry the flag. I did not touch add_safe_mode_query_var() itself, so this should not conflict with #570.

restAPI.ts / useRestAPI.tsx

New applySafeMode request interceptor on the shared axios instance. When the page URL has snippets-safe-mode, it adds it to the request URL with the existing addQueryArg() 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.ts

buildRequestUrl() now copies the flag from the page URL to the admin bar snippet list request, since that one uses its own fetch and not the axios instance.

Tests

  • test_rest_urls_are_unchanged_in_safe_mode checks rest_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_var checks normal home and admin links still get the flag, including a page path that contains wp-json.

Testing

Test 1: Remove a crashing snippet with per-page safe mode

  1. Set permalinks to Post name.
  2. Activate a snippet like add_action( 'init', function () { undefined_function_for_test(); } );. The site shows a critical error.
  3. Open wp-admin/admin.php?page=snippets&snippets-safe-mode=1.
  4. Deactivate or delete the snippet from the list.
    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

  1. Repeat Test 1 with Plain permalinks, then with /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

  1. On a safe mode page, check that admin menu links, Edit links and Visit Site still carry snippets-safe-mode=1.
  2. Open the admin bar Snippets menu. The lists load.
  3. Without safe mode, toggle, edit, save and delete snippets. No request has snippets-safe-mode.
  4. CODE_SNIPPETS_SAFE_MODE constant still works as before.
    Result: works as expected.

PHPUnit (359 tests), lint:php and lint:js all pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot remove a snippet when activating it crashes wp-admin

1 participant