Make WordPress Core

Opened 6 months ago

Last modified 6 weeks ago

#60208 new enhancement

Prevent redirect loops

Reported by: kkmuffme's profile kkmuffme Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch dev-feedback has-unit-tests
Focuses: Cc:

Description

There are all kinds of issues open that deal with redirect loops and their patches try to resolve the symptom.

I propose to instead fix the root cause by checking in wp_redirect/wp_safe_redirect if the $location is equal to the current host/request uri and if that's the case to not execute the redirect but wp_die - just like it's done in those function if response code is not in the 3xx range.

If a rogue plugin causes this, this will at least give you a hint of what was happening/where to start, when your WP page suddenly starts to reload indefinitely.

Change History (11)

This ticket was mentioned in PR #6017 on WordPress/wordpress-develop by @kkmuffme.


5 months ago
#1

  • Keywords has-patch added

Prevent redirect loops when trying to redirect to the current URL, except when changing http to https

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


4 months ago

#3 @swissspidy
4 months ago

  • Keywords needs-unit-tests needs-testing added

WordPress server setups can be vastly different (think reverse proxies, containers, headless setups, etc.). This seems like a test that requires a good amount of testing to ensure this won't break anything.

#4 @kkmuffme
4 months ago

Testing for what specifically? The $_SERVER superglobals?

If they aren't set, then the code that was added by the PR will not do anything and it will behave as before. It will only change the behavior if they're set.

#5 @swissspidy
4 months ago

  • Keywords dev-feedback added

We would want to verify that this code does what it promises to do, right, so this can be tested with unit tests but also with manual testing.

#6 @kkmuffme
4 months ago

so this can be tested with unit tests

Not possible, since unit tests run with PHP-CLI which does not populate INPUT_SERVER (see PHP core for why)

#7 @kkmuffme
4 months ago

Which is the reason why wp_redirect does not have any unit tests in the first place - impossible to test a redirect in CLI, isn't it?

#8 @kkmuffme
4 months ago

I will separate it out into a function and modify it, so it will just use the superglobals if sapi === CLI
filter_input function doesn't exist, so we'll have a unit test.

Will take care of that tomorrow.

#9 @kkmuffme
3 months ago

  • Keywords has-unit-tests added; needs-unit-tests needs-testing removed

Found a way to test it (or at least the error case where it should not redirect), and removed FILTER_INPUT since that is moved to a separate ticket, since it affects lots of places anyway.

Failing tests are unrelated. It's ready for a review and merge.

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


3 months ago

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


6 weeks ago

Note: See TracTickets for help on using tickets.