You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2010/08/17 16:43:45 UTC
svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Author: jim
Date: Tue Aug 17 14:43:45 2010
New Revision: 986333
URL: http://svn.apache.org/viewvc?rev=986333&view=rev
Log:
Further checks for non-body requests...
Modified:
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
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=986333&r1=986332&r2=986333&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 17 14:43:45 2010
@@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
* Send the HTTP/1.1 request to the remote server
*/
+ /*
+ * To be compliant, we only use 100-Continue for requests with no bodies.
+ * We also make sure we won't be talking HTTP/1.0 as well.
+ */
do_100_continue = (worker->ping_timeout_set
&& !r->header_only
+ && !r->kept_body
+ && !(apr_table_get(r->headers_in, "Content-Length"))
+ && !(apr_table_get(r->headers_in, "Transfer-Encoding"))
&& (PROXYREQ_REVERSE == r->proxyreq)
&& !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
@@ -1397,10 +1404,12 @@ apr_status_t ap_proxy_http_process_respo
do_100_continue = (worker->ping_timeout_set
&& !r->header_only
+ && !r->kept_body
+ && !(apr_table_get(r->headers_in, "Content-Length"))
+ && !(apr_table_get(r->headers_in, "Transfer-Encoding"))
&& (PROXYREQ_REVERSE == r->proxyreq)
&& !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
-
bb = apr_brigade_create(p, c->bucket_alloc);
pass_bb = apr_brigade_create(p, c->bucket_alloc);
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Jim Jagielski <ji...@apache.org>.
>
> as we only want do_100_continue to be true *if* we have a request body,
My mistake.... for some reason I commented and coded the
reverse, even though I knew what it should have been...
Too much on my mind...
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 18, 2010, at 8:26 AM, Joe Orton wrote:
> On Tue, Aug 17, 2010 at 06:00:58PM +0200, "Plüm, Rüdiger, VF-Group" wrote:
>> I think you should use
>>
>> && ((apr_table_get(r->headers_in, "Content-Length") ||
>> apr_table_get(r->headers_in, "Transfer-Encoding")))
>>
>> as we only want do_100_continue to be true *if* we have a request body,
>> which means that either Content-Length or Transfer-Encoding is set
>> in the request. *If* both are unset then we have no request body and
>> hence do_100_continue should be false.
>
> This needs to check that C-L is non-zero if it is present; sending
> 100-continue with C-L: 0 will break.
>
Agreed... In fact, I think I'll create a small util function
that abstracts out whether a request has a body or not... we
do this test all over the place, with various versions of
the logic. Would be nice to standardize that, I think.
Re: svn commit: r986333 -
/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Aug 17, 2010 at 06:00:58PM +0200, "Plüm, Rüdiger, VF-Group" wrote:
> I think you should use
>
> && ((apr_table_get(r->headers_in, "Content-Length") ||
> apr_table_get(r->headers_in, "Transfer-Encoding")))
>
> as we only want do_100_continue to be true *if* we have a request body,
> which means that either Content-Length or Transfer-Encoding is set
> in the request. *If* both are unset then we have no request body and
> hence do_100_continue should be false.
This needs to check that C-L is non-zero if it is present; sending
100-continue with C-L: 0 will break.
Regards, Joe
RE: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Jim Jagielski
> Sent: Dienstag, 17. August 2010 17:22
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r986333 -
> /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>
>
> On Aug 17, 2010, at 10:55 AM, Rainer Jung wrote:
>
> > On 17.08.2010 16:43, jim@apache.org wrote:
> >> Author: jim
> >> Date: Tue Aug 17 14:43:45 2010
> >> New Revision: 986333
> >>
> >> URL: http://svn.apache.org/viewvc?rev=986333&view=rev
> >> Log:
> >> Further checks for non-body requests...
> >>
> >> Modified:
> >> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> >>
> >> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> >> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/m
> od_proxy_http.c?rev=986333&r1=986332&r2=986333&view=diff
> >>
> ==============================================================
> ================
> >> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> >> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue
> Aug 17 14:43:45 2010
> >> @@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
> >> * Send the HTTP/1.1 request to the remote server
> >> */
> >>
> >> + /*
> >> + * To be compliant, we only use 100-Continue for
> requests with no bodies.
> >> + * We also make sure we won't be talking HTTP/1.0 as well.
> >> + */
> >> do_100_continue = (worker->ping_timeout_set
> >> && !r->header_only
> >> +&& !r->kept_body
Why does r->kept_body matter here? If we have a request body either
Content-Length or Transfer-Encoding is set.
> >> +&& !(apr_table_get(r->headers_in, "Content-Length"))
> >> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
> >
> > Did you mean:
> >
> > && (!apr_table_get(r->headers_in, "Content-Length") ||
> > !apr_table_get(r->headers_in, "Transfer-Encoding")
> >
>
> I think you meant:
>
> && (!(apr_table_get(r->headers_in, "Content-Length") ||
> apr_table_get(r->headers_in, "Transfer-Encoding")))
I think you should use
&& ((apr_table_get(r->headers_in, "Content-Length") ||
apr_table_get(r->headers_in, "Transfer-Encoding")))
as we only want do_100_continue to be true *if* we have a request body,
which means that either Content-Length or Transfer-Encoding is set
in the request. *If* both are unset then we have no request body and
hence do_100_continue should be false.
Regards
Rüdiger
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 17.08.2010 17:22, Jim Jagielski wrote:
>
> On Aug 17, 2010, at 10:55 AM, Rainer Jung wrote:
>
>> On 17.08.2010 16:43, jim@apache.org wrote:
>>> Author: jim
>>> Date: Tue Aug 17 14:43:45 2010
>>> New Revision: 986333
>>>
>>> URL: http://svn.apache.org/viewvc?rev=986333&view=rev
>>> Log:
>>> Further checks for non-body requests...
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>>
>>> 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=986333&r1=986332&r2=986333&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 17 14:43:45 2010
>>> @@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
>>> * Send the HTTP/1.1 request to the remote server
>>> */
>>>
>>> + /*
>>> + * To be compliant, we only use 100-Continue for requests with no bodies.
>>> + * We also make sure we won't be talking HTTP/1.0 as well.
>>> + */
>>> do_100_continue = (worker->ping_timeout_set
>>> && !r->header_only
>>> +&& !r->kept_body
>>> +&& !(apr_table_get(r->headers_in, "Content-Length"))
>>> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
>>
>> Did you mean:
>>
>> && (!apr_table_get(r->headers_in, "Content-Length") ||
>> !apr_table_get(r->headers_in, "Transfer-Encoding")
>>
>
> I think you meant:
>
> && (!(apr_table_get(r->headers_in, "Content-Length") || apr_table_get(r->headers_in, "Transfer-Encoding")))
>
> Which xforms to
>
> && !a
> && !b
See my other post about using expect only *if* there is a body, but
indeed I got it wrong (sigh!):
&& (apr_table_get(r->headers_in, "Content-Length") ||
apr_table_get(r->headers_in, "Transfer-Encoding")
Regards,
Rainer
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 17, 2010, at 10:55 AM, Rainer Jung wrote:
> On 17.08.2010 16:43, jim@apache.org wrote:
>> Author: jim
>> Date: Tue Aug 17 14:43:45 2010
>> New Revision: 986333
>>
>> URL: http://svn.apache.org/viewvc?rev=986333&view=rev
>> Log:
>> Further checks for non-body requests...
>>
>> Modified:
>> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>
>> 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=986333&r1=986332&r2=986333&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 17 14:43:45 2010
>> @@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
>> * Send the HTTP/1.1 request to the remote server
>> */
>>
>> + /*
>> + * To be compliant, we only use 100-Continue for requests with no bodies.
>> + * We also make sure we won't be talking HTTP/1.0 as well.
>> + */
>> do_100_continue = (worker->ping_timeout_set
>> && !r->header_only
>> +&& !r->kept_body
>> +&& !(apr_table_get(r->headers_in, "Content-Length"))
>> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
>
> Did you mean:
>
> && (!apr_table_get(r->headers_in, "Content-Length") ||
> !apr_table_get(r->headers_in, "Transfer-Encoding")
>
I think you meant:
&& (!(apr_table_get(r->headers_in, "Content-Length") || apr_table_get(r->headers_in, "Transfer-Encoding")))
Which xforms to
&& !a
&& !b
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 17.08.2010 17:08, Jim Jagielski wrote:
>
> On Aug 17, 2010, at 10:55 AM, Rainer Jung wrote:
>
>> On 17.08.2010 16:43, jim@apache.org wrote:
>>> Author: jim
>>> Date: Tue Aug 17 14:43:45 2010
>>> New Revision: 986333
>>>
>>> URL: http://svn.apache.org/viewvc?rev=986333&view=rev
>>> Log:
>>> Further checks for non-body requests...
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>>
>>> 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=986333&r1=986332&r2=986333&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 17 14:43:45 2010
>>> @@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
>>> * Send the HTTP/1.1 request to the remote server
>>> */
>>>
>>> + /*
>>> + * To be compliant, we only use 100-Continue for requests with no bodies.
>>> + * We also make sure we won't be talking HTTP/1.0 as well.
>>> + */
>>> do_100_continue = (worker->ping_timeout_set
>>> && !r->header_only
>>> +&& !r->kept_body
>>> +&& !(apr_table_get(r->headers_in, "Content-Length"))
>>> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
>>
>> Did you mean:
>>
>> && (!apr_table_get(r->headers_in, "Content-Length") ||
>> !apr_table_get(r->headers_in, "Transfer-Encoding")
>>
>
> If it has CL but not TE then we get
>
> && ( 0 || 1)
>
> which is not what we want... We need to make sure it doesn't
> have CL and that is also doesn't have TE. If it has either
> one, then a body exists.
Sorry, then my remark should have already been applied higher up (at the
new code comment). Doesn't the RFC say, the expectation is *not* allowed
if there is *no* request body?
Cited from 8.2.3: "A client MUST NOT send an Expect request-header field
(section 14.20) with the "100-continue" expectation if it does not
intend to send a request body."
Regards,
Rainer
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 17, 2010, at 10:55 AM, Rainer Jung wrote:
> On 17.08.2010 16:43, jim@apache.org wrote:
>> Author: jim
>> Date: Tue Aug 17 14:43:45 2010
>> New Revision: 986333
>>
>> URL: http://svn.apache.org/viewvc?rev=986333&view=rev
>> Log:
>> Further checks for non-body requests...
>>
>> Modified:
>> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>
>> 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=986333&r1=986332&r2=986333&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 17 14:43:45 2010
>> @@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
>> * Send the HTTP/1.1 request to the remote server
>> */
>>
>> + /*
>> + * To be compliant, we only use 100-Continue for requests with no bodies.
>> + * We also make sure we won't be talking HTTP/1.0 as well.
>> + */
>> do_100_continue = (worker->ping_timeout_set
>> && !r->header_only
>> +&& !r->kept_body
>> +&& !(apr_table_get(r->headers_in, "Content-Length"))
>> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
>
> Did you mean:
>
> && (!apr_table_get(r->headers_in, "Content-Length") ||
> !apr_table_get(r->headers_in, "Transfer-Encoding")
>
If it has CL but not TE then we get
&& ( 0 || 1)
which is not what we want... We need to make sure it doesn't
have CL and that is also doesn't have TE. If it has either
one, then a body exists.
Re: svn commit: r986333 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 17.08.2010 16:43, jim@apache.org wrote:
> Author: jim
> Date: Tue Aug 17 14:43:45 2010
> New Revision: 986333
>
> URL: http://svn.apache.org/viewvc?rev=986333&view=rev
> Log:
> Further checks for non-body requests...
>
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>
> 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=986333&r1=986332&r2=986333&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 17 14:43:45 2010
> @@ -704,8 +704,15 @@ int ap_proxy_http_request(apr_pool_t *p,
> * Send the HTTP/1.1 request to the remote server
> */
>
> + /*
> + * To be compliant, we only use 100-Continue for requests with no bodies.
> + * We also make sure we won't be talking HTTP/1.0 as well.
> + */
> do_100_continue = (worker->ping_timeout_set
> && !r->header_only
> +&& !r->kept_body
> +&& !(apr_table_get(r->headers_in, "Content-Length"))
> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
Did you mean:
&& (!apr_table_get(r->headers_in, "Content-Length") ||
!apr_table_get(r->headers_in, "Transfer-Encoding")
> && (PROXYREQ_REVERSE == r->proxyreq)
> && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
>
> @@ -1397,10 +1404,12 @@ apr_status_t ap_proxy_http_process_respo
>
> do_100_continue = (worker->ping_timeout_set
> && !r->header_only
> +&& !r->kept_body
> +&& !(apr_table_get(r->headers_in, "Content-Length"))
> +&& !(apr_table_get(r->headers_in, "Transfer-Encoding"))
Same as above
> && (PROXYREQ_REVERSE == r->proxyreq)
> && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
>
> -
> bb = apr_brigade_create(p, c->bucket_alloc);
> pass_bb = apr_brigade_create(p, c->bucket_alloc);
Regards,
Rainer