apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Reid" <dr...@jetnet.co.uk>
Subject Re: [PATCH] allocate properly-sized buffer for header
Date Thu, 25 Jan 2001 17:21:42 GMT
[moved from new-httpd]

Ryan,

Can you put in words and some simple examples what's wrong with the API?

I'm asking here as the buckets are part of APR aren't they?

david
----- Original Message -----
From: <rbb@covalent.net>
To: <new-httpd@apache.org>; <trawickj@bellsouth.net>
Sent: Thursday, January 25, 2001 5:13 PM
Subject: Re: [PATCH] allocate properly-sized buffer for header


>
> This looks okay, but it is a very fragile solution.  The real problem
> IMHO, is that the bucket API is wrong.  This is just another symptom of
> the small bucket problem.  I think having to check for the length of the
> headers is a bad idea, because it will slow us down every time.  What we
> really want, is the same solution for the coalesce filter, namely the
> apr_brigade_* stuff, in whatever form it takes.
>
> What the ap_r* patch did was fix a symtpom, it didn't cure the disease.
> This is another symptom.
>
> Ryan
>
> On Thu, 25 Jan 2001, Jeff Trawick wrote:
>
> > ap_basic_http_header() does a couple of different things:
> >
> > 1) makes decisions about protocol and keepalive
> > 2) builds part of the header
> >
> > When used by ap_http_header_filter(), other stuff has to happen
> > between step 1 and step 2, and Ryan and Greg found out recently.
> >
> > A simple solution is to split the function in two parts and change any
> > caller of the old function to call the two functions instead.
> > However, if we expect other code to call ap_basic_http_header(), it
> > seems worthwhile to maintain the old interface (i.e., one call instead
> > of two) and use a different approach (keep old ap_basic_http_header(),
> > which now calls each of two new functions).
> >
> > IMHO basic_http_header_check() is a stupid name.
> >
> > I hope that this doesn't break anything which got fixed recently :)
> >
> > Index: modules/http/http_protocol.c
> > ===================================================================
> > RCS file: /home/cvspublic/httpd-2.0/modules/http/http_protocol.c,v
> > retrieving revision 1.277
> > diff -u -r1.277 http_protocol.c
> > --- modules/http/http_protocol.c 2001/01/24 23:47:42 1.277
> > +++ modules/http/http_protocol.c 2001/01/25 16:54:29
> > @@ -1803,13 +1803,9 @@
> >      return 1;
> >  }
> >
> > -AP_DECLARE(void) ap_basic_http_header(request_rec *r, char *buf)
> > +static void basic_http_header_check(request_rec *r,
> > +                                    const char **protocol)
> >  {
> > -    char *protocol;
> > -    char *date = NULL;
> > -    char *tmp;
> > -    header_struct h;
> > -
> >      if (r->assbackwards)
> >          return;
> >
> > @@ -1823,13 +1819,23 @@
> >          || (r->proto_num == HTTP_VERSION(1,0)
> >              && apr_table_get(r->subprocess_env, "force-response-1.0")))
{
> >
> > -        protocol = "HTTP/1.0";
> > +        *protocol = "HTTP/1.0";
> >          r->connection->keepalive = -1;
> >      }
> >      else {
> > -        protocol = AP_SERVER_PROTOCOL;
> > +        *protocol = AP_SERVER_PROTOCOL;
> >      }
> > +}
> >
> > +static void basic_http_header(request_rec *r, char *buf, const char
*protocol)
> > +{
> > +    char *date = NULL;
> > +    char *tmp;
> > +    header_struct h;
> > +
> > +    if (r->assbackwards)
> > +        return;
> > +
> >      /* Output the HTTP/1.x Status-Line and the Date and Server fields
*/
> >
> >      tmp = apr_pstrcat(r->pool, protocol, " ", r->status_line, CRLF,
NULL);
> > @@ -1849,6 +1855,14 @@
> >      apr_table_unset(r->headers_out, "Server");
> >  }
> >
> > +AP_DECLARE(void) ap_basic_http_header(request_rec *r, char *buf)
> > +{
> > +    const char *protocol;
> > +
> > +    basic_http_header_check(r, &protocol);
> > +    basic_http_header(r, buf, protocol);
> > +}
> > +
> >  /* Navigator versions 2.x, 3.x and 4.0 betas up to and including 4.0b2
> >   * have a header parsing bug.  If the terminating \r\n occur starting
> >   * at offset 256, 257 or 258 of output then it will not properly parse
> > @@ -2432,6 +2446,7 @@
> >      char *date = NULL;
> >      request_rec *r = f->r;
> >      char *buff, *buff_start;
> > +    const char *protocol;
> >      apr_bucket *e;
> >      apr_bucket_brigade *b2;
> >      apr_size_t len = 0;
> > @@ -2483,14 +2498,7 @@
> >       * and the basic http headers don't overflow this buffer.
> >       */
> >      len += strlen(ap_get_server_version()) + 100;
> > -    buff_start = buff = apr_pcalloc(r->pool, len);
> > -    ap_basic_http_header(r, buff);
> > -    buff += strlen(buff);
> > -
> > -    h.r = r;
> > -    h.buf = buff;
> > -
> > -
> > +    basic_http_header_check(r, &protocol);
> >      ap_set_keepalive(r);
> >
> >      if (r->chunked) {
> > @@ -2565,6 +2573,13 @@
> >                   (void *) &len, r->headers_out, NULL);
> >      }
> >
> > +    buff_start = buff = apr_pcalloc(r->pool, len);
> > +    basic_http_header(r, buff, protocol);
> > +    buff += strlen(buff);
> > +
> > +    h.r = r;
> > +    h.buf = buff;
> > +
> >      if (r->status == HTTP_NOT_MODIFIED) {
> >          apr_table_do((int (*)(void *, const char *, const char *))
form_header_field,
> >                      (void *) &h, r->headers_out,
> >
> > --
> > Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
> >        http://www.geocities.com/SiliconValley/Park/9289/
> >              Born in Roswell... married an alien...
> >
> >
>
>
>
____________________________________________________________________________
___
> Ryan Bloom                        rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> --------------------------------------------------------------------------
-----
>
>


Mime
View raw message