You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2002/07/09 16:47:24 UTC
cvs commit: apache-1.3/src/main http_protocol.c
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;
Re: cvs commit: apache-1.3/src/main http_protocol.c
Posted by David Burry <db...@tagnet.org>.
While we are debating the best way to accomplish this Content-Length fix for the next release, I kind of need to have it working for me right now in the current released version. Therefore I've implemented this partial fix against 1.3.26 on my system:
root@nueva[391] pwd
/usr/share/src/packages/apache_1.3.26/src/main
root@nueva[392] diff -c http_protocol.original.c http_protocol.c
*** http_protocol.original.c Tue Jul 9 13:35:54 2002
--- http_protocol.c Tue Jul 9 12:35:59 2002
***************
*** 1991,1997 ****
r->read_chunked = 1;
}
! else if (lenp) {
const char *pos = lenp;
int conversion_error = 0;
--- 1991,1997 ----
r->read_chunked = 1;
}
! else if (lenp && *lenp != '\0') {
const char *pos = lenp;
int conversion_error = 0;
Admittedly it only opens up empty string (blank) Content-Length values to default to 0, not white space ones, but I think that's all I really need to get me going for now until the next release. I believe this may be the "simple check of *lenp" that Roy was talking about. Since I'm brand new to the Apache source code in general, and not really a C expert either, any comments or criticisms are welcome regarding this.
Dave
At 11:18 AM 7/9/2002 -0700, 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.
Re: cvs commit: apache-1.3/src/main http_protocol.c
Posted by "Roy T. Fielding" <fi...@apache.org>.
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.
....Roy
On Tuesday, July 9, 2002, at 07:47 AM, jim@apache.org 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;
>
>
>
>