New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chunked encoding + Content-Length = broken #203

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 5 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

From the discussion list:

I did the following:
1. checked out and built version 0.9.14.6-414
2. i updated the file src/third_party/serf/instaweb_response_buckets.c. I did 
the following:
#if 0
            /* Are we C-L, chunked, or conn close? */
            v = serf_bucket_headers_get(ctx->headers, "Content-Length");
            if (v) {
                apr_uint64_t length;
                length = apr_strtoi64(v, NULL, 10);
                if (errno == ERANGE) {
                    return APR_FROM_OS_ERROR(ERANGE);
                }
                ctx->body = serf_bucket_limit_create(ctx->body, length,
                                                     bkt->allocator);
            }
            else {
                v = serf_bucket_headers_get(ctx->headers, "Transfer-Encoding");

                /* Need to handle multiple transfer-encoding. */
                if (v && strcasecmp("chunked", v) == 0) {
                    ctx->chunked = 1;
                    ctx->body = serf_bucket_dechunk_create(ctx->body,
                                                           bkt->allocator);
                }

                if (!v && (ctx->sl.code == 204 || ctx->sl.code == 304)) {
                    ctx->state = STATE_DONE;
                }
            }
#endif
            if ((v = serf_bucket_headers_get(ctx->headers, "Transfer-Encoding")) != NULL)
            {
                /* Need to handle multiple transfer-encoding. */
                if (v && strcasecmp("chunked", v) == 0) {
                    ctx->chunked = 1;
                    ctx->body = serf_bucket_dechunk_create(ctx->body,
                                                           bkt->allocator);
                }
            }
            else if ((v = serf_bucket_headers_get(ctx->headers, "Content-Length")) != NULL)
            {
                apr_uint64_t length;
                length = apr_strtoi64(v, NULL, 10);
                if (errno == ERANGE) {
                    return APR_FROM_OS_ERROR(ERANGE);
                }
                ctx->body = serf_bucket_limit_create(ctx->body, length,
                                                     bkt->allocator);
            }
            else
            {
                if ((ctx->sl.code == 204 || ctx->sl.code == 304)) {
                    ctx->state = STATE_DONE;
                }
            }
This solved the problem.
Thanks
Gary

Original issue reported on code.google.com by sligocki@google.com on 31 Jan 2011 at 4:12

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This looks like it's actually a bug in the original Serf library itself 
(third_party/serf/instaweb_response_buckets.c), I'm working on the fix, but we 
should push this upstream to them as well (If they haven't already fixed it).

Original comment by sligocki@google.com on 31 Jan 2011 at 4:53

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Oops, the file I meant to ref was: 
third_party/serf/src/buckets/response_buckets.c

Original comment by sligocki@google.com on 31 Jan 2011 at 4:54

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

From Gary:

I took a look at the RFC
http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html. In section 4.4
it states "Messages MUST NOT include both a Content-Length header
field and a non-identity transfer-coding. If the message does include
a non- identity transfer-coding, the Content-Length MUST be ignored."
With the patch I saw a considerable improvement in the load time for
the page - thanks!
Thanks
Gary

Original comment by sligocki@google.com on 31 Jan 2011 at 5:52

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Should be fixed in trunk in r423, can you give that a try and verify that fix 
works?

I chose a slightly different patch than yours to preserve as much of the 
original code structure and style as possible. Specifically, it will act 
differently if your responses have Transfer-Encoding set to anything other than 
chunked.

Original comment by sligocki@google.com on 31 Jan 2011 at 7:25

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by sligocki@google.com on 1 Feb 2011 at 10:41

  • Added labels: release-note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment