Make WordPress Core

Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#15518 closed defect (bug) (fixed)

Exit After wp_redirect

Reported by: filosofo's profile filosofo Owned by: filosofo's profile 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 14 years ago.
exit-after-redirects.15518.2.diff (11.7 KB) - added by filosofo 14 years ago.
plugin-editor.15518.diff (622 bytes) - added by duck_ 14 years ago.

Download all attachments as: .zip

Change History (18)

#1 @nacin
14 years ago

  • Keywords needs-refresh added

#2 @filosofo
14 years ago

  • Keywords needs-refresh removed

Patch refreshed.

#3 @nacin
14 years ago

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

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

#4 @aaroncampbell
14 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.

#5 @duck_
14 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.

#6 @nacin
14 years ago

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

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

#7 @filosofo
14 years ago

Whoops. Thanks for catching that.

#8 @hakre
14 years ago

Related: #15965

#9 @iandunn
13 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.

#10 @nacin
13 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.

#11 @iandunn
13 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.

#12 @nacin
13 years ago

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

#13 @iandunn
13 years ago

Ok, I created #20060.

#14 @juliobox
12 years ago

  • Cc juliobosk@… added

#15 @juliobox
12 years 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.