You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Eric Covener <co...@gmail.com> on 2018/04/20 09:50:07 UTC

Re: svn commit: r1829642 - in /httpd/httpd/trunk: CHANGES modules/http/http_filters.c

On Fri, Apr 20, 2018 at 5:46 AM,  <yl...@apache.org> wrote:
> Author: ylavic
> Date: Fri Apr 20 09:46:51 2018
> New Revision: 1829642
>
> URL: http://svn.apache.org/viewvc?rev=1829642&view=rev
> Log:
> http: LimitRequestBody applies to proxied requests.
>
> If f->r->proxyreq is PROXYREQ_PROXY or PROXYREQ_REVERSE in ap_http_filter(),
> we are still handling the request, not the response where LimitRequestBody
> does not apply.
>
>
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/http/http_filters.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1829642&r1=1829641&r2=1829642&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Apr 20 09:46:51 2018
> @@ -1,6 +1,8 @@
>                                                           -*- coding: utf-8 -*-
>  Changes with Apache 2.5.1
>
> +  *) http: LimitRequestBody applies to proxied requests.  [Yann Ylavic]
> +
>    *) mod_proxy_http: Fix response header thrown away after the previous one
>       was considered too large and truncated. PR 62196. [Yann Ylavic]
>
>
> Modified: httpd/httpd/trunk/modules/http/http_filters.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1829642&r1=1829641&r2=1829642&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_filters.c (original)
> +++ httpd/httpd/trunk/modules/http/http_filters.c Fri Apr 20 09:46:51 2018
> @@ -317,12 +317,11 @@ apr_status_t ap_http_filter(ap_filter_t
>          f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
>          ctx->state = BODY_NONE;
>
> -        /* LimitRequestBody does not apply to proxied responses.
> +        /* LimitRequestBody does not apply to proxied responses, which have
> +         * their own ResponseFieldSize parameter.


Not sure about change but re: comment

Are those comparable? The new proxy parm ResponseFieldSize is like
LimitRequestFieldSize not LimitRequestBody?

Re: svn commit: r1829642 - in /httpd/httpd/trunk: CHANGES modules/http/http_filters.c

Posted by Eric Covener <co...@gmail.com>.
On Fri, Apr 20, 2018 at 7:07 AM, Eric Covener <co...@gmail.com> wrote:
> I don't remember this at all but:
>
> http://svn.apache.org/viewvc?rev=1031564&view=rev

This seems like a step 0.5 of document the status quo but I cannot
find any thing that precedes or follows it :/

Re: svn commit: r1829642 - in /httpd/httpd/trunk: CHANGES modules/http/http_filters.c

Posted by Eric Covener <co...@gmail.com>.
I don't remember this at all but:

http://svn.apache.org/viewvc?rev=1031564&view=rev



On Fri, Apr 20, 2018 at 7:06 AM, Eric Covener <co...@gmail.com> wrote:
> On Fri, Apr 20, 2018 at 6:54 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> On Fri, Apr 20, 2018 at 11:50 AM, Eric Covener <co...@gmail.com> wrote:
>>>
>>> Not sure about change but re: comment
>>>
>>> Are those comparable? The new proxy parm ResponseFieldSize is like
>>> LimitRequestFieldSize not LimitRequestBody?
>>
>> Yes indeed, I made a confusion here (comment restored in r1829643).
>>
>> Regarding the code change itself (!proxyreq => proxyreq !=
>> PROXYREQ_RESPONSE), I think this is the right thing to do because
>> otherwise we don't limit proxyied *request* bodies. Actually I had
>> this patch locally for a while, just noticing it now by adding the
>> ap_rgetline() stuff (that I need to "release" ASAP :) . Will re-check
>> if this is the case.
>>
>> This change is possibly not backportable to 2.4 though.
>
> It rings a bell, I  only meant I had not really thought about the
> change not that it was suspicious, sorry for the wording.
> I think i actually initiated a thread about this confusing choice many
> years ago, I will see if I can dig it up.
>
>
> --
> Eric Covener
> covener@gmail.com



-- 
Eric Covener
covener@gmail.com

Re: svn commit: r1829642 - in /httpd/httpd/trunk: CHANGES modules/http/http_filters.c

Posted by Eric Covener <co...@gmail.com>.
On Fri, Apr 20, 2018 at 6:54 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Fri, Apr 20, 2018 at 11:50 AM, Eric Covener <co...@gmail.com> wrote:
>>
>> Not sure about change but re: comment
>>
>> Are those comparable? The new proxy parm ResponseFieldSize is like
>> LimitRequestFieldSize not LimitRequestBody?
>
> Yes indeed, I made a confusion here (comment restored in r1829643).
>
> Regarding the code change itself (!proxyreq => proxyreq !=
> PROXYREQ_RESPONSE), I think this is the right thing to do because
> otherwise we don't limit proxyied *request* bodies. Actually I had
> this patch locally for a while, just noticing it now by adding the
> ap_rgetline() stuff (that I need to "release" ASAP :) . Will re-check
> if this is the case.
>
> This change is possibly not backportable to 2.4 though.

It rings a bell, I  only meant I had not really thought about the
change not that it was suspicious, sorry for the wording.
I think i actually initiated a thread about this confusing choice many
years ago, I will see if I can dig it up.


-- 
Eric Covener
covener@gmail.com

Re: svn commit: r1829642 - in /httpd/httpd/trunk: CHANGES modules/http/http_filters.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Apr 20, 2018 at 11:50 AM, Eric Covener <co...@gmail.com> wrote:
>
> Not sure about change but re: comment
>
> Are those comparable? The new proxy parm ResponseFieldSize is like
> LimitRequestFieldSize not LimitRequestBody?

Yes indeed, I made a confusion here (comment restored in r1829643).

Regarding the code change itself (!proxyreq => proxyreq !=
PROXYREQ_RESPONSE), I think this is the right thing to do because
otherwise we don't limit proxyied *request* bodies. Actually I had
this patch locally for a while, just noticing it now by adding the
ap_rgetline() stuff (that I need to "release" ASAP :) . Will re-check
if this is the case.

This change is possibly not backportable to 2.4 though.