Opened 3 years ago
Last modified 15 months ago
#53362 accepted defect (bug)
Invalid paginated requests not treated as such
Reported by: | daleharrison | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 5.7.2 |
Component: | Canonical | Keywords: | has-unit-tests needs-refresh |
Focuses: | Cc: |
Description
This is a follow-up to #40773.
The security department that scans our WordPress websites identified the issue resolved in #40773 as cross-site scripting, as our themes add the webpage's precise URL to Open Graph data (og:url
) in the <head>
.
So, we were happy to see the resolutions applied in that ticket. Unfortunately, while testing, we found that adding /0/
to any URL is still possible, e.g. https://example.com/about-us/0/ does not redirect back to the canonical https://example.com/about-us/.
Additionally, we have found that it is possible to append /page/
followed by a number to the URL of a page that does not support pagination, e.g. https://example.com/about-us/page/0/, https://example.com/about-us/page/12345/, etc.
In the latter example, the <title>
also changes from "About Us" to "About Us – Page 12345" as WordPress seems to think this is a valid paginated page. The paged-12345
and page-paged-12345
classes are also added to the <body>
. These specific tests were done using the Twenty Twenty-One theme.
Furthermore, appending /page/0/
to the page that displays blog posts does not trigger a 404 or a redirect, e.g. https://example.com/blog/ is "identical" to https://example.com/blog/page/0/.
Attachments (1)
Change History (17)
#2
@
3 years ago
Hi there, welcome back to WordPress Trac! Thanks for the report.
Additionally, we have found that it is possible to append
/page/
followed by a number to the URL of a page that does not support pagination, e.g. https://example.com/about-us/page/0/, https://example.com/about-us/page/12345/, etc.
Just noting that this part of the ticket is already being tracked in #28081.
#4
@
2 years ago
- Keywords has-patch added
- Milestone changed from Awaiting Review to 6.1
- Owner set to SergeyBiryukov
- Status changed from new to accepted
#5
follow-up:
↓ 7
@
2 years ago
@SergeyBiryukov Is this ticket on your radar for 6.1?
As we change test_redirect_canonical_with_nextpage_pagination
test is it good to use ticket number for the this test?
This ticket was mentioned in PR #3388 on WordPress/wordpress-develop by SergeyBiryukov.
2 years ago
#6
- Keywords has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/53362
#7
in reply to:
↑ 5
@
2 years ago
- Milestone changed from 6.1 to 6.2
Replying to mukesh27:
As we change
test_redirect_canonical_with_nextpage_pagination
test is it good to use ticket number for the this test?
Thanks! As these tests cover multiple tickets, the ticket number is included in the data provider and passed to assertCanonical()
as the third parameter.
It looks like this needs some more work, as the new test cases pass with this patch, but some older rewrite tests fail. Moving to 6.2 for now.
This ticket was mentioned in Slack in #core by costdev. View the logs.
20 months ago
20 months ago
#9
Hi @SergeyBiryukov, could you merge/rebase on trunk
and push to the PR so we can look into the test failures? 🙂
This ticket was mentioned in Slack in #core by costdev. View the logs.
20 months ago
#11
@
20 months ago
- Milestone changed from 6.2 to 6.3
This ticket was discussed in the bug scrub and it was agreed to move this ticket to 6.3.
This ticket was mentioned in Slack in #core by oglekler. View the logs.
16 months ago
@mukesh27 commented on PR #3388:
16 months ago
#13
@SergeyBiryukov Can you please update PR and look into test failures?
#14
@
16 months ago
- Keywords needs-refresh added; has-patch removed
This ticket was discussed in the recent bug scrub.
The PR needs some work as some unit tests are failing.
Prop to @oglekler
Related #29587, #29599, #29655, #40836, #40848