WordPress.org
Get WordPress

Make WordPress Core

Opened 19 years ago

Closed 16 years ago

Last modified 11 months ago

#2860 closed defect (bug) (wontfix)

Using wp_redirect when headers are sent

Reported by: robmiller's profile robmiller Owned by: robmiller's profile robmiller
Milestone: Priority: normal
Severity: normal Version: 2.1
Component: General Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Currently wp_redirect() redirects by sending a Refresh/Location header, making it unusable when the headers have already been sent (such as on an admin page).

This patch checks if the headers are sent and if so echos a meta refresh instead. What do people think about this?

Attachments (1)

2860.diff​ (579 bytes) - added by robmiller 19 years ago.

Download all attachments as: .zip

Change History (14)

@robmiller
19 years ago

#1 @robmiller
19 years ago

  • Keywords bg|has-patch bg|2nd-opinion added
  • Owner changed from anonymous to robmiller
  • Status changed from new to assigned

#2 @ryan
19 years ago

Right now we only call wp_redirect() when redirecting after sending a cookie. wp_redirect() uses Refresh to work-around an IIS Set-Cookie bug. Does headers_sent() evaluateto true if cookies have been sent? If so, does http-equiv='refresh' workaround the IIS set cookie bug? Just want to make sure we don't break IIS users.

#3 @robmiller
19 years ago

I'm not sure and I'm afraid I don't have IIS to test. I'm pretty sure cookies will definitely be set, though, since it's making an entirely new request and not simply being redirected á la a Header('Location: '); call.

#4 @skeltoac
19 years ago

​This comment has the answer. It won't hurt anyone to patch this though. It'll only leave some IIS users right where they already are because it's a false negative (false returned falsely) which mimics prior logic.

However, Meta tags don't belong anywhere but the head section and will invalidate xhtml if they appear elsewhere. I have tested this on the validator.w3.org

#5 @RuddO
19 years ago

Doesn't meta refresh only work in the head tag?

#6 @robmiller
19 years ago

It works everywhere but is only valid in the head.

#7 @foolswisdom
18 years ago

  • Keywords has-patch 2nd-opinion added; bg|has-patch bg|2nd-opinion removed
  • Milestone changed from 2.0.4 to 2.2

#8 @tombarta
18 years ago

If the page is already expecting to redirect, it should not have any content displayed, and it would seem pretty easy just to write a valid HTML document along the lines of

<html>
  <head><meta refresh-magic-syntax></head>
  <body>
    <p>Sorry, automatic redirection failed.
    You should <a href="wherever-we-are-redirecting">continue here</a>.
  </body>
</html>

That way, you get Javascript redirection as a fallback plus a plain old link as a secondary fallback.

#9 @foolswisdom
18 years ago

  • Milestone changed from 2.2 to 2.3

#10 @ryan
17 years ago

  • Milestone changed from 2.3 to 2.4 (next)

#11 @pishmishy
17 years ago

  • Milestone changed from 2.5 to 2.6

Bumping for 2.5 feature freeze.

#12 @Denis-de-Bernardy
16 years ago

  • Milestone 2.9 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

this has been open for 3 years without a commit.

if the current implementation was dysfunctional in genuine use cases, it would have been reported over and over again.

and the suggested patch would hinder debugging efforts on post requests.

#13 @hakre
15 years ago

There is a partial implementation of this done in [13167] / #12089.

Note: See TracTickets for help on using tickets.