Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

["Roy T. Fielding" <fi...@gbiv.com>]

-1.  Bloat like this belongs in a module.

....Roy

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

["Roy T. Fielding" <fi...@gbiv.com>]

On Apr 11, 2008, at 8:46 PM, Graham Leggett wrote:
> Roy T. Fielding wrote:
>
>> New features belong in modules so that they don't get compiled into
>> the server by those of us who don't want that feature.  I don't  
>> recall
>> why or how KeptBodySize made it into http_filter (where it does  
>> not belong),
>> but I am quite certain that the core will not be parsing HTTP body  
>> content
>> any time soon.  So, either find another way or remove the feature  
>> as well.
>
> Done (r647263).

Thanks Graham,

....Roy

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Roy T. Fielding wrote:

> New features belong in modules so that they don't get compiled into
> the server by those of us who don't want that feature.  I don't recall
> why or how KeptBodySize made it into http_filter (where it does not 
> belong),
> but I am quite certain that the core will not be parsing HTTP body content
> any time soon.  So, either find another way or remove the feature as well.

Done (r647263).

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Roy T. Fielding wrote:

> New features belong in modules so that they don't get compiled into
> the server by those of us who don't want that feature.  I don't recall
> why or how KeptBodySize made it into http_filter (where it does not 
> belong),
> but I am quite certain that the core will not be parsing HTTP body content
> any time soon.  So, either find another way or remove the feature as well.

Before I waste another few weeks of my time finding a way only to have 
it shot down right at the end, I would appreciate some help with finding 
out a way to do this in advance.

util_filter.c is actually http_module, which is not the core, or do you 
disagree, yes?

If I can find a way to introduce another module into modules/http to 
provide HTTP support services like the KeptBody stuff and the ability to 
parse a form, which an admin can choose to not enable, will this be 
sufficient?

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

["Roy T. Fielding" <fi...@gbiv.com>]

On Apr 9, 2008, at 3:10 PM, Graham Leggett wrote:
> Roy T. Fielding wrote:
>
>> -1.  Bloat like this belongs in a module.
>
> This piece of code depends on the KeptBodySize directive, which is  
> part of the http_filter, and sits alongside ap_discard_request_body().
>
> I can move it into another module, but then that just gives the  
> administrator one extra thing to configure incorrectly.
>
> Putting it in mod_auth_form meant that mod_auth_form would be  
> delving into the internals of the http_filter module, which seems  
> wrong to me. Rather let http_filter export a formal function to the  
> world, than having external modules poking around inside it.

New features belong in modules so that they don't get compiled into
the server by those of us who don't want that feature.  I don't recall
why or how KeptBodySize made it into http_filter (where it does not  
belong),
but I am quite certain that the core will not be parsing HTTP body  
content
any time soon.  So, either find another way or remove the feature as  
well.

....Roy

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Akins, Brian wrote:

>> This kind of stuff belongs in a
>> common place.
> 
> Doesn't libapreq do this??

Do what exactly?

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

["Akins, Brian" <Br...@turner.com>]

On 4/10/08 7:05 AM, "Graham Leggett" <mi...@sharp.fm> wrote:

> This kind of stuff belongs in a
> common place.

Doesn't libapreq do this??

-- 
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Nick Kew wrote:

> c.f. my mod_form (at apache.webthing.com), which does just that,
> providing optional functions for other modules to access the
> form data.

This is upside down - modules should not have to do anything at all to 
get a possibly inserted request body or form data. What happens when a 
potential mod_auth_openid comes along, now modules have to look in two 
potential places for the body, not one? This kind of stuff belongs in a 
common place.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Nick Kew <ni...@webthing.com>]

On 9 Apr 2008, at 17:33, Joe Orton wrote:
> On Wed, Apr 09, 2008 at 05:07:33PM +0200, Graham Leggett wrote:
>> Joe Orton wrote:
>>
>>> I don't understand why *that* stuff needed to be in the core.  It is
>>> certainly possible to consume then reinject the request body,  
>>> without
>>> changing one line of core filters; it's done in mod_ssl, see
>>> ssl_io_filter_buffer/ssl_io_buffer_fill.
>>
>> What about modules that call ap_discard_request_body? Once the  
>> body has
>> been discarded, the body is discarded, there is no way to get it  
>> back.
>
> The entire request body passes through the input filter stack when  
> it is
> being discarded.  So an input filter inserted by a module can capture
> and buffer it somewhere very easily; and can provide access to that
> buffered copy however it likes, without needing to modify request_rec.

c.f. my mod_form (at apache.webthing.com), which does just that,
providing optional functions for other modules to access the
form data.

mod_form isn't (as it stands) clean enough to propose for
inclusion in httpd.  But I think it would be a pretty trivial job
either to fix or duplicate it.  And I'm happy for a module derived
from it to be ASL-licensed if it's being dropped into httpd.

-- 
Nick Kew

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Paul Querna <ch...@force-elite.com>]

Graham Leggett wrote:
> Joe Orton wrote:
> 
>> The entire request body passes through the input filter stack when it 
>> is being discarded.  So an input filter inserted by a module can 
>> capture and buffer it somewhere very easily; and can provide access to 
>> that buffered copy however it likes, without needing to modify 
>> request_rec.
> 
> In practise it is not that simple[1].
> 
> If mod_include had its own kept_body filter, it would not take into 
> account the magic that mod_auth_form performs to keep the contents of 
> the end user's form from being lost.
> 
> In other words, modules need to cooperate with one another to make this 
> work effectively, and that means common code.
> 
> So where does this common code go?

apreq handles this.

import apreq to turnk.

IMO.

-Paul

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Joe Orton wrote:

> The entire request body passes through the input filter stack when it is 
> being discarded.  So an input filter inserted by a module can capture 
> and buffer it somewhere very easily; and can provide access to that 
> buffered copy however it likes, without needing to modify request_rec.

In practise it is not that simple[1].

If mod_include had its own kept_body filter, it would not take into 
account the magic that mod_auth_form performs to keep the contents of 
the end user's form from being lost.

In other words, modules need to cooperate with one another to make this 
work effectively, and that means common code.

So where does this common code go?

HTTP request bodies being most closely associated with the HTTP 
protocol, the http_module seemed the most logical place for this 
functionality.

Others may disagree that the http_module is the best place for this, and 
that is perfectly fine by me. I am open to suggestions for alternatives.

Just to be clear: the current intention is to add it to the HTTP module, 
*not* the core.

The fact that the HTTP module still is half melted into a core is a 
separate problem.

[1] Making KeptBodySize work was like getting blood out of a stone.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Joe Orton <jo...@redhat.com>]

On Wed, Apr 09, 2008 at 05:07:33PM +0200, Graham Leggett wrote:
> Joe Orton wrote:
>
>> I don't understand why *that* stuff needed to be in the core.  It is 
>> certainly possible to consume then reinject the request body, without 
>> changing one line of core filters; it's done in mod_ssl, see 
>> ssl_io_filter_buffer/ssl_io_buffer_fill.
>
> What about modules that call ap_discard_request_body? Once the body has 
> been discarded, the body is discarded, there is no way to get it back.

The entire request body passes through the input filter stack when it is 
being discarded.  So an input filter inserted by a module can capture 
and buffer it somewhere very easily; and can provide access to that 
buffered copy however it likes, without needing to modify request_rec.

joe

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Joe Orton wrote:

> I don't understand why *that* stuff needed to be in the core.  It is 
> certainly possible to consume then reinject the request body, without 
> changing one line of core filters; it's done in mod_ssl, see 
> ssl_io_filter_buffer/ssl_io_buffer_fill.

What about modules that call ap_discard_request_body? Once the body has 
been discarded, the body is discarded, there is no way to get it back.

Regards,
Graham
--

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Joe Orton <jo...@redhat.com>]

On Wed, Apr 09, 2008 at 03:10:25PM +0200, Graham Leggett wrote:
> Roy T. Fielding wrote:
>
>> -1.  Bloat like this belongs in a module.
>
> This piece of code depends on the KeptBodySize directive, which is part of 
> the http_filter, and sits alongside ap_discard_request_body().

I don't understand why *that* stuff needed to be in the core.  It is 
certainly possible to consume then reinject the request body, without 
changing one line of core filters; it's done in mod_ssl, see 
ssl_io_filter_buffer/ssl_io_buffer_fill.

joe

Re: svn commit: r646281 - in /httpd/httpd/trunk: CHANGES include/http_protocol.h modules/http/http_filters.c

[Graham Leggett <mi...@sharp.fm>]

Roy T. Fielding wrote:

> -1.  Bloat like this belongs in a module.

This piece of code depends on the KeptBodySize directive, which is part 
of the http_filter, and sits alongside ap_discard_request_body().

I can move it into another module, but then that just gives the 
administrator one extra thing to configure incorrectly.

Putting it in mod_auth_form meant that mod_auth_form would be delving 
into the internals of the http_filter module, which seems wrong to me. 
Rather let http_filter export a formal function to the world, than 
having external modules poking around inside it.

Regards,
Graham
--