httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: [Patch 1.3] Strict proxy C-L / T-E conformance
Date Thu, 07 Jul 2005 15:22:21 GMT
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 = ap_strtol(content_length, &len_end, 10);
> >>
> >...
> >
> >>+                if (errno || (c->len < 0) || (len_end &&  
> >>*len_end)) {
> >>
> >
> >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
> >
> 
> The len_end == content_length just means that there was no digits
> seen (possibly a blank)... not sure if we should consider that
> an error. 

An empty string, right: I think it's certainly correct to treat that as 
invalid C-L header; indeed some strtol's themselves set errno for that 
case.  (the perl-framework C-L tests picked up this inconsistency a 
while back)

> That's why in all other places we've used the (len_end && *len_end) 
> check, which ensures that

There is no case where strtol will set len_end = NULL so the first half 
of the conditional is redundant.  (also len_end was not NULL-initialized 
so if it was an attempt to catch cases where strtol does *not* set 
len_end, it was not correct ;)

> it's valid *and* that it's seen an invalid char. ap_strtol(" "...) 
> returns 0 but isn't an "error" (though in this context, I see your 
> point).

Mime
View raw message