You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 2000/11/02 21:57:58 UTC
Re: cvs commit: apache-2.0/src/main http_core.c
Woah! Wait a sec.
Why is this done? discard_request_body() will already do this. And you
really need to discard the body for *any* method, not just POST methods.
If I send a PROPFIND method, and it ends up at the default handler, then the
PROPFIND body better get thrown out.
I don't understand two things with this patch:
1) why there is a new function to duplicate what discard does
2) why it gets used on a POST instead of the standard discard
Cheers,
-g
On Thu, Nov 02, 2000 at 08:48:53PM -0000, sascha@locus.apache.org wrote:
> sascha 00/11/02 12:48:51
>
> Modified: src/main http_core.c
> Log:
> Make the default handler read the dechunked request body on a POST request.
>
> Revision Changes Path
> 1.194 +25 -15 apache-2.0/src/main/http_core.c
>
> Index: http_core.c
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/main/http_core.c,v
> retrieving revision 1.193
> retrieving revision 1.194
> diff -u -u -r1.193 -r1.194
> --- http_core.c 2000/11/02 20:33:48 1.193
> +++ http_core.c 2000/11/02 20:48:48 1.194
> @@ -2912,14 +2912,25 @@
>
> static int do_nothing(request_rec *r) { return OK; }
>
> -/*
> - * Default handler for MIME types without other handlers. Only GET
> - * and OPTIONS at this point... anyone who wants to write a generic
> - * handler for PUT or POST is free to do so, but it seems unwise to provide
> - * any defaults yet... So, for now, we assume that this will always be
> - * the last handler called and return 405 or 501.
> - */
> +#define POST_CHUNK_SIZE 4096
>
> +static int handle_request_body(request_rec *r)
> +{
> + int rv;
> + char buf[POST_CHUNK_SIZE];
> + long n;
> +
> + if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK)))
> + return rv;
> +
> + if ((rv = ap_should_client_block(r)) == 0)
> + return APR_SUCCESS;
> +
> + while ((n = ap_get_client_block(r, buf, POST_CHUNK_SIZE)) > 0);
> +
> + return APR_SUCCESS;
> +}
> +
> static int default_handler(request_rec *r)
> {
> core_dir_config *d =
> @@ -2940,13 +2951,6 @@
> int bld_content_md5 =
> (d->content_md5 & 1) && r->output_filters->frec->ftype != AP_FTYPE_CONTENT;
>
> - /* This handler has no use for a request body (yet), but we still
> - * need to read and discard it if the client sent one.
> - */
> - if ((errstatus = ap_discard_request_body(r)) != OK) {
> - return errstatus;
> - }
> -
> ap_allow_methods(r, MERGE_ALLOW, "GET", "OPTIONS", NULL);
>
> if (r->method_number == M_INVALID) {
> @@ -2967,7 +2971,13 @@
> : r->filename);
> return HTTP_NOT_FOUND;
> }
> - if (r->method_number != M_GET) {
> + if (r->method_number == M_POST) {
> + if ((errstatus = handle_request_body(r)) != APR_SUCCESS) {
> + return errstatus;
> + }
> + } else if ((errstatus = ap_discard_request_body(r)) != OK) {
> + return errstatus;
> + } else if (r->method_number != M_GET) {
> return HTTP_METHOD_NOT_ALLOWED;
> }
>
>
>
>
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/main http_core.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Nov 02, 2000 at 03:04:40PM -0800, rbb@covalent.net wrote:
> > Both of them should have the same semantic: toss the body.
> >
> > The ap_setup_client_block() can have the same value, whatever it needs to
> > be. Since they are simply tossing the darn body, it makes no difference.
> > Looking at the code: both can/should be REQUEST_CHUNKED_DECHUNK.
> >
> > [ we've had desires to toss the CHUNKED_PASS variant, so this will get rid
> > of one other usage ]
>
> Unless I am remembering incorrectly, this is the only usage of
> REQUEST_CHUNKED_PASS, so it should officially go away now. :-)
Yup. http_protocol.c has two tests for it, and http_protocol.c is the only
one that passes it.
ap_discard_request_body() should be updated to _DECHUNK and Sascha's new
function in http_core should be tossed. That will also deflate the logic to
just always calling ap_discard_request_body().
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/main http_core.c
Posted by rb...@covalent.net.
> Both of them should have the same semantic: toss the body.
>
> The ap_setup_client_block() can have the same value, whatever it needs to
> be. Since they are simply tossing the darn body, it makes no difference.
> Looking at the code: both can/should be REQUEST_CHUNKED_DECHUNK.
>
> [ we've had desires to toss the CHUNKED_PASS variant, so this will get rid
> of one other usage ]
Unless I am remembering incorrectly, this is the only usage of
REQUEST_CHUNKED_PASS, so it should officially go away now. :-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apache-2.0/src/main http_core.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Nov 02, 2000 at 10:22:20PM +0100, Sascha Schumann wrote:
> On Thu, 2 Nov 2000, Greg Stein wrote:
>...
> > I don't understand two things with this patch:
> >
> > 1) why there is a new function to duplicate what discard does
> > 2) why it gets used on a POST instead of the standard discard
>
> The difference basically boils down to this line:
>
> > > + if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK)))
>
> ap_discard_request_body() uses REQUEST_CHUNKED_PASS which is
> not what we need, if I understand the comments correctly.
Both of them should have the same semantic: toss the body.
The ap_setup_client_block() can have the same value, whatever it needs to
be. Since they are simply tossing the darn body, it makes no difference.
Looking at the code: both can/should be REQUEST_CHUNKED_DECHUNK.
[ we've had desires to toss the CHUNKED_PASS variant, so this will get rid
of one other usage ]
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/main http_core.c
Posted by Jeff Trawick <tr...@bellsouth.net>.
Sascha Schumann <sa...@schumann.cx> writes:
> ap_discard_request_body() uses REQUEST_CHUNKED_PASS which is
> not what we need, if I understand the comments correctly.
>
> - Sascha
You aren't alluding to filtering bodies which are discarded by the
default handler, are you?
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Apache Process Termination time
Posted by Austin Gonyou <au...@coremetrics.com>.
Hey,
I was wondering if there is a way to get apache to dump processes
really fast? Say I've maxxed my server at 600 Concurrent processes, and
that was just a peak, and usually it's at 100 or so. After the rush, I
want the server to go ahead and dump as many processes as quickly as it
can. Right now, it looks as though it dumps 1 process/sec. Is this just
some thing I can do in HTTPD, or should I have to modify some source?
Austin
- GrandMasterLee -
Re: cvs commit: apache-2.0/src/main http_core.c
Posted by Sascha Schumann <sa...@schumann.cx>.
On Thu, 2 Nov 2000, Greg Stein wrote:
> Woah! Wait a sec.
>
> Why is this done? discard_request_body() will already do this. And you
> really need to discard the body for *any* method, not just POST methods.
>
> If I send a PROPFIND method, and it ends up at the default handler, then the
> PROPFIND body better get thrown out.
Right, I committed on the wrong machine in a broken tree. :(
I'll fix that ASAP.
> I don't understand two things with this patch:
>
> 1) why there is a new function to duplicate what discard does
> 2) why it gets used on a POST instead of the standard discard
The difference basically boils down to this line:
> > + if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK)))
ap_discard_request_body() uses REQUEST_CHUNKED_PASS which is
not what we need, if I understand the comments correctly.
- Sascha