Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that validation query vars persist through redirects #4544

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Apr 7, 2020

Summary

Given a plugin that performs a redirect, such as the contrived example which redirects any AMP page to automatically get redirected=1 added as an additional query var:

add_action(
	'template_redirect',
	function () {
		if ( function_exists( 'is_amp_endpoint' ) && is_amp_endpoint() && ! isset( $_GET['redirected'] ) ) {
			$url = wp_unslash( $_SERVER['REQUEST_URI'] );
			$url = add_query_arg( 'redirected', '1', $url );
			$url = remove_query_arg( [ 'amp_validate', 'amp_cache_bust' ], $url );
			wp_redirect( $url );
			exit;
		}
	}
);

At the moment for a site in Transitional mode, going to a non-AMP page at /about/ and clicking Validate in the admin bar results in:

image

The issue is that the amp_validate query var was not persisting across redirects, so this PR fixes that problem so that this is now the result when attempting to validate /about/ on a Transitional mode site:

image

This issue came up specifically where WPML was attempting to redirect the homepage to /en/ in this support topic: https://wordpress.org/support/topic/url-validation-failed-due-to-unexpected-json-in-amp-validation-response/#post-12637035

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added Bug Something isn't working Developer Tools labels Apr 7, 2020
@westonruter westonruter added this to the v1.5.3 milestone Apr 7, 2020
@googlebot googlebot added the cla: yes Signed the Google CLA label Apr 7, 2020
@westonruter westonruter merged commit 29405a0 into develop Apr 7, 2020
@westonruter westonruter deleted the fix/validating-redirected-urls branch April 7, 2020 14:30
westonruter added a commit that referenced this pull request Apr 10, 2020
…aching-reenable-button

* 'develop' of github.com:ampproject/amp-wp:
  Restore unification of multi-page post content in Reader mode (#4547)
  Prevent styles from being removed when in Customizer preview with Standard mode (#4553)
  Omit Jetpack from being activated during PHPUnit test runs
  Use title case for Paired Browsing link in edit post screen (#4540)
  Ensure that validation query vars persist through redirects (#4544)
  Update dependency babel-jest to v25.2.6 (#4510)
  Update dependency css-loader to v3.5.0 (#4537)
  Update dependency autoprefixer to v9.7.6 (#4539)
  Add requirements to plugin file header (#4543)
  Force status code of validation responses to be 200 (#4533)
  Update optimizer test specs (#4527)
  Bump stable tag to 1.5.2
  Cache response status and headers when fetching external stylesheets (#4509)
  Fix securing multi-line mustache templates (#4521)
  Add CSS monitoring time series to Site Health debugging info (#4519)
  Update hostname used for WordPress TV embeds to fix external HTTP requests (#4524)
  Mock Imgur embed tests
  Mock Facebook embed tests
  Standardize file and class names for embed handlers
@pierlon
Copy link
Contributor

pierlon commented Apr 14, 2020

Verified in QA; the query vars amp_validate and amp_cache_bust are persisted when redirecting.

westonruter added a commit that referenced this pull request Apr 15, 2020
* tag '1.5.3':
  Bump 1.5.3
  Bump version to 1.5.3-RC1
  Fix handling of Mustache templates (#4583)
  Stub request based on test scenario (#4588)
  Update tests after block-library/style.css changes in Gutenberg 7.9 (#4579)
  Restrict doing plugin upgrade routine when not in admin (#4538)
  Add new accessibility sanitizer (#4535)
  Fix unit tests (#4564)
  Add button into Site Health to reenable CSS transient caching (#4522)
  Restore unification of multi-page post content in Reader mode (#4547)
  Prevent styles from being removed when in Customizer preview with Standard mode (#4553)
  Omit Jetpack from being activated during PHPUnit test runs (#4474)
  Mock Facebook embed tests (#4474)
  Mock Imgur embed tests (#4474)
  Use title case for Paired Browsing link in edit post screen (#4540)
  Ensure that validation query vars persist through redirects (#4544)
  Add requirements to plugin file header (#4543)
  Force status code of validation responses to be 200 (#4533)
  Update optimizer test specs (#4527)
  Bump 1.5.3-alpha
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working cla: yes Signed the Google CLA Developer Tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants