Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <4C6A3FC3.8080102@apache.org>
Date: Tue, 17 Aug 2010 09:52:35 +0200
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.24) Gecko/20100301 SeaMonkey/1.1.19
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r986090 - in /httpd/httpd/trunk: CHANGES
 docs/manual/mod/mod_proxy.xml
 modules/proxy/mod_proxy_http.c
References: <20100816183619.EEBE32388980@eris.apache.org>
 <4C6A3553.5010704@apache.org>
In-Reply-To: <4C6A3553.5010704@apache.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit


On 08/17/2010 09:08 AM, Ruediger Pluem wrote:
> 
> On 08/16/2010 08:36 PM, jim@apache.org wrote:
>> Author: jim
>> Date: Mon Aug 16 18:36:19 2010
>> New Revision: 986090
>>
>> URL: http://svn.apache.org/viewvc?rev=986090&view=rev
>> Log:
>> For backends which are HTTP/1.1, do a quick test (ping)
>> of the "connection" via 100-Continue for reverse
>> proxies...
>>
>> ACO and Filip Hanik also helped out with the idea...
>>
>> Modified:
>>     httpd/httpd/trunk/CHANGES
>>     httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
>>     httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>
> 
> General remark: You added a lot of tabs with your commit. Could you please
> clean this up an detab?
> 
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=986090&r1=986089&r2=986090&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Mon Aug 16 18:36:19 2010
>> @@ -669,7 +669,7 @@ static int spool_reqbody_cl(apr_pool_t *
>>  
>>  static
>>  int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
>> -                                   proxy_conn_rec *p_conn, conn_rec *origin,
>> +                                   proxy_conn_rec *p_conn, proxy_worker *worker,
>>                                     proxy_server_conf *conf,
>>                                     apr_uri_t *uri,
>>                                     char *url, char *server_portstr)
>> @@ -694,6 +694,8 @@ int ap_proxy_http_request(apr_pool_t *p,
>>      int force10, rv;
>>      apr_table_t *headers_in_copy;
>>      proxy_dir_conf *dconf;
>> +    conn_rec *origin = p_conn->connection;
>> +    int do_100_continue;
>>  
>>      dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
>>      header_brigade = apr_brigade_create(p, origin->bucket_alloc);
>> @@ -702,6 +704,11 @@ int ap_proxy_http_request(apr_pool_t *p,
>>       * Send the HTTP/1.1 request to the remote server
>>       */
>>  
>> +    do_100_continue = (worker->ping_timeout_set
>> +                       && !r->header_only
>> +                       && (PROXYREQ_REVERSE == r->proxyreq)
>> +                       && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
>> +    
> 
> 
> Unless I miss something important I do not see a check whether the request to the proxy
> by the client has a request body. According to 8.2.3 we MUST NOT add the Expect header
> if we do not intend to sent a body. So this would be an RFC violation.
> My prior understanding was that we are liberal of what we accept but strict in what we
> generate.

Further update: If sending a request with
Expect: 100-continue

to an httpd 2.3.5 and up webserver with no request body, the connection will be closed by
the server after this request.
See PR47087 (https://issues.apache.org/bugzilla/show_bug.cgi?id=47087) and
r888310 (http://svn.apache.org/viewvc?view=revision&revision=888310).
As most requests to the origin server will IMHO look like this (Expect: 100-continue with
no request body) this seems to make this feature pretty much useless as it effectively
disables keepalives with the origin server.

Regards

Rüdiger