Make WordPress Core

Opened 7 weeks ago

Last modified 3 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 needs-unit-tests needs-testing dev-feedback
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 (8)

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


3 weeks 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.


3 weeks ago

#3 @swissspidy
3 weeks 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
3 weeks 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
3 weeks 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
3 weeks 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
3 weeks 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
3 weeks 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.

Note: See TracTickets for help on using tickets.