You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 2000/12/15 15:16:50 UTC
PR 6973 / 1.3.15 showstopper
Seems to me that Tony's patch is in the right direction. I think the
if (*start == *end)
return 0;
block should be inserted in line 164 though.
--
===========================================================================
Jim Jagielski [|] jim@jaguNET.com [|] http://www.jaguNET.com/
"Casanova will have many weapons; To beat him you will
have to have more than forks and flatulence."
Re: PR 6973 / 1.3.15 showstopper
Posted by Tony Finch <do...@dotat.at>.
"William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:
>
>Don't we fall down on null elements at the moment?
Probably, hence my comment about ap_getword being the wrong thing.
Tony.
--
f.a.n.finch fanf@covalent.net dot@dotat.at
"Well, as long as they can think we'll have our problems.
But those whom we're using cannot think."
RE: PR 6973 / 1.3.15 showstopper
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> From: Tony Finch [mailto:dot@dotat.at]
> Sent: Friday, December 15, 2000 5:15 PM
> To: new-httpd@apache.org; jim@jaguNET.com
> Subject: Re: PR 6973 / 1.3.15 showstopper
>
>
> Jim Jagielski <ji...@jaguNET.com> wrote:
> >
> >Is whitespace allowed? The copy of the RFP I have doesn't say, but
> >it certainly implies it's not.
>
> [...]
>
> #rule
> A construct "#" is defined, similar to "*", for defining lists of
> elements. The full form is "<n>#<m>element" indicating at least
> <n> and at most <m> elements, each separated by one or more commas
> (",") and OPTIONAL linear white space (LWS). This makes the usual
> form of lists very easy; a rule such as
> ( *LWS element *( *LWS "," *LWS element ))
> can be shown as
> 1#element
> Wherever this construct is used, null elements are allowed, but do
> not contribute to the count of elements present. That is,
> "(element), , (element) " is permitted, but counts as only two
> elements. Therefore, where at least one element is required, at
> least one non-null element MUST be present.
Don't we fall down on null elements at the moment?
Re: PR 6973 / 1.3.15 showstopper
Posted by Tony Finch <do...@dotat.at>.
Jim Jagielski <ji...@jaguNET.com> wrote:
>
>Is whitespace allowed? The copy of the RFP I have doesn't say, but
>it certainly implies it's not.
[...]
#rule
A construct "#" is defined, similar to "*", for defining lists of
elements. The full form is "<n>#<m>element" indicating at least
<n> and at most <m> elements, each separated by one or more commas
(",") and OPTIONAL linear white space (LWS). This makes the usual
form of lists very easy; a rule such as
( *LWS element *( *LWS "," *LWS element ))
can be shown as
1#element
Wherever this construct is used, null elements are allowed, but do
not contribute to the count of elements present. That is,
"(element), , (element) " is permitted, but counts as only two
elements. Therefore, where at least one element is required, at
least one non-null element MUST be present. Default values are 0
and infinity so that "#element" allows any number, including zero;
"1#element" requires at least one; and "1#2element" allows one or
two.
[...]
byte-ranges-specifier = bytes-unit "=" byte-range-set
byte-range-set = 1#( byte-range-spec | suffix-byte-range-spec )
byte-range-spec = first-byte-pos "-" [last-byte-pos]
first-byte-pos = 1*DIGIT
last-byte-pos = 1*DIGIT
ISTM that ap_getword in util.c is a very poor implementation of the
#rule. Didn't this come up with reference to cookies?
Tony.
--
f.a.n.finch fanf@covalent.net dot@dotat.at
" ``Well, let's go down and find out who's grave it is.''
``How?'' ``By going down and finding out!'' "
Re: PR 6973 / 1.3.15 showstopper
Posted by Tony Finch <do...@dotat.at>.
"William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:
>
>Ok... based on our conjecture that #<sp>-<sp># is absolutely invalid, and that
>someday we will do things the 'right way' - here is the patch - sort of back
>to where I was two days ago. Note, especially, that this can be easily cleaned
>up for the new behavior, the #ifdef's are temporary things.
No substantive comments (I'm drunk) but "ternary" isn't spelt the way
you spelt it :-)
Tony.
--
f.a.n.finch fanf@covalent.net dot@dotat.at
"If I didn't see it with my own eyes I would never have believed it!"
RE: PR 6973 / 1.3.15 showstopper
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Oops... small typos, sorry. Try this on for size, instead.
-----Original Message-----
From: William A. Rowe, Jr. [mailto:wrowe@rowe-clan.net]
Sent: Friday, December 15, 2000 11:48 PM
To: new-httpd@apache.org
Subject: RE: PR 6973 / 1.3.15 showstopper
Ok... based on our conjecture that #<sp>-<sp># is absolutely
invalid, and that
someday we will do things the 'right way' - here is the patch -
sort of back
to where I was two days ago. Note, especially, that this can
be easily cleaned
up for the new behavior, the #ifdef's are temporary things.
static int parse_byterange(char *range, long clength, long
*start, long *end)
{
char *off, *off2;
long lead, tail;
/* Look for leading dash */
while (ap_isspace(*range))
++range;
if (*(range) == '-')
off = range + 1;
else if (ap_isdigit(*range)) {
/* Parse first value */
lead = strtol(range, &off, 10);
/* Skip dash */
if (*(off) != '-') /* Dash is -required- */
#ifdef TERNIARY_BYTERANGE
return -1;
#else
return 0;
#endif
++off;
}
#ifdef TERNIARY_BYTERANGE
else if (*range)
return -1; /* Dash was -required- */
#endif
else
return 0; /* Empty is syntactically valid */
/* Parse second value */
if (ap_isdigit(*off))
tail = strtol(off, &off2, 10);
else
off2 = off;
/* Confirm end of segment */
while (ap_isspace(*off2))
++off2;
if (*off2) /* nothing else is valid */
#ifdef TERNIARY_BYTERANGE
return -1;
#else
return 0;
#endif
if (*range == '-')
{
/* format was "-5" */
*start = clength - tail;
*end = clength - 1;
#ifdef TERNIARY_BYTERANGE
if (tail < 1)
return -1;
#endif
if (*start < 0) /* Correct "-5" longer than clength */
*start = 0;
}
else {
*start = lead;
if (!ap_isdigit(*off)) /* format was "5-" */
*end = clength - 1;
else { /* format was "5-9" */
*end = tail;
#ifdef TERNIARY_BYTERANGE
if (*start > *end) /* "9-5" is invalid */
return -1;
#endif
if (*end >= clength) /* End of "5-9" is past clength */
*end = clength - 1;
}
}
/* This is a more general test; catch effectively empty responses */
if (*start > *end)
return 0;
return 1;
}
RE: PR 6973 / 1.3.15 showstopper
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
As Tony noted, there was one error below (see -/+). Also,
note that nolead = (off == range); actually does something,
in the case that *range == "X" or some other nonsense.
Jim, Tony's and I have both parsed this against the spec ... it is
actually a little liberal in that <sp>#<sp>-<sp>#<sp> is allowed,
but my reading of the spec is that only <sp>#-#<sp> is permitted.
Which reading do you agree with?
If you like, feel free to apply, and change to exclude the spaces
permitted between #-# if you rather.
Bill
> -----Original Message-----
> From: William A. Rowe, Jr. [mailto:wrowe@rowe-clan.net]
> Sent: Friday, December 15, 2000 4:34 PM
> To: new-httpd@apache.org
> Subject: RE: PR 6973 / 1.3.15 showstopper
>
>
> Not a diff... this is a little meaty, needs reading ...
> not the prettiest thing either, but moderately optimal, fel
> free to improve.
>
> static int parse_byterange(char *range, long clength, long
> *start, long *end)
> {
> int nolead, notail;
> char *off, *off2;
> long lead, tail;
>
> /* Look for leading dash */
> while (ap_isspace(*range))
> ++range;
> if (*(range) == '-') {
> nolead = 1;
- off = range;
+ off = range + 1;
> }
> else {
> /* Parse first value */
> lead = strtol(range, &off, 10);
> nolead = (off == range);
>
> /* Confirm and skip dash */
> while (ap_isspace(*off))
> ++off;
> if (*(off) != '-') /* Dash is -required- */
> return 0;
> ++off;
> }
>
> /* Parse second value */
> tail = strtol(off, &off2, 10);
> notail = (off == off2);
>
> /* Confirm end of text */
> while (ap_isspace(*off2))
> ++off2;
> if (*off2) /* nothing else allowed */
> return 0;
>
> if (nolead)
> {
> if (notail) /* "-" alone is insufficient */
> return 0;
> /* else format was "-5", could be "--5" */
> *start = clength - tail;
> *end = clength - 1;
>
> if (*start < 0) /* Correct "-5" longer than clength */
> *start = 0;
> }
> else {
> *start = lead;
> if (notail) /* format was "5-" */
> *end = clength - 1;
> else {
> /* format was "5-6", could be "5-4" */
> *end = tail;
>
> if (*end >= clength) /* Correct end of "5-9" past
> clength */
> *end = clength - 1;
> }
> }
>
> if (*start > *end) /* "5-4", "--5", start beyond length
> all fall out */
> return 0;
>
> return 1;
> }
>
>
>
RE: PR 6973 / 1.3.15 showstopper
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Not a diff... this is a little meaty, needs reading ...
not the prettiest thing either, but moderately optimal, fel free to improve.
static int parse_byterange(char *range, long clength, long *start, long *end)
{
int nolead, notail;
char *off, *off2;
long lead, tail;
/* Look for leading dash */
while (ap_isspace(*range))
++range;
if (*(range) == '-') {
nolead = 1;
off = range;
}
else {
/* Parse first value */
lead = strtol(range, &off, 10);
nolead = (off == range);
/* Confirm and skip dash */
while (ap_isspace(*off))
++off;
if (*(off) != '-') /* Dash is -required- */
return 0;
++off;
}
/* Parse second value */
tail = strtol(off, &off2, 10);
notail = (off == off2);
/* Confirm end of text */
while (ap_isspace(*off2))
++off2;
if (*off2) /* nothing else allowed */
return 0;
if (nolead)
{
if (notail) /* "-" alone is insufficient */
return 0;
/* else format was "-5", could be "--5" */
*start = clength - tail;
*end = clength - 1;
if (*start < 0) /* Correct "-5" longer than clength */
*start = 0;
}
else {
*start = lead;
if (notail) /* format was "5-" */
*end = clength - 1;
else {
/* format was "5-6", could be "5-4" */
*end = tail;
if (*end >= clength) /* Correct end of "5-9" past clength */
*end = clength - 1;
}
}
if (*start > *end) /* "5-4", "--5", start beyond length all fall out */
return 0;
return 1;
}
RE: PR 6973 / 1.3.15 showstopper
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ok... based on our conjecture that #<sp>-<sp># is absolutely invalid, and that
someday we will do things the 'right way' - here is the patch - sort of back
to where I was two days ago. Note, especially, that this can be easily cleaned
up for the new behavior, the #ifdef's are temporary things.
static int parse_byterange(char *range, long clength, long *start, long *end)
{
char *off, *off2;
long lead, tail;
/* Look for leading dash */
while (ap_isspace(*range))
++range;
if (*(range) == '-')
off = range + 1;
else if (ap_isdigit(*range)) {
/* Parse first value */
lead = strtol(range, &off, 10);
/* Skip dash */
if (*(off) != '-') /* Dash is -required- */
#ifdef TERNIARY_BYTERANGE
return -1;
#else
return 0;
#endif
++off;
}
#ifdef TERNIARY_BYTERANGE
else if (*range)
return -1; /* Dash was -required- */
#endif
else
return 0; /* Empty is syntactically valid */
/* Parse second value */
if (ap_isdigit(*off))
tail = strtol(off, &off2, 10);
else
off2 = off;
/* Confirm end of segment */
while (ap_isspace(*off2))
++off2;
if (*off2) /* nothing else is valid */
#ifdef TERNIARY_BYTERANGE
return -1;
#else
return 0;
#endif
if (*(range) == '-')
{
/* format was "-5" */
*start = clength - tail;
*end = clength - 1;
if (*start < 0) /* Correct "-5" longer than clength */
*start = 0;
}
else {
*start = lead;
if (!ap_isdigit(*off)) /* format was "5-9" */
*end = tail;
else { /* format was "5-" */
*end = clength - 1;
#ifdef TERNIARY_BYTERANGE
if (*start > *end) /* "9-5" is invalid */
return -1;
#endif
if (*end >= clength) /* End of "5-9" is past clength */
*end = clength - 1;
}
}
#ifdef TERNIARY_BYTERANGE
/* Effectively empty response */
if (*start > clength)
#else
/* This is more optimal for a binary response*/
if (*start > *end)
#endif
return 0;
return 1;
}
Re: PR 6973 / 1.3.15 showstopper
Posted by Tony Finch <do...@dotat.at>.
Jim Jagielski <ji...@jaguNET.com> wrote:
>
>I think it's that final return statement... Should be:
>
> (*start >= 0 || *end <= clength - 1)
I think it should just be 1, and your test above is equivalent to
that. That is all the change that is needed for a fix to the immediate
problem; the other chunk in the patch I added to the PR is bogus.
I've been talking with wrowe about this and we agree that there are
some infelicities with the code as it is right now: it doesn't handle
whitespace properly and it doesn't check the syntax of numbers properly.
Bill's going to work up a more thorough patch.
Tony.
--
f.a.n.finch fanf@covalent.net dot@dotat.at
" ``Well, let's go down and find out who's grave it is.''
``How?'' ``By going down and finding out!'' "
RE: PR 6973 / 1.3.15 showstopper
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Nak... it doesn't catch legitimate requests for -1.
if (*start > *end) already catches -0, --1 etc.
> -----Original Message-----
> From: Jim Jagielski [mailto:jim@jaguNET.com]
> Sent: Friday, December 15, 2000 8:17 AM
> To: new-httpd@apache.org
> Subject: PR 6973 / 1.3.15 showstopper
>
>
> Seems to me that Tony's patch is in the right direction. I think the
>
> if (*start == *end)
> return 0;
>
> block should be inserted in line 164 though.
>
> --
> ==============================================================
> =============
> Jim Jagielski [|] jim@jaguNET.com [|]
> http://www.jaguNET.com/
> "Casanova will have many weapons; To beat him you will
> have to have more than forks and flatulence."
>