I use drupal+apache on backend and ngnix on frontend
Nginx request apache by HTTP/1.0 but apache response HTTP/1.1 using chunked
Therefore, I have trouble displaying some pages.

My solution to the problem - run this

find includes/ | xargs grep -ilE 'header[(]'\''HTTP/1.1' | xargs perl -pi -e 's#'\''HTTP/1.1 #\$_SERVER['\''SERVER_PROTOCOL'\''].'\'' #g;'

It is replace

drupal_set_header('HTTP/1.1 503 Service Unavailable');
to
drupal_set_header($_SERVER['SERVER_PROTOCOL'].' 503 Service Unavailable');

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kbahey’s picture

Version: 6.0-rc2 » 7.x-dev
Status: Active » Needs review
FileSize
3.56 KB

I think this is important to have, so we get away from hard coded HTTP protocol versions.

Here is a patch against today's HEAD.

Needs testing with various environments.

kbahey’s picture

Title: frontend + backend + drupal » Stop hardcoding the HTTP protocol version

Changing title ...

Bart Jansens’s picture

Squid users also have problems with this. Its the cause of the random hex characters appearing at the top of error pages. There are several issues in the D5 queue about this already.

I don't think just using SERVER_PROTOCOL is sufficient. The page cache caches the response headers as well, so it will use the protocol version of the first user who requests the page. I see two solutions:

1. Just use HTTP/1.0 for error pages.
2. Modify the code to use the correct version on output.

While (2) is probably the best solution, it can be a bit tricky. I'm using (1) on all my D5 sites, i can live without HTTP/1.1 on error pages..

c960657’s picture

Note that SERVER_PROTOCOL contains whatever protocol version the user sends.

If a client sometime in the future makes an HTTP/2.0 request to an HTTP/1.1 server, the server should not claim support for HTTP/2.0 but rather respond with HTTP/1.1.

Perhaps the third parameter for header() can be used.

c960657’s picture

In Wordpress, the response is sent as either HTTP/1.0 or HTTP/1.1, depending on the request:
http://trac.wordpress.org/ticket/3886

lilou’s picture

Status: Needs review » Needs work

Patch #1 need to be re-rolled.

kbahey’s picture

Status: Needs work » Needs review
FileSize
3.77 KB

New patch that eliminates the hard coded protocol.

JacobSingh’s picture

Status: Needs review » Needs work

I'm not sure this is exactly what is needed.

If I read it correctly, this means that drupal_http_request will send its requests via the SERVER_PROTOCOL of the user which is requesting the page. This is not always desired. For instance, say I have a client who is accessing a page with HTTP 1.0, but on that page I have to load some data from a web service, and I want to use HTTP 1.1 to make that request. With this patch, the request goes out as 1.0

Perhaps better is to add a $conf variable for outgoing HTTP requests and use this in drupal_http_request.

Best,
Jacob

Damien Tournoud’s picture

Status: Needs review » Needs work
FileSize
3.45 KB

drupal_http_request should stay HTTP 1.0. Here is a reroll.

Damien Tournoud’s picture

Status: Needs work » Needs review
JacobSingh’s picture

Status: Needs work » Needs review

Hi Damien,

Can you explain to me the rationale for your last statement? Defaulting to 1.0 is one thing, but I can't see any reason to hardcode it (or base outgoing request protocol versions on the incoming protocol version)

Best,
Jacob

c960657’s picture

Defaulting to 1.0 is one thing, but I can't see any reason to hardcode it (or base outgoing request protocol versions on the incoming protocol version)

The HTTP client in drupal_http_request() is not HTTP/1.1 compliant but only supports HTTP/1.0.

For instance, HTTP/1.1 compliant clients MUST support the chunked transfer encoding, but this is not supported by drupal_http_request(). So if Drupal claims HTTP/1.1 support in the request header, the server may return something that drupal_http_request() doesn't understand.

So it is correct to hardcode the version number for outgoing requests - at least until Drupal gets support for HTTP/1.1.

Damien Tournoud’s picture

@c960657: thank you! I felt that our implementation was not HTTP/1.1 but I was unable to find a compelling argument.

Damien Tournoud’s picture

There was also an hardcoded version in install.php. Also, we are ensuring that the SERVER_PROTOCOL is sane in the new drupal_initialize_variables().

Damien Tournoud’s picture

The patch for install.php didn't make it for some reason.

JacobSingh’s picture

Ah! Thanks for the explanation.

Still, is there someway I can hack around it? Even if it means doing something like:

if (!defined('DRUPAL_HTTP_VERSION')) {
define('DRUPAL_HTTP_VERSION','HTTP/1.1');
}

Because I think there are cases where even though the drupal_http_client doesn't support all 1.1 features, other systems might require it but not actually require those features, and for hackers like me, there are cases.

I guess I just wonder why we would want to hardcode anything. If nothing else, a constant or a $conf var eliminates the need for someone to then hack core / replicate the function. But yeah, I get your point, it shouldn't be 1.1 by default.

Best,
Jacob

Damien Tournoud’s picture

@JacobSingh: drupal_http_request is a mess right now. There would be a lot of work to do on it to make it good. I don't want to cluter it more by adding a somewhat unneeded new feature.

Dries’s picture

I've committed this patch to CVS HEAD. We can still discuss Jacob's case separately. Hence, I'm not marking this 'fixed' yet.

c960657’s picture

Because I think there are cases where even though the drupal_http_client doesn't support all 1.1 features, other systems might require it but not actually require those features, and for hackers like me, there are cases.

That sounds like some rather exotic cases. Could you give some examples?

c960657’s picture

Status: Needs review » Postponed (maintainer needs more info)

@JacobSingh: Any comments on #19?

pwolanin’s picture

re #19 - see: http://pajamadesign.com/2008/09/02/how-to-gzip-compression-between-drupa...

nginx does not support gzip unless it's HTTP 1.1

sun.core’s picture

Status: Postponed (maintainer needs more info) » Closed (fixed)

Reverting status.

TR’s picture

Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Active

This was never backported to Drupal 6. Should it be?

c960657’s picture

Issue tags: +Needs backport to D6

Yes, I think a backport would be appropriate.

TR’s picture

Status: Active » Needs review
FileSize
3.94 KB

OK, here's a straight port to Drupal 6 of the patch from#15. Let's see what the testbot has to say about it.

pwolanin’s picture

I don't think the bot tests D6 patches - it must have applied yours against D7, since you did not follow the naming convention.

TR’s picture

Actually, if you look at the log, it did apply it against D6 and test it against D6. But since there are no test cases for core D6, the test results don't mean much other than showing the patch does apply and doesn't introduce any syntax errors.

What naming convention are you referring to? The one I know about says the opposite - that you DON'T end the patch name in -Dn unless you want the bot to ignore it. For example, if you uploaded patches for D7 and D8 into an 8.x issue you would suffix the D7 patch with -D7 to tell the bot not to test it. That's not the case here - this thread is a D6 thread so the bot assumes the patch is for D6 unless told otherwise.

c960657’s picture

Status: Needs review » Reviewed & tested by the community
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

Drupal 6 code style requires different treatment for string concatenation.

TR’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
3.93 KB

Same patch as in #25, but with the D6 code style for string concatenations. Setting back to RTBC, if that's the only objection.

protoplasm’s picture

Thank you! I'm running nginx proxy in front of apache on a big D6 site. 404's generated those strange numbers and I was at a loss for fixing it. Development server using windows/wamp and no proxy had no such issue. This patch fixed the issue. Thanks!

I'm just curious--could you just suppress the error message without reverting to the 1.0 header? I don't know much about this, so I'm sure it's something you already considered and dismissed as not viable.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Committed to Drupal 6, thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D6

Automatically closed -- issue fixed for 2 weeks with no activity.