Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

Change the http version from 1.0 to 1.1 in PHP #186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arthurdarcet
Copy link

Why is the requests served with a HTTP/1.0 header?

Without this patch, on one of my setup, upgrading from Apache 2.2 to Apache 2.4 would make the request for the JS asset hang.
To be more specific: if the JS asset was under 8000 bytes (exactly), the request would go through perfectly and Apache would add the appropriate Content-Length header. Over 8000 bytes, Apache would serve only part of what was printed by the php script and then hang until the client timed out. (Using the PHP function error_log I was able to make sure the PHP script actually returned, and printed everything).
With this patch, for a JS asset over 8000 bytes, Apache switches to a Transfer-Encoding: chunked header (instead of the Content-Length one) and serve the response perfectly.

I have no idea what is the underlying problem, but updating to HTTP/1.1 makes Apache happy.
Another option would be to use $_SERVER["SERVER_PROTOCOL"] instead of HTTP/1.1

Or, even better, if Genghis is not supposed to run on PHP < 5.4, this whole line should be replaced by http_response_code($this->status); and the statusCodes array could be dropped.

@bobthecow
Copy link
Owner

The reason it returns HTTP/1.0 header is that Genghis doesn't implement any part of HTTP/1.1 :)

That does sound like an Apache configuration issue, but I've never run into it before so I can't say for sure.

Genghis is supposed to run on any PHP since 5.2, so http_response_code isn't going to do the trick.

@arthurdarcet
Copy link
Author

I agree that this must be an Apache configuration issue, but I think i'm really not far from the stock ubuntu configuration (after at least two dist-upgrade, so I can't really say for sure).
I tried disabling about everything i could think of in the apache module and nothing solved the issue.

I updated the pull request to use $_SERVER['SERVER_PROTOCOL'], that should be harmless enough ?

@bobthecow
Copy link
Owner

It should still work in the absence of a proper $_SERVER['SERVER_PROTOCOL'] version though... How about this instead?

$version = ($_SERVER['SERVER_PROTOCOL'] === 'HTTP/1.0') ? '1.0' : '1.1';
header(sprintf('HTTP/%s %s %s', $version, $this->status, self::$statusCodes[$this->status]));

Edit: updated because I hit submit before I finished actually writing valid code :)

@bobthecow
Copy link
Owner

There shouldn't be any harm in serving HTTP/1.1 for any client that doesn't explicitly request HTTP/1.0.

@arthurdarcet
Copy link
Author

I added a test to handle requests with no SERVER_PROTOCOL key in the $_SERVER array.

@bobthecow
Copy link
Owner

What I was suggesting is not that we should blindly return whatever is set… If they request the protocol BANANA/9000, Genghis most certainly will not honor it :)

If they request HTTP/1.0, return that. Otherwise return HTTP/1.1.

@julien-c
Copy link

julien-c commented Sep 4, 2014

Genghis does not support BANANA/9000?!

💩

Without this, on one of my setup, upgrading from Apache 2.2 to Apache 2.4 would make the request for the JS asset hang.
@arthurdarcet
Copy link
Author

I'm pretty sure a BANANA/9000 request would not be routed through apache to php, but i agree a sanity check can't hurt

@bobthecow
Copy link
Owner

You may be right, but Genghis doesn't just run behind Apache. It also runs as standalone in newer versions of PHP, and I doubt PHP has any such logic in place :)

@arthurdarcet
Copy link
Author

Actually it does, the php builtin server would close the connection and log [Thu Sep 4 18:09:11 2014] ::1:58846 Invalid request (Malformed HTTP request)

@bobthecow
Copy link
Owner

Oh, nice.

@arthurdarcet
Copy link
Author

I narrowed down my problem a bit: it has to do with SSL (switching to plain HTTP works) and the timeout which eventually closes the connection is Apache TimeOut directive (this should mean that the PHP does not return, but i'm pretty sure it does).

This is probably a very weird Apache bug, glad i found a dirty fix…

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants