Skip to content

Ensure that validation query vars persist through redirects#4544

Merged
westonruter merged 1 commit intodevelopfrom
fix/validating-redirected-urls
Apr 7, 2020
Merged

Ensure that validation query vars persist through redirects#4544
westonruter merged 1 commit intodevelopfrom
fix/validating-redirected-urls

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Apr 7, 2020 •
edited by schlessera
Loading

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
@westonruter westonruter requested a review from schlessera April 7, 2020 06:01
@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 7, 2020
@westonruter
Ensure that validation query vars persist through redirects (#4544)
d875e92
westonruter added a commit that referenced this pull request Apr 10, 2020
@westonruter
Merge branch 'develop' of github.com:ampproject/amp-wp into add-css-c…
daa92ca 
…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
Copy Markdown
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
@westonruter
Merge tag '1.5.3'
4e4195e 
* 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

Reviewers

@schlessera schlessera schlessera approved these changes

Assignees

No one assigned

Labels

Bug Something isn't working cla: yes Signed the Google CLA Developer Tools

Projects

None yet

Milestone

v1.5.3

Development

Successfully merging this pull request may close these issues.

4 participants

@westonruter @pierlon @schlessera @googlebot