httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: [PATCH] allocate properly-sized buffer for header
Date Thu, 25 Jan 2001 17:13:11 GMT

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