Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-07 Thread Roy T. Fielding
On Jul 5, 2005, at 8:56 PM, William A. Rowe, Jr. wrote: Attached is the mystery patch [omitted from the last note - whoops]. IMHO we should apply the same to ap_http_filter() in 2.1's http_filters.c It looks like a band-aid to me -- how does this module know that the server doesn't support

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-07 Thread Joe Orton
On Wed, Jul 06, 2005 at 02:53:52PM -0400, Jim Jagielski wrote: On Jul 6, 2005, at 2:22 PM, Joe Orton wrote: On Wed, Jul 06, 2005 at 11:45:21AM -0500, William Rowe wrote: ... +else { +char *len_end; +errno = 0; +c-len =

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-07 Thread Jim Jagielski
Joe Orton wrote: An empty string, right: I think it's certainly correct to treat that as invalid C-L header; Bill just asked Roy about this very question. indeed some strtol's themselves set errno for that case. (the perl-framework C-L tests picked up this inconsistency a while

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-07 Thread Jim Jagielski
Joe Orton wrote: An empty string, right: I think it's certainly correct to treat that as invalid C-L header; While we wait on Roy's response, my own PoV is that we should accept it and assume it means '0', so be as lenient and forgiving as possible in input (be generous in input, rigorous

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-07 Thread William A. Rowe, Jr.
At 12:09 PM 7/7/2005, Jim Jagielski wrote: This was, iirc, to handle cases where a strtol could possibly set it to NULL; someone, can't recall who, seemed to remember one implementation which did that, so we just figured to-hell-with-it and add a safety check, just in case :) In httpd-1.3,

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-07 Thread Jim Jagielski
William A. Rowe, Jr. wrote: At 12:09 PM 7/7/2005, Jim Jagielski wrote: This was, iirc, to handle cases where a strtol could possibly set it to NULL; someone, can't recall who, seemed to remember one implementation which did that, so we just figured to-hell-with-it and add a safety check,

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-06 Thread Jim Jagielski
+char *len_end; +c-len = ap_strtol(content_length, *len_end, 10); -if (c-len 0) { -ap_kill_timeout(r); -return ap_proxyerror(r, HTTP_BAD_GATEWAY, ap_pstrcat(r-pool, - Invalid Content-Length from remote server, -

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-06 Thread Jim Jagielski
On Jul 6, 2005, at 9:06 AM, Jim Jagielski wrote: +char *len_end; +c-len = ap_strtol(content_length, *len_end, 10); -if (c-len 0) { -ap_kill_timeout(r); -return ap_proxyerror(r, HTTP_BAD_GATEWAY, ap_pstrcat(r-pool, -

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-06 Thread William A. Rowe, Jr.
At 08:12 AM 7/6/2005, Jim Jagielski wrote: On Jul 6, 2005, at 9:06 AM, Jim Jagielski wrote: +char *len_end; +c-len = ap_strtol(content_length, *len_end, 10); +if ((c-len 0) || *len_end) { Oops... Should be: c-len = ap_strtol(content_length,

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-06 Thread Joe Orton
On Wed, Jul 06, 2005 at 11:45:21AM -0500, William Rowe wrote: ... +else { +char *len_end; +errno = 0; +c-len = ap_strtol(content_length, len_end, 10); ... +if (errno || (c-len 0) || (len_end *len_end)) { You should

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-06 Thread Jim Jagielski
On Jul 6, 2005, at 2:22 PM, Joe Orton wrote: On Wed, Jul 06, 2005 at 11:45:21AM -0500, William Rowe wrote: ... +else { +char *len_end; +errno = 0; +c-len = ap_strtol(content_length, len_end, 10); ... +if (errno ||

Re: [Patch 1.3] Strict proxy C-L / T-E conformance

2005-07-06 Thread William A. Rowe, Jr.
At 01:22 PM 7/6/2005, Joe Orton wrote: You should copy the logic used on the trunk to get the correct tests for a strtol parse error: errno || len_end == content_length || *len_end || c-len 0 What is len_end == content_length? wouldn't *content_length be clearer? And this test doesn't