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/12/07 00:12:41 UTC
Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
On Wed, Dec 06, 2000 at 09:12:07PM -0000, rbb@locus.apache.org wrote:
>...
> @@ -1349,9 +1349,12 @@
> perchild_server_conf *sconf = (perchild_server_conf *)
> ap_get_module_config(r->server->module_config,
> &mpm_perchild_module);
> - char *foo = sconf->buffer;
> - int len = strlen(sconf->buffer);
> + char *foo;
> + int len;
>
> + apr_get_userdata((void **)&foo, "PERCHILD_BUFFER", r->connection->pool);
> + len = strlen(foo);
Couldn't that content be binary data? The strlen() isn't going to work in
that case.
[ it could be binary on a file upload or a PUT ]
>...
> @@ -1467,11 +1467,18 @@
> return rv;
> }
>
> +
> AP_BRIGADE_FOREACH(e, b) {
> - const char *str;
> + char *buffer = NULL;
> + const char *str;
> apr_size_t len;
> +
> + apr_get_userdata((void **)&buffer, "PERCHILD_BUFFER", f->c->pool);
> +
> ap_bucket_read(e, &str, &len, AP_NONBLOCK_READ);
> - apr_pstrcat(f->r->pool, sconf->buffer, str);
> + apr_pstrcat(f->c->pool, buffer, str);
> +
> + apr_set_userdata(&buffer, "PERCHILD_BUFFER", apr_null_cleanup, f->c->pool);
That set should probably be apr_set_userdata(buffer, ...) (no ampersand).
The apr_pstrcat() will also be a bit wonky with binary data. [not to mention
that the result value from the call isn't being used]
Finally: who places the buffer into the userdata in the first place?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Dec 06, 2000 at 07:20:43PM -0800, rbb@covalent.net wrote:
>
> > > > Nope. At this point, we are ONLY dealing with header data. I'm not
> > > > convinced that this works completely, because there may be
> > > some data on
> > > > in the bucket brigade that we haven't read yet, but the
> > > data that I am
> > > > doing a strlen on is definately ASCII data.
> > >
> > > Ah! Okee...
> >
> > Sure this won't bite anyone implementing a protocol other than http/https?
>
> By the time we are reading this data, it must be readable by the server
> itself. If we are using some protocol other than http/https that passes
> data that is not ASCII, then the server is hosed in more ways than just
> this, so we are safe. Am I sure this will always work? Nope, but I am
> sure that this will work 99% of the time, so it is good enough.
More importantly: it works for our current requirements. If some yin-yang
wants to use the perchild MPM for something other than HTTP(S), then they'll
have a bit of work to do.
Heck, they're already going to have a good chunk in terms of separating out
the request mechanisms and I/O handling from HTTP handling. Sure...
theoretically they are independent. But we have a ways to go on that :-) And
we don't need it for the Apache 2.0 release!
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
RE: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Posted by rb...@covalent.net.
> > > Nope. At this point, we are ONLY dealing with header data. I'm not
> > > convinced that this works completely, because there may be
> > some data on
> > > in the bucket brigade that we haven't read yet, but the
> > data that I am
> > > doing a strlen on is definately ASCII data.
> >
> > Ah! Okee...
>
> Sure this won't bite anyone implementing a protocol other than http/https?
By the time we are reading this data, it must be readable by the server
itself. If we are using some protocol other than http/https that passes
data that is not ASCII, then the server is hosed in more ways than just
this, so we are safe. Am I sure this will always work? Nope, but I am
sure that this will work 99% of the time, so it is good enough.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
RE: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Wednesday, December 06, 2000 6:43 PM
>
> On Wed, Dec 06, 2000 at 03:47:33PM -0800, rbb@covalent.net wrote:
> > >
> > > Couldn't that content be binary data? The strlen() isn't
> > > going to work in that case.
> > >
> > > [ it could be binary on a file upload or a PUT ]
> >
> > Nope. At this point, we are ONLY dealing with header data. I'm not
> > convinced that this works completely, because there may be
> some data on
> > in the bucket brigade that we haven't read yet, but the
> data that I am
> > doing a strlen on is definately ASCII data.
>
> Ah! Okee...
Sure this won't bite anyone implementing a protocol other than http/https?
Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Posted by rb...@covalent.net.
> > > That set should probably be apr_set_userdata(buffer, ...) (no ampersand).
> > >
> > > The apr_pstrcat() will also be a bit wonky with binary data. [not to mention
> > > that the result value from the call isn't being used]
> >
> > The strcat isn't an issue, because this is header data. Not using the
> > returned data is a bug, as is the ampersand.
>
> Looking at apr_pstrcat() and slapping my forehead... it is a variadic
> function. You need to pass in all the strings to catenate, followed by a
> NULL. If you pass buffer==NULL, then you'll get the empty string back :-)
Do me a favor, slap me too. :-) ARGH! I'm fixing mod_include right now,
but I'll get this next.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Dec 06, 2000 at 03:47:33PM -0800, rbb@covalent.net wrote:
> > > + apr_get_userdata((void **)&foo, "PERCHILD_BUFFER", r->connection->pool);
> > > + len = strlen(foo);
> >
> > Couldn't that content be binary data? The strlen() isn't going to work in
> > that case.
> >
> > [ it could be binary on a file upload or a PUT ]
>
> Nope. At this point, we are ONLY dealing with header data. I'm not
> convinced that this works completely, because there may be some data on
> in the bucket brigade that we haven't read yet, but the data that I am
> doing a strlen on is definately ASCII data.
Ah! Okee...
> > > ap_bucket_read(e, &str, &len, AP_NONBLOCK_READ);
> > > - apr_pstrcat(f->r->pool, sconf->buffer, str);
> > > + apr_pstrcat(f->c->pool, buffer, str);
> > > +
> > > + apr_set_userdata(&buffer, "PERCHILD_BUFFER", apr_null_cleanup, f->c->pool);
> >
> > That set should probably be apr_set_userdata(buffer, ...) (no ampersand).
> >
> > The apr_pstrcat() will also be a bit wonky with binary data. [not to mention
> > that the result value from the call isn't being used]
>
> The strcat isn't an issue, because this is header data. Not using the
> returned data is a bug, as is the ampersand.
Looking at apr_pstrcat() and slapping my forehead... it is a variadic
function. You need to pass in all the strings to catenate, followed by a
NULL. If you pass buffer==NULL, then you'll get the empty string back :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Posted by rb...@covalent.net.
> > + apr_get_userdata((void **)&foo, "PERCHILD_BUFFER", r->connection->pool);
> > + len = strlen(foo);
>
> Couldn't that content be binary data? The strlen() isn't going to work in
> that case.
>
> [ it could be binary on a file upload or a PUT ]
Nope. At this point, we are ONLY dealing with header data. I'm not
convinced that this works completely, because there may be some data on
in the bucket brigade that we haven't read yet, but the data that I am
doing a strlen on is definately ASCII data.
> > ap_bucket_read(e, &str, &len, AP_NONBLOCK_READ);
> > - apr_pstrcat(f->r->pool, sconf->buffer, str);
> > + apr_pstrcat(f->c->pool, buffer, str);
> > +
> > + apr_set_userdata(&buffer, "PERCHILD_BUFFER", apr_null_cleanup, f->c->pool);
>
> That set should probably be apr_set_userdata(buffer, ...) (no ampersand).
>
> The apr_pstrcat() will also be a bit wonky with binary data. [not to mention
> that the result value from the call isn't being used]
The strcat isn't an issue, because this is header data. Not using the
returned data is a bug, as is the ampersand.
> Finally: who places the buffer into the userdata in the first place?
Nobody. The first call gets NULL back, which is exactly what I
want. After we have seen the first byte of data, this filter puts the
buffer in the userdata.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------