You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ni...@apache.org on 2007/11/17 15:36:58 UTC

svn commit: r595954 - /httpd/httpd/trunk/modules/http/http_filters.c

Author: niq
Date: Sat Nov 17 06:36:58 2007
New Revision: 595954

URL: http://svn.apache.org/viewvc?rev=595954&view=rev
Log:
Safer fix to PR43882 than in r595672.

Modified:
    httpd/httpd/trunk/modules/http/http_filters.c

Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=595954&r1=595953&r2=595954&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Sat Nov 17 06:36:58 2007
@@ -115,11 +115,11 @@
         lenp = apr_table_get(f->r->headers_in, "Content-Length");
 
         if (tenc) {
-            /* RFC2616 allows qualifiers, so use strncasecmp */
-            if (!strncasecmp(tenc, "chunked", 7) && !ap_strchr_c(tenc, ',')) {
+            if (!strcasecmp(tenc, "chunked")) {
                 ctx->state = BODY_CHUNK;
             }
-            else {
+	    /* test lenp, because it gives another case we can handle */
+            else if (!lenp) {
                 /* Something that isn't in HTTP, unless some future
                  * edition defines new transfer ecodings, is unsupported.
                  */



Re: svn commit: r595954 - /httpd/httpd/trunk/modules/http/http_filters.c

Posted by Nick Kew <ni...@webthing.com>.
On Mon, 26 Nov 2007 12:17:16 -0500
"Jeff Trawick" <tr...@gmail.com> wrote:

> On Nov 26, 2007 11:50 AM, Nick Kew <ni...@webthing.com> wrote:
> > On Mon, 26 Nov 2007 10:38:28 -0500
> > "Jeff Trawick" <tr...@gmail.com> wrote:
> >
> > > What is the intention for the UNHANDLED case?    The code/comments
> > > seem to imply we'll end up in the "respect CL" path.
> >
> > Exactly.
> 
> Cool; we're in sync so far, which brings us to my concern: We won't
> end up in the "respect CL" path ;)

Aha.  Neither would we before, so no possible regression.

> Finding any Transfer-Encoding, supported or not, results in bypassing
> this support for CL:
> 
>         else if (lenp) {
>             char *endstr;
> 
>             ctx->state = BODY_LENGTH;
>             errno = 0;

Indeed.  Fixing that seems to make sense, but it'll need the
backport vote to be reset.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Re: svn commit: r595954 - /httpd/httpd/trunk/modules/http/http_filters.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Nov 26, 2007 11:50 AM, Nick Kew <ni...@webthing.com> wrote:
> On Mon, 26 Nov 2007 10:38:28 -0500
> "Jeff Trawick" <tr...@gmail.com> wrote:
>
> > What is the intention for the UNHANDLED case?    The code/comments
> > seem to imply we'll end up in the "respect CL" path.
>
> Exactly.

Cool; we're in sync so far, which brings us to my concern: We won't
end up in the "respect CL" path ;)

Finding any Transfer-Encoding, supported or not, results in bypassing
this support for CL:

        else if (lenp) {
            char *endstr;

            ctx->state = BODY_LENGTH;
            errno = 0;

Re: svn commit: r595954 - /httpd/httpd/trunk/modules/http/http_filters.c

Posted by Nick Kew <ni...@webthing.com>.
On Mon, 26 Nov 2007 10:38:28 -0500
"Jeff Trawick" <tr...@gmail.com> wrote:

> What is the intention for the UNHANDLED case?    The code/comments
> seem to imply we'll end up in the "respect CL" path.

Exactly.

The alternative is to reject it, which might risk breaking
something that worked before.  The current fix falls back
to old behaviour in the have-CL-bad-TE case, so there's
no risk of regression.

What else do you think we should do in that case?
Perhaps logging an error would be a good idea.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Re: svn commit: r595954 - /httpd/httpd/trunk/modules/http/http_filters.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Nov 17, 2007 9:36 AM,  <ni...@apache.org> wrote:
> Author: niq
> Date: Sat Nov 17 06:36:58 2007
> New Revision: 595954
>
> URL: http://svn.apache.org/viewvc?rev=595954&view=rev
> Log:
> Safer fix to PR43882 than in r595672.
>
> Modified:
>     httpd/httpd/trunk/modules/http/http_filters.c
>
> Modified: httpd/httpd/trunk/modules/http/http_filters.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=595954&r1=595953&r2=595954&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_filters.c (original)
> +++ httpd/httpd/trunk/modules/http/http_filters.c Sat Nov 17 06:36:58 2007
> @@ -115,11 +115,11 @@
>          lenp = apr_table_get(f->r->headers_in, "Content-Length");
>
>          if (tenc) {
> -            /* RFC2616 allows qualifiers, so use strncasecmp */
> -            if (!strncasecmp(tenc, "chunked", 7) && !ap_strchr_c(tenc, ',')) {
> +            if (!strcasecmp(tenc, "chunked")) {
>                  ctx->state = BODY_CHUNK;
>              }
> -            else {
> +           /* test lenp, because it gives another case we can handle */
> +            else if (!lenp) {
>                  /* Something that isn't in HTTP, unless some future
>                   * edition defines new transfer ecodings, is unsupported.

The overall flow now looks like:

if Transfer-Encoding {
    if Transfer-Encoding == chunked {
        respect it
    }
    else if no Content-Length {
        log it and return an error
    }
    else {
        UNHANDLED case with unrecognized Transfer-Encoding as well as
Content-Length
    }
}
else if Content-Length {
    respect CL
}

What is the intention for the UNHANDLED case?    The code/comments
seem to imply we'll end up in the "respect CL" path.