WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 14 months ago

#15518 closed defect (bug) (fixed)

Exit After wp_redirect

Reported by: filosofo Owned by: filosofo
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

We should end code execution after calling wp_redirect, except in those rare cases where code execution is meant to occur after sending the headers, such as in cron and plugin activation check.

Otherwise, we risk unexpected behavior and unseen results in some cases.

Attachments (3)

exit-after-redirects.15518.diff (13.0 KB) - added by filosofo 3 years ago.
exit-after-redirects.15518.2.diff (11.7 KB) - added by filosofo 3 years ago.
plugin-editor.15518.diff (622 bytes) - added by duck_ 3 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 nacin3 years ago

  • Keywords needs-refresh added

comment:2 filosofo3 years ago

  • Keywords needs-refresh removed

Patch refreshed.

comment:3 nacin3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [16847]) Always exit after wp_redirect. props filosofo, fixes #15518.

comment:4 aaroncampbell3 years ago

Would it make more sense to add the exit to the end of the wp_redirect function? I haven't checked out the cases where we need to execute code after the redirect, but maybe we could make an optional parameter that defaults to true and pass false in the cases where we DON'T want to exit?

The advantage is that this will also fix the possible "unexpected behavior" for many plugins that use it.

duck_3 years ago

comment:5 duck_3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[16847] removed the exit after a redirect in plugin-editor.php. This results in active plugins not being reactivated automatically after editing.

comment:6 nacin3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [17114]) Restore an exit we need. props duck_, fixes #15518.

comment:7 filosofo3 years ago

Whoops. Thanks for catching that.

comment:8 hakre3 years ago

Related: #15965

comment:9 iandunn2 years ago

  • Cc ian_dunn@… added

I don't understand why this was closed as "fixed" when wp_redirect() is still returning instead of exit()'ing, like aaroncampbell suggested. Am I missing something? Leaving it like this is a security vulnerability.

comment:10 nacin2 years ago

We use wp_redirect() in a number of locations where we deliberately want the page to continue processing.

A case could probably be made for a separate function for these, but nonetheless, a change in behavior with wp_redirect() would break compatibility with existing plugins and themes.

comment:11 iandunn2 years ago

Ok, I see your point about the backwards-compatibility issues. Maybe the best way forward would be to have wp_redirect() accept a bool parameter to determine whether or not it redirects, and have it be false by default. Then, create a new function like wp_redirect_exit(), which calls wp_redirect( true ), and then promote wp_redirect_exit() on the Codex and elsewhere as the function that plugins/themes should be using unless they actually need to continue processing. I can create a ticket for that if you think it's a good idea.

comment:12 nacin2 years ago

That's how I'd do it. Not sure if it'll catch on, but it is worth a ticket.

comment:13 iandunn2 years ago

Ok, I created #20060.

comment:14 juliobox14 months ago

  • Cc juliobosk@… added

comment:15 juliobox14 months ago

Just to tell, if you add a "die" or "exit" right after the "header" in wp_redirect, you can't activate any plugin now.
I just tested it in mutisite and monosite, using a mu-plugins and a plugged wp_redirect with die/exit before the last }
#my2cents

Note: See TracTickets for help on using tickets.