[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: <[EMAIL PROTECTED]> To: <[email protected]>; <[EMAIL PROTECTED]> 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 | [EMAIL PROTECTED] | PGP public key at web site: > > http://www.geocities.com/SiliconValley/Park/9289/ > > Born in Roswell... married an alien... > > > > > > > ____________________________________________________________________________ ___ > Ryan Bloom [EMAIL PROTECTED] > 406 29th St. > San Francisco, CA 94131 > -------------------------------------------------------------------------- ----- > >
