Re: [PATCH] Increase readability of HttpHdrRangeSpec::parseInit

On 28/12/2012 10:07 a.m., Kinkie wrote:
> Hi all,
> the attached patch stems from Coverity issue 434032 (dereference of
> possibly null pointer).
> Coverity's is a false positive, but it highlights the fact that the
> code is poorly readable.
>
> The attached patch tries to address the readability issue a bit. It's
> a zero-sum patch, but it changes the formatting so I'm running the
> standard submission process instead of committing straight away.
>
> --
> /kinkie

I think we should be able to optimize the Range parsing a bit more than
this. The use of strchr() to identify the '-' is a bit exccessive,
especially when there is no '-' nearby (or anywhere) in the headers to
cause it to abort quickly.

IMO, we should locate each expected token in the order they occur then
validate the whole.

+ if - initial octet, flag as suffix handling (offset -1?),
    else call httpHeaderParseOffset() to set 'offset'.
+ if the next octet is '-', skip it,
    else abort as invalid.
+ if there is more data in the field, call httpHeaderParseOffset() to
set 'length',
    else handle as "A-" and return
+ if there is more data after the length (or non-digits) - abort as invalid.
+ verify the offset < length or offset==UnknownPosition (suffix range),
else abort as invalid

Amos
Received on Sat Jan 12 2013 - 10:39:38 MST