Amp-wp: With pretty permalinks enabled, adding /amp/ will break rewrite

Created on 14 Jan 2018  路  22Comments  路  Source: ampproject/amp-wp

When pretty permalinks enabled, AMP will add /amp/ to the url, but this will break the automatic rewrite rule when you set the site address different from the wordpress address.

My wordpress address is 'blog.oracle48.nl/wordpress', but in WP settings I have set 'blog.oracle48.nl' as site address. This will make 'blog.oracle48.nl/wordpress/post/' goto 'blog.oracle48.nl/post/' and works out of the (wordpress) box.

But whem /amp/ is added (by LinkedIn or other site), this rewrite does not work any more. 'blog.oracle48.nl/wordpress/post/amp/' does not get rewritten to 'blog.oracle48.nl/post/amp/' and visitors will be shown the 404 page.

Bug

All 22 comments

Hi @Airell, I quickly spinned up a test site and cannot reproduce the issue, the redirect works fine on my side even with WordPress installed in a subdirectory. Which method described in this article are you using?

_I am closing this for now, it may be reopened if more people are facing the same issue_

Hi @ThierryA,

I'm using method number 2, with URL change. I have copied .htaccess and the index.php and changed the root's index.php.

The next rewrite works:
http://blog.oracle48.nl/wordpress/oracle-linux-patches-for-meltdown-and-spectre-information/
Changes to:
http://blog.oracle48.nl/oracle-linux-patches-for-meltdown-and-spectre-information/

Now with AMP, the URL is:
http://blog.oracle48.nl/oracle-linux-patches-for-meltdown-and-spectre-information/amp/

But the rewrite from the previous example does not work when /amp/ is added to the url:
http://blog.oracle48.nl/_wordpress_/oracle-linux-patches-for-meltdown-and-spectre-information/amp/

I'm using the lastest Wordpress, my .htaccess is changed by Wordpress (step 11.) to:

# BEGIN WordPress
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteBase /
RewriteRule ^index\.php$ - [L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /index.php [L]
</IfModule>
# END WordPress

Regards, Airell.

Thanks for providing more details @Airell, I could reproduce the issue. I did some very quick research and the AMP query var is altering the logic in the redirect_canonical() function here.

Is there a resolution to this issue yet? I saw elsewhere guidance to change /amp/ to ?amp for the endpoint, but this still doesn't seem to resolve the issue for us.

What version of the plugin are you using? The amp slug is not used anymore. Now in paired mode, the AMP version of a page is accessed via the ?amp query parameter.

The /amp/ slug is still used in Classic mode. I assume that is the mode in question here?

I am having this issue as well. Visiting the WP > Settings > Permalinks and clicking the [Save Changes] button appears to fix it, as well as running the CLI rewrite flush command. However any time that flush_rewrite_rules(true); is ran (lets say with another plugin like WP Migrate Pro or with an automated process), the Permalinks for AMP pages are broken again.

I am using version (1.1.3). I can't change the slug from /amp/ to ?amp as there's no option to do so in my AMP settings (classic mode).

Aside: The latest version of the plugin is 1.2.0, not 1.1.3.

What happens if you force the use of ?amp instead of /amp/? See https://github.com/ampproject/amp-wp/issues/1148#issuecomment-388987756

I'm testing on my install in Reader mode and when I do wp rewrite flush the expected rewrite rules are present for the post post type

$ wp rewrite flush && wp rewrite list --source=post | grep amp
Success: Rewrite rules flushed.
| ([0-9]{4})/([0-9]{1,2})/([0-9]{1,2})/([^/]+)/amp(/(.*))?/?$                              | index.php?year=$matches[1]&monthnum=$matches[2]&day=$matches[3]&name=$matches[4]&amp=$matches[6]   | post   |
| ([0-9]{4})/([0-9]{1,2})/([0-9]{1,2})/amp(/(.*))?/?$                                      | index.php?year=$matches[1]&monthnum=$matches[2]&day=$matches[3]&amp=$matches[5]                    | post   |
| ([0-9]{4})/([0-9]{1,2})/amp(/(.*))?/?$                                                   | index.php?year=$matches[1]&monthnum=$matches[2]&amp=$matches[4]                                    | post   |
| ([0-9]{4})/amp(/(.*))?/?$                                                                | index.php?year=$matches[1]&amp=$matches[3]                                                         | post   |

~mkendall07~ @mkormendy Does the /amp/ URL show a 404 or just the regular post as if the endpoint was not added?

Issue may have been reported here as well: https://wordpress.org/support/topic/plugin-does-not-appear-to-work-2/

@westonruter not sure who mkendall07 is .. but maybe you mistyped. Sorry 1.1.3 is considered "latest" by my WordPress installation - it hasn't asked me to update the plugin to a newer version, 1.2.0 or otherwise. Spoke too soon, I've updated, tested, same issue.

I get 404 pages for both ?amp (forced) and /amp/.

Yes .. wp rewrite flush will fix rewrites for AMP as does [Save Changes] on the Permalinks page in the Admin Settings.

@mkormendy sorry, yes, autocomplete problem.

Please share the URL to your site.

Because I've had this issue for some time, I have created a cron job that runs the wp rewrite flush CLI command on a regular basis so you won't be able to see the issue. I'd have to turn it off. Let me set up an environment to test with and get back to you.

I'm confused, though. Why would adding ?amp show a 404? Adding a query param should have no impact on whether or not WordPress is able to successfully perform a query.

I lied again. So apparently ?amp does kick to the AMP page after all. Only /amp/ 404's out.

You can test right now for the next 3 minutes (after which point the cron job will flush rewrites via CLI and fix /amp/ rewrites):

https://www.smartstartinc.com/blog/easiest-ignition-interlock-device/?amp

https://www.smartstartinc.com/blog/easiest-ignition-interlock-device/amp/

Ok, so for an immediate workaround I suggest adding this code to force the query param as opposed to the endpoint:

add_filter( 'amp_pre_get_permalink', function( $pre, $post_id ) {
    return add_query_arg( amp_get_slug(), '', get_permalink( $post_id ) );
}, 10, 2 );

Here's a before and after.

before-flush-rewrites
after-flush-rewrites

I'll try the force, I may have to sit with our SEO Strategist to make a mark in her analytics if she gets changes in her expected attribution paths/urls.

What matters is the canonical URL. Whether using the query param or the endpoint for the paired AMP URL, the canonical URL stays the same.

Now, as for the underlying cause for why flushing rewrite rules doesn't work... I'm guessing the cause is a plugin calling flush_rewrite_rules() too early, before AMP has had a chance to register it's rewrite endpoint.

For sure on the canonical, I do know that she has some specific tags/triggers set up for attributing our efforts in adding AMP to the site in the first place. As for the flush_rewrite_rules() firing too early, the process that triggers that and breaks it has to do with WP Migrate Pro .. which I use for automating the migration of specific tables, forms and content from our staging environment to live and vice versa.

I'm going to close this in favor of #2204, where we should move to using ?amp query param instead of using the /amp/ endpoint. We already only use ?amp for Transitional mode, and ?amp is used in Reader mode when viewing a page. So using ?amp everywhere will be more consistent and there won't be headaches with rewrite rules.

Thanks @westonruter!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

maciejmackowiak picture maciejmackowiak  路  5Comments

westonruter picture westonruter  路  5Comments

miina picture miina  路  5Comments

miina picture miina  路  3Comments

swissspidy picture swissspidy  路  5Comments