Roy T. Fielding wrote:
>
> WTF? -1 Jim, that code is doing an error check prior to the
> strtol. It is not looking for the start of the number, but
> ensuring that the number is non-negative and all digits prior
> to calling the library routine. A simple check of *lenp would
> have been sufficient for the blank case.
>
> I need to go through the other changes as well, so I'll fix it,
> but don't release with this code.
Never said it was looking for the start... :)
The original code checked for all spaces and digits, which is well
and good. In this case, however, we just check for spaces, and if all
spaces, we don't bother with the ap_strtol() call. However, if
we *do* hit a non-space, we leverage the fact that ap_strtol() will
do the right thing. The first non-digit it sees (since we are
base 10) it will bail out and error, and we'll see the error with
the (endstr && *endstr) check. So we leverage the fact that ap_strtol()
knows all about digits and what are valid ones and we check for non-digits.
So the only other thing we need to check for is negative, which we do
(a later commit). With ap_strtol, the ap_isdigit() check is not needed
since we check for non-digits when we check the return values for
ap_strtol.
The below (with the follow-up commit) does exactly what we need.
>
> ....Roy
>
>
> On Tuesday, July 9, 2002, at 07:47 AM, [EMAIL PROTECTED] wrote:
>
> > jim 2002/07/09 07:47:24
> >
> > Modified: src CHANGES
> > src/main http_protocol.c
> > Log:
> > Allow for null/all-whitespace C-L fields as we did pre-1.3.26. However,
> > we do not allow for the total bogusness of values for C-L, just this
> > one special case. IMO a C-L field of "iloveyou" is bogus as is one
> > of "123yabbadabbado", which older versions appear to have allowed
> > (and in the 1st case, assume 0 and in the 2nd assume 123). Didn't
> > make sense to make this runtime, but a documented special case
> > instead.
> >
> > Revision Changes Path
> > 1.1836 +8 -0 apache-1.3/src/CHANGES
> >
> > Index: CHANGES
> > ===================================================================
> > RCS file: /home/cvs/apache-1.3/src/CHANGES,v
> > retrieving revision 1.1835
> > retrieving revision 1.1836
> > diff -u -r1.1835 -r1.1836
> > --- CHANGES 8 Jul 2002 18:06:54 -0000 1.1835
> > +++ CHANGES 9 Jul 2002 14:47:23 -0000 1.1836
> > @@ -1,5 +1,13 @@
> > Changes with Apache 1.3.27
> >
> > + *) In 1.3.26, a null or all blank Content-Length field would be
> > + triggered as an error; previous versions would silently ignore
> > + this and assume 0. As a special case, we now allow this and
> > + behave as we previously did. HOWEVER, previous versions would
> > + also silently accept bogus C-L values; We do NOT do that. That
> > + *is* an invalid value and we treat it as such.
> > + [Jim Jagielski]
> > +
> > *) Add ProtocolReqCheck directive, which determines if Apache will
> > check for a valid protocol string in the request (eg: HTTP/1.1)
> > and return HTTP_BAD_REQUEST if not valid. Versions of Apache
> >
> >
> >
> > 1.324 +8 -2 apache-1.3/src/main/http_protocol.c
> >
> > Index: http_protocol.c
> > ===================================================================
> > RCS file: /home/cvs/apache-1.3/src/main/http_protocol.c,v
> > retrieving revision 1.323
> > retrieving revision 1.324
> > diff -u -r1.323 -r1.324
> > --- http_protocol.c 8 Jul 2002 18:06:55 -0000 1.323
> > +++ http_protocol.c 9 Jul 2002 14:47:24 -0000 1.324
> > @@ -2011,10 +2011,16 @@
> > const char *pos = lenp;
> > int conversion_error = 0;
> >
> > - while (ap_isdigit(*pos) || ap_isspace(*pos))
> > + while (ap_isspace(*pos))
> > ++pos;
> >
> > if (*pos == '\0') {
> > + /* special case test - a C-L field NULL or all blanks is
> > + * assumed OK and defaults to 0. Otherwise, we do a
> > + * strict check of the field */
> > + r->remaining = 0;
> > + }
> > + else {
> > char *endstr;
> > errno = 0;
> > r->remaining = ap_strtol(lenp, &endstr, 10);
> > @@ -2023,7 +2029,7 @@
> > }
> > }
> >
> > - if (*pos != '\0' || conversion_error) {
> > + if (conversion_error) {
> > ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
> > "Invalid Content-Length");
> > return HTTP_BAD_REQUEST;
> >
> >
> >
> >
>
--
===========================================================================
Jim Jagielski [|] [EMAIL PROTECTED] [|] http://www.jaguNET.com/
"A society that will trade a little liberty for a little order
will lose both and deserve neither" - T.Jefferson
