Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#50976 closed defect (bug) (wontfix)

WordPress stripping out "page" query parameter with 301 redirect

Reported by: aleksihulkkonen's profile aleksihulkkonen Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Rewrite Rules Keywords:
Focuses: Cc:

Description

This is a follow-up to #45337.

I run a site and we use "page" query parameter to switch between views on that specific page. So we have for example:

www.example.com/example?page=offers&id=xxx

After upgrading to 5.5 the page parameter is being dropped from the url with 301 redirect and becomes:

www.example.com/example?id=xxx

This was breaking our site, but we downgraded to 5.4.2. I would like to install 5.5 but this is breaking the site. I saw ticket #45337 related to 5.5 and it seemed like a possible culprit.

Any ideas what is happening?

Change History (29)

#1 @johnbillion
4 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to 5.5.1

Thanks for the report @aleksihulkkonen .

Moving to 5.5.1 for investigation.

#2 @Bowromir
4 years ago

I can confirm that we're running into the same issue for one of our plugins that uses the ?page query parameter, breaking navigation and the loading of custom templates inside our plugin.

#3 @geowrge
4 years ago

This is pretty major due to the fact that many themes/plugins are using this query param. Clients are going nuts as we speak.

#4 @obroin
4 years ago

This has broken our plugin and themes too which runs loads of customers sites. To echo the sentiment, clients are going nuts

#5 @studioscully
4 years ago

I can confirm this has also broken in one of our sites - when attempting to view any page other than the first via a custom knowledge base plugin, the "page" query param is removed from the URL meaning users can't navigate past the first page of results.

#6 @SergeyBiryukov
4 years ago

Hi there, welcome to WordPress Trac! Thanks for the report.

Either [47727] / #40773 or [47760] / #45337 indeed would be the possible culprit here. I'm sorry to hear this is causing issues since the latest update.

That said, page is one of the reserved query vars in WordPress that has a very specific usage for pagination, it's used to display parts of a post separated with the <!--nextpage--> tag, on a URL like /example/123/.

It should only have a numeric value, and is not meant to be used for any other purpose, e.g. ?page=offers&id=xxx.

So I would recommend changing this variable in any custom code to some other name to avoid conflicts.

As another possible solution, you can disable the redirect_canonical() function for these requests, that is attached to the template_redirect action by default.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#7 @slickremix
4 years ago

I can also confirm the pagination is not working properly after 5.5 update. Although I am not getting a 301 redirect but rather the page just reloads with no new page in the url. Example: https://feedthemgallery.com/gallery-demo-four/

<?php
$pagination_counts = paginate_links(
            array(
                'base'      => add_query_arg( 'page', '%#%' ),
                'format'    => '?page=%#%',
                'current'   => max( 1, get_query_var( 'page' ) ),
                'mid_size'  => 3,
                'end_size'  => 3,
                'prev_text' => __( '&#10094;' ),
                'next_text' => __( '&#10095;' ),
                'total'     => ceil( $total_pagination_count / $per_page ), // 3 items per page
            )
        );
Last edited 4 years ago by slickremix (previous) (diff)

#8 follow-up: @paperkawaii
4 years ago

After searching found slickremix's post elsewhere as I am having the same issue. No redirect just reloads the first page. Manually visiting the page/2 etc gave a 404 error.
Have downgraded for now.

#9 @SergeyBiryukov
4 years ago

#51033 was marked as a duplicate.

#10 @alexfurr
4 years ago

Same problem for me using a custom theme. Have downgraded for now which works and will rebuild the theme using 'mypage=' instead of 'page=' in the query string.
I was surprised this change wasn't documented in the release notes as it's had a fairly significant effect.

#11 @mikeyott
4 years ago

For what it's worth, we're also experiencing the same issue on more than one site that I have noticed so far, except we get a 404.

I'm sharing this info in case it helps others. Our custom theme is using the add_rewrite_rule() function which does indeed reference page= in the query string.

Rolling back to WordPress 5.4.2 'solves' the problem.

Kudos for acknowledging potential problem. Curious to see if the core team wants to fix this, or if we have to end up implementing our own solution.

#12 in reply to: ↑ 8 @cegomez
4 years ago

I'm having same issue at some sites, when I try to access /category-slug/page/2 it redirects me (301) to /category-slug/2 and get a 404.
Downgrade fix it too.

Last edited 4 years ago by cegomez (previous) (diff)

#13 @slickremix
4 years ago

Any time frame as to when this will be resolved? It's been one hectic week of answering support tickets :)

#14 @Otto42
4 years ago

@slickremix It very likely will not be resolved. You need to change your plugin or theme to not use "page" as a query parameter in URLs. It wasn't valid before, and it is not valid now.

WordPress uses "page". Your add-ons, therefore, can't use it.

#15 @paperkawaii
4 years ago

I have changed the URL parameter to

'format' => '?mypage=%#%',

Which works fine in 5.4.2 however makes no difference to the problem in 5.5.

It still gives a 404 error (after disabling the redirect_canonical)

Therefore I don't understand the previous comments about rebuilding without the page= in the URL.

I'm splitting posts into multiple pages. Not using the 'next page' function as I have a completely custom theme with custom splits for each post, integrated with Advanced Custom Fields gallery.. which splits at a different image depending on how many images there are..

#16 @oglekler
4 years ago

I tried 'rewrite_rules_array' filter to make custom permalinks for user-friendly URLs and it works fine.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


4 years ago

#18 @johnbillion
4 years ago

#51128 was marked as a duplicate.

#19 @audrasjb
4 years ago

  • Keywords 2nd-opinion removed
  • Milestone 5.5.1 deleted
  • Resolution set to wontfix
  • Status changed from new to closed
  • Version 5.5 deleted

As per yesterday’s bug scrub, it appears this issue is related to a wrong usage of a reserved term.

Indeed, the page query var has a very specific usage in core, and there's no way for core to see if any custom code is also using it for some other purpose.

Those using it have an option to disable redirect_canonical() for those custom requests, or to rename the variable.

We understand that it may be annoying for themes and plugins authors to fix their usage of this variable, but it would introduce a way more issues if it was changed on WordPress Core side. It's objectively better to get this fixed on plugins and themes since it is a wrong usage of WordPress variables.

Closing this as wontfix, as the issue shouldn't be fixed on WordPress core but rather on plugins and themes side.

Feel free to ask for further details, discussion can happens even on closed tickets :-)

#20 @leec87
4 years ago

  • Resolution wontfix deleted
  • Severity changed from normal to critical
  • Status changed from closed to reopened

Hi, this issue needs to be reversed because I haven't changed any of the query vars @audrasjb, but I'm using the page number as a PIN to display different data on the page, however when I have a URL such as /page/1234/ (or any number), this is being redirected to /page/ and the website is therefore broken because it's not showing the relevant info (based on the pin entered)

This was working perfectly fine before, please reverse this. How else can I fix this bug? Why is this removing the page number from the URL?

Last edited 4 years ago by leec87 (previous) (diff)

#21 follow-up: @audrasjb
4 years ago

  • Resolution set to wontfix
  • Severity changed from critical to normal
  • Status changed from reopened to closed

Hi,
You can use any variable name you want, but page is a reserved term because it's used for pagination. It shouldn't be used for any other purpose. The fact it previously used to work was undocumented and really unexpected. WordPress can't maintain undocumented use cases :)

To make it work again, you'll just have to use another var name and update the rewrite rules accordingly.

#22 in reply to: ↑ 21 @leec87
4 years ago

  • Resolution wontfix deleted
  • Severity changed from normal to major
  • Status changed from closed to reopened

Replying to audrasjb:

Hi,
You can use any variable name you want, but page is a reserved term because it's used for pagination. It shouldn't be used for any other purpose. The fact it previously used to work was undocumented and really unexpected. WordPress can't maintain undocumented use cases :)

To make it work again, you'll just have to use another var name and update the rewrite rules accordingly.

You’re not listening, I haven’t used any query or anything. I’m just using the pagination feature of WordPress, but whatever update this has done, has now made all page number URLs redirect to the first page, which isn’t what it should do.

This redirection from /pagename/[any page number] to /pagename/ needs to be reverted.

No one has explained why this has been changed yet and how it benefits. It’s just breaking lots of websites that use it (WordPress is at fault here).

#23 @Otto42
4 years ago

  • Resolution set to wontfix
  • Severity changed from major to normal
  • Status changed from reopened to closed

@leec87 The redirection is not going to be reverted as it is for valid and important reasons. Please stop reopening the ticket.

Also, to be clear, your problem is different than the issue in this ticket.

For the case of a multi-page post, where the nextpage tag is used to split the page into multiple pages, then WordPress has always used page numbers in the URL, such as /slug/1/, /slug/2/ and so on.

WordPress recognizes that /slug/number combination in its rewrite rules by default, and assigns pagename=slug and page=number in the internal query.

The change to where /slug/1234/ gets redirected to /slug/ only happens where there is no page number of 1234. Previous versions would just show the first page on the wrong URL, which was undesirable behavior for SEO purposes.

Now, if you happened to use that undocumented behavior to get that number from the URL and do something else with it, then you were relying on a side-effect of how WordPress worked. It was not intended to work that way, and it should not be used that way. If you want to use it in that fashion, then you should change your code to either remove the existing rewrite rule, or to add your own rewrite rule for your page, in order to specifically handle that type of behavior.

This ticket was mentioned in Slack in #forums by joyously. View the logs.


4 years ago

#25 @sabernhardt
4 years ago

#51472 was marked as a duplicate.

#26 @silentz0r
4 years ago

Beginning of rant

It's ridiculous that this is not mentioned in any changelog but developers have to dig deep and end up on a thread where this is mentioned on the side. It is things like this one that give WordPress the "anti-developer" reputation that it has.

I understand this is not something that's going to be changed and I have no problem with that, but not mentioning it on any changelog and also trying to pass it as "something undocumented that shouldn't have worked" is ridiculous. Wordpress is one of the main web drivers and should handle such things in a better way. It is very unprofessional to dismiss this issue.

Now, onto specifics. The term "page" is most likely the most common term used in web development. Writing an MVC application, if you are writing a controller and want to pass the name/slug of the page you want to access, you most likely are going to end up using the query parameter "page". Reserving such a common term is bad, let alone doing it silently. Since the query parameter is not even visible by the end user and is stripped before the 301 redirect, Wordpress could have used something like "_page", "wp_page" or even have a standard for reserved query parameters (e.g. all parameters starting with "wp_"). Reserving one, if not the, most common terms as a reserved parameter is bad.

It's insane that you do not want to understand that you broke a lot of plugins and websites and that this is not the way to go about things. I wouldn't be surprised if the people who made the change for 5.5 were even aware that the "page" query parameter worked the way it did. And of course my crappy plugins that I only use myself are irrelevant to Wordpress, but I'm sure famous plugins will eventually suffer from this as well.

End of rant

Please provide a git commit hash or line of code that shows how this "undocumented behavior" was changed in 5.5 so we can shed some light into this.

#27 follow-up: @Otto42
4 years ago

WordPress could have used something like "_page", "wp_page" or even have a standard for reserved query parameters (e.g. all parameters starting with "wp_"). Reserving one, if not the, most common terms as a reserved parameter is bad.

Sure, except that WordPress has used 'page' for this purpose for over 16 years now. You can see where it was originally added here:

https://core.trac.wordpress.org/changeset/2535

As for what changed, well, 'page' was expected to be a numeric parameter. It has been reserved for such a purpose for a very long time now.

Until 5.5, if it wasn't a valid page number, then it was essentially ignored and left as is. However, this resulted in page=1234 (where 1234 or anything else there was not a valid page number) being ignored and thus resulting in the page content of page 1 showing, just as if it wasn't there are all.

However, this means that links like http://localhost/pagename/1234 (and anything else in place of 1234) would show the same content. For SEO purposes, this is obviously not a good thing to do.

So, now it checks if the page number is an invalid one, and if so, it redirects to the correct URL for the page content. Thus avoiding duplicate content issues.

If you were using the reserved 'page' keyword for another purpose, then you essentially got away with it because the invalid values you were sending it were ignored. They should not have been ignored, and now they are not. Sorry that this inconveniences you, however if having the keyword 'page' be reserved and used in the query parameters for over 16 years isn't long enough, then I don't really know what a better answer would be.

#28 in reply to: ↑ 27 ; follow-up: @silentz0r
4 years ago

Replying to Otto42:

WordPress could have used something like "_page", "wp_page" or even have a standard for reserved query parameters (e.g. all parameters starting with "wp_"). Reserving one, if not the, most common terms as a reserved parameter is bad.

Sure, except that WordPress has used 'page' for this purpose for over 16 years now. You can see where it was originally added here:

https://core.trac.wordpress.org/changeset/2535

As for what changed, well, 'page' was expected to be a numeric parameter. It has been reserved for such a purpose for a very long time now.

Until 5.5, if it wasn't a valid page number, then it was essentially ignored and left as is. However, this resulted in page=1234 (where 1234 or anything else there was not a valid page number) being ignored and thus resulting in the page content of page 1 showing, just as if it wasn't there are all.

However, this means that links like http://localhost/pagename/1234 (and anything else in place of 1234) would show the same content. For SEO purposes, this is obviously not a good thing to do.

So, now it checks if the page number is an invalid one, and if so, it redirects to the correct URL for the page content. Thus avoiding duplicate content issues.

Thanks for a detailed answer. Could you also provide the change in 5.5 that ensures the numerical check?

If you were using the reserved 'page' keyword for another purpose, then you essentially got away with it because the invalid values you were sending it were ignored. They should not have been ignored, and now they are not. Sorry that this inconveniences you, however if having the keyword 'page' be reserved and used in the query parameters for over 16 years isn't long enough, then I don't really know what a better answer would be.

Yes, WordPress made (in my opinion) a bad call in reserving one of the most popular words and it has reserved it for over 15 years. But the reservation was buggy and it led into people using that word for those 15 years in a plethora of plugins, and it was silently fixed in a minor Wordpress release and not mentioned in the changelog resulting into broken websites everywhere. Don't you think things like this should be mentioned in the patch notes?

#29 in reply to: ↑ 28 @audrasjb
4 years ago

Replying to silentz0r:

Thanks for a detailed answer. Could you also provide the change in 5.5 that ensures the numerical check?

Here is the changeset that shipped in WordPress 5.5: [47760]

It was silently fixed in a minor WordPress release and not mentioned in the changelog resulting into broken websites everywhere. Don't you think things like this should be mentioned in the patch notes?

Just noting that WP 5.5 is a major release of WordPress (and not a minor one), and also that as far as I know, the change did not result into "broken websites everywhere". We had a couple of reported issues/tickets, but I don't have any large plugin in mind that reported severe issues with this particular change. BTW, we are really sorry to hear you experienced issues with WP 5.5.

Note: See TracTickets for help on using tickets.