Opened 17 years ago
Closed 11 years ago
#7361 closed defect (bug) (wontfix)
Fixes for wp-app with PHP-CGI
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.9 |
Component: | AtomPub | Keywords: | has-patch 2nd-opinion |
Focuses: | Cc: |
Description
I was trying to use Atom Publishing Protocol with my blog I a have found some quirks and bugs.
My blog is hosted at Dreamhost, and, as far as I know, is using Apache 2.0.61 and PHP 5.2.6 as CGI. My first problem was, obviously, the authentication, but neither the solutions from the Codex or this other blog post worked. Reserching seems like PHP5 as CGI doesn't forward HTTP_AUTHORIZATION header, and nothing seems to change its mind. But it seems to forward REMOTE_USER as REDIRECT_REMOTE_USER. With my patch and this code in the .htaccess the authorization seems to work:
RewriteCond %{HTTP:Authorization} !^$ RewriteRule wp-app.php wp-app.php [E=REMOTE_USER:%{HTTP:Authorization},QSA,L]
The other patch is to fix a bug in the wp-app.php code. It seems that when working with PHP as CGI, the "Status:" header needs to follow a specific format, with the number, and then the reason. The actual code (from Subversion, but it's there at least from version 2.5.1) do not send the number, making CGI/PHP/Apache to return with a "500 Internal Server Error" instead the "401 Credentials Requiered", confusing Atom clients. My other patch fixes this.
Attachments (5)
Change History (36)
#5
@
16 years ago
- Milestone changed from 2.7 to 2.8
- Resolution fixed deleted
- Status changed from closed to reopened
- Version changed from 2.5.1 to 2.8
When running WordPress on a server with PHP as CGI, the HTTP status header will not be correctly set.
Seeing is believing, please compare the status headers of these pages.
http://test.eastcoder.com/wp/trunk/without-the-patch
http://test.eastcoder.com/wp/trunk/with-the-patch
Related ticket: #8226
#9
@
16 years ago
- Milestone changed from 2.8 to Future Release
I can already hear westi requiring unit tests on this one...
#12
follow-up:
↓ 27
@
15 years ago
It seems
if('cgi' == substr(strtolower(php_sapi_name()), 0, 3))
is insufficient to be a solid identifier.
References:
http://drupal.org/node/175855
http://core.trac.wordpress.org/ticket/3528
How about using a constant to let user decide applying the code?
2.9 still needs a remedy for this issue.
#13
@
15 years ago
- Keywords needs-unit-tests removed
Especially considering http://bugs.php.net/bug.php?id=20943 et al., the constant remedy appears to be a no-harm bandage that can go into 2.9 until we can come up with a solid remedy.
#14
@
15 years ago
- Keywords needs-info added; has-patch removed
- Milestone changed from 2.9 to Future Release
This ticket has already had a number of patches applied and it is now not clear what the remaining issue is and how to reproduce it.
Could someone please summarise the information so that the patch can be reviewed and the best solution resolved.
Moving to Future Release until that is done.
#15
@
15 years ago
- Keywords has-patch added; needs-info removed
- Milestone changed from Future Release to 2.9
- Version changed from 2.8 to 2.9
westi,
Remaining issue:
When running WordPress on a server with PHP as CGI, the HTTP status header will not be correctly set.
(Recent patches are originally about the issue of #8226. I know it is confusing.)
How to reproduce it:
- Run WordPress on a server with PHP as CGI.
- Access a page that does not exist so that "404 OK" HTTP status header is returned.
Demo:
This URL returns "404 OK".
http://test.eastcoder.com/wp/trunk/without-the-patch
This URL returns "404 Not Found" correctly with the actual patch.
http://test.eastcoder.com/wp/trunk/with-the-patch
I hope this is a good enough explanation.
#16
@
15 years ago
- Keywords 2nd-opinion added
Ok I have tested locally with php using fast-cgi with apache2 and I cannot reproduce the issue you are seeing.
The patch appears to apply fine without causing any issues when not running as fast cgi so I think it should be safe to apply however I would like to get a 2nd-opinion before committing as I cannot reproduce the issue and I know we have had issues getting this working correctly in the past.
#17
@
15 years ago
I have tested this with DreamHost FastCGI environment and I cannot reproduce the issue either.
Maybe this is a regular CGI specific problem, not FastCGI.
#18
@
15 years ago
- Cc markjaquith added
Ok some related tickets here:
#3528 - Looks like we used to send the Status: header for non-cgi setups - http://core.trac.wordpress.org/changeset/4684/trunk/wp-includes/functions.php
#6779 - Change to wp_redirect to use the argument of header which takes a status - http://core.trac.wordpress.org/changeset/12358
Also search through all calls to status_header for redirects we don't send it for cgi-fcgi
sapi where as all other calls to status_header are unconditional - I wonder if we should be rolling the cgi check into status header or whether the check in wp_redirect is specific to Location
headers
What server software / php version are you running which exhibits the issue?
#19
@
15 years ago
Setting two lines together like below seems to output a proper status header with various server setups.
header('Status: 403 Forbidden', true, 403); header('HTTP/1.1 403 Forbidden', true, 403);
I have tested with these and their status headers look ok so far.
- Apache 1.3.41, PHP 4.4.8, php_sapi: cgi (This one exhibits the issue as seen in demo.)
- Apache version unknown, PHP 5.2.9, php_sapi: cgi-fcgi
- Apache version unknown, PHP 5.2.11, php_sapi: apache
One side effect of this is that unnecessary "Status: XXX" line is added to headers under non cgi setups. However, it appears to be harmless.
HTTP/1.1 403 Forbidden Date: Mon, 14 Dec 2009 07:53:12 GMT Server: Apache Status: 403 Forbidden // added line Keep-Alive: timeout=15, max=100 Connection: Keep-Alive Content-Type: text/html Content-Length: 3
#20
@
15 years ago
- Milestone changed from 2.9 to 3.0
- Owner changed from anonymous to markjaquith
- Status changed from reopened to assigned
I don't want to change this right before RC. This is a "fix early in the dev cycle" bug.
Also, note that even the server sending the incorrect header is sending the correct status code. the part that comes after the status code is for humans — machines are supposed to read the code.
#23
@
15 years ago
Asking those who provided patches / discuessed: Can we draw a conclusion line here and summarize this up? Looks like a valid ticket to me and it would be nice to have this a bit more compact / easy to test & commit. Something to test? Just let me know the steps.
#25
in reply to:
↑ 24
@
15 years ago
Replying to hakre:
How to reproduce?
http://core.trac.wordpress.org/ticket/7361#comment:15 and the four that follow it.
#26
@
15 years ago
I am not able to reproduce against current trunk on CGI nor FCGI. Since this ticket was opened month ago I think it's valuable to think about that the third parameter to the header function was added in PHP 4.3. That third parameter is now used in the codebase (at least at the places I had been looking into, I did not a regex search right now) and it is related to the Status Code, I pretty much assume that this problem is now fixed but it might not have been for older PHP version. Can that be the case here?
Which PHP Version is used on that testbed where it's said it's reproduceable? Is it updated to 2.9.1 or current trunk?
#27
in reply to:
↑ 12
@
15 years ago
Replying to tenpura:
It seems
if('cgi' == substr(strtolower(php_sapi_name()), 0, 3))is insufficient to be a solid identifier.
References:
http://drupal.org/node/175855
http://core.trac.wordpress.org/ticket/3528
How about using a constant to let user decide applying the code?
2.9 still needs a remedy for this issue.
This code is used here:
php_sapi_name() == 'cgi-fcgi'
Related: #10187
suggestions welcome. I can do sniffing over the current codebase then.
#28
@
15 years ago
- Keywords early added
- Milestone changed from 3.0 to 3.1
Close to RC's and beta's, Not the time to be changing this again, Punting to 3.1 early for dealing with
#30
@
12 years ago
- Cc kurtpayne added
Is this still valid?
A related php bug #27345 notes that this behavior is now covered by the cgi.rfc2616_headers php.ini setting
#31
@
11 years ago
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from assigned to closed
As AtomPub was removed from core in 2012, see #21866. There is a plugin for it here: http://wordpress.org/plugins/atom-publishing-protocol/.
I doubt anyone really interfaces with WordPress via AtomPub anymore. Did anyone ever truly use it? I heard that when WordPress.com had it enabled, a few dozen posts came through it a day, all from few clients.
So realistically, AtomPub is essentially considered "end of life" to us. This plugin was merely done for backwards compatibility.
Closing this ticket as wontfix.
Fix for sending "Status:" header with the correct format so CGI do no send 500 response.