Eric,

You've convinced me. The other header values are taken care of.
Your patch includes all of the work I had done and then some. I have
tested my subpart (nearly identical to your code to add request/
response times) and it works fine. I am less familiar with the
rest of your patch, but have desk checked it and the concept looks fine.
At this point I would recommend finishing your testing and submit
all of your work (in parts if too big) and I or someone else will
commit it for you.

Thanks.

Paul J. Reder

Eric Prud'hommeaux wrote:

> On Thu, Aug 01, 2002 at 07:55:32PM -0400, Paul J. Reder wrote:
> 
>>This only addresses part of the headers. You aren't handling last_
>>modified header value, the content_type or the filename (which are
>>all handled in the mem_cache).
>>
> 
> I believe the special elements in the request structure are filled by
> ap_scan_script_header_err_core's copious special header handing
> (server/util_script.c:438).
> 
>   Content-type        -> r->content_type && set handler
>   Status      -> r->status and r->status_line -- not relevent outside script
>   Location    -> r->headers_out
>   Content-Length-> r->headers_out
>   Transfer-Encoding -> r->headers_out
>   Last-Modified       -> r->mtime and r->headers_out
>   Set-Cookie  -> overlayed on r->err_headers_out
>   -others-    -> r->err_headers_out
> 
> Merging these seems to work out well (included in the attached diff):
> 
>     ap_scan_script_header_err(r, dobj->hfd, NULL);    /* read headers into 
>err_headers_out */
>     apr_table_overlap(r->headers_out, r->err_headers_out,
>                     APR_OVERLAP_TABLES_MERGE);/* these are the real headers, not 
>error headers */
>     r->err_headers_out = apr_table_make(r->pool, 20);
>     rv = apr_file_gets(&urlbuff[0], urllen, dobj->hfd);           /* Read status  */
> 
> Now ideally, none of the headers above will come in a GET request. We
> can therefor call ap_scan_script_header_err and it will not corrupt/
> contribute to the already set r->headers_out.
> 
>     rv = apr_file_gets(&urlbuff[0], urllen, dobj->hfd);   /* Skip empty line. */
>     r->err_headers_out = h->req_hdrs = apr_table_make(r->pool, 20);/* Pick up */
>     ap_scan_script_header_err(r, dobj->hfd, NULL);    /* request headers also */
>     r->err_headers_out = apr_table_make(r->pool, 20);    /* in metadata file. */
> 
> 
> If we are less confident of this, and elect to protect r->headers_out,
> we can use a tempTable to hold the contents:
> 
>     apr_table_t * tempTable;
>     ....
>     rv = apr_file_gets(&urlbuff[0], urllen, dobj->hfd);   /* skip empty line  */
>     tempTable = r->headers_out;
>     r->headers_out = h->req_hdrs = apr_table_make(r->pool, 20);    /* pick up */
>     ap_scan_script_header_err(r, dobj->hfd, NULL);    /* request headers also */
>     r->headers_out = tempTable;                                /* in metadata file. 
>*/
>     apr_table_overlap(h->req_hdrs, r->err_headers_out,
>                     APR_OVERLAP_TABLES_MERGE);
>     r->err_headers_out = apr_table_make(r->pool, 20);
> 
> I vote the former way. We are supposed to be dealing with compliant
> agents. Downside, a bogus agent could, by sending a Content-Type: in a
> GET request, override the response for everyone until the cache got
> stale.
> 
> 
>>You also cut and pasted the offset value in your code.
>>
>>+    offset += (sizeof(info->expire)*2) + 1;
>>+    info->request_time = ap_cache_hex2usec(urlbuff + offset);
>>+    offset += (sizeof(info->expire)*2) + 1;
>>+    info->response_time = ap_cache_hex2usec(urlbuff + offset);
>>
>>should at a minimum be
>>
>>+    offset += (sizeof(dobj->version)*2) + 1;
>>+    info->request_time = ap_cache_hex2usec(urlbuff + offset);
>>+    offset += (sizeof(info->request_time)*2) + 1;
>>+    info->response_time = ap_cache_hex2usec(urlbuff + offset);
>>
>>I know this is pedantic, but the values might change.
>>
> 
> Yeah, I blindly copied that stuff. Thanks for noticing.
> 
> 
>>I am in fact working on this very thing. I will add the extra code
>>you have done to my patch if that is okay with you. Otherwise I can
>>just do a more detailed look at your code with comments and commit
>>your patch for you.
>>
>>Let me know.
>>
> 
> Seems like adopting it is the easiest for you. Either is fine by me.
> 
> 
>>By the way, how tested is your code?
>>
> 
> The patches I sent you represent a subset of the patches I've
> introduced to handle HTTP Extensions (I'll submit them later). I
> have been running that code for a week or so now, specifically
> testing different scenarios.
> 
> As for the patches I sent you, I thought it was tested., but now I
> think I was testing mem. Duh.
> 
> Anyways, current status:
> Three code paths:
> 
> ====== new, uncached resource -- established by `rm -r 
>/usr/local/w3c-apache-clean/proxy/*`
> 
> GET http://mr-pink.w3.org:8081/robots.txt HTTP/1.1
> Host: mr-pink.w3.org:8081
> Foo: bar
> 
> HTTP/1.1 200 OK
> Date: Fri, 02 Aug 2002 06:54:18 GMT
> Server: Apache/1.3.26 (Unix) PHP/3.0.18
> P3P: policyref="http://www.w3.org/2001/05/P3P/p3p.xml";
> Cache-Control: max-age=21600
> Expires: Fri, 02 Aug 2002 12:54:18 GMT
> Last-Modified: Thu, 06 Jun 2002 09:44:28 GMT
> ETag: "3cff2efc"
> Accept-Ranges: bytes
> Content-Type: text/plain; qs=0.4; charset=ISO-8859-1
> Via: 1.1 mr-pink.w3.org:8081
> Content-Length: 473
> 
> #
> # robots.txt for http://www.w3.org/
> ...
> 
> 
> ===== served from valid cache
> 
> GET http://mr-pink.w3.org:8081/robots.txt HTTP/1.1
> Host: mr-pink.w3.org:8081
> Foo: bar
> 
> HTTP/1.1 200 OK
> Date: Fri, 02 Aug 2002 06:55:53 GMT
> Server: Apache/1.3.26 (Unix) PHP/3.0.18
> Last-Modified: Thu, 06 Jun 2002 09:44:28 GMT
> P3P: policyref="http://www.w3.org/2001/05/P3P/p3p.xml";
> Cache-Control: max-age=21600
> Expires: Fri, 02 Aug 2002 12:54:18 GMT
> ETag: "3cff2efc"
> Accept-Ranges: bytes
> Via: 1.1 mr-pink.w3.org:8081
> Content-Type: text/plain; qs=0.4; charset=ISO-8859-1
> Age: 98403064
> Content-Length: 473
> 
> #
> # robots.txt for http://www.w3.org/
> 
> 
> ===== stale cache not used -- simulated with forcing
>   ap_cache_check_freshness to return 0. I've also tested the date
>   parsing stuff by setting up a 10 second expiry and seeing it "do the
>   right thing".
> 
> GET http://mr-pink.w3.org:8081/robots.txt HTTP/1.1
> Host: mr-pink.w3.org:8081
> Foo: bar
> 
> HTTP/1.1 200 OK
> Date: Fri, 02 Aug 2002 06:55:53 GMT
> Server: Apache/1.3.26 (Unix) PHP/3.0.18
> Last-Modified: Thu, 06 Jun 2002 09:44:28 GMT
> P3P: policyref="http://www.w3.org/2001/05/P3P/p3p.xml";
> Cache-Control: max-age=21600
> Expires: Fri, 02 Aug 2002 12:54:18 GMT
> ETag: "3cff2efc"
> Accept-Ranges: bytes
> Via: 1.1 mr-pink.w3.org:8081
> Content-Type: text/plain; qs=0.4; charset=ISO-8859-1
> Age: 98403064
> Content-Length: 473
> 
> #
> # robots.txt for http://www.w3.org/
> ...
> 
> 
> ===== end of samples
> 
> 
>>Thanks,
>>
> 
> Cheers,
> 


-- 
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it.  Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein


Reply via email to