Forgot to add varnish-dev on this. -Martin
---------- Forwarded message ---------- From: Martin Blix Grydeland <[email protected]> Date: Mon, Oct 25, 2010 at 14:21 Subject: Re: Request for code review of varnish-cache.org ticket #780 (Read CRLF in fetch_chunked) To: Poul-Henning Kamp <[email protected]> On Mon, Oct 18, 2010 at 16:52, Poul-Henning Kamp <[email protected]> wrote: > So I think the right logic is more like: > > c = Rx one char > if (c == CR) > Rx one char > if (c != LF) > accept fetch > log error notice > close connection to prevent reuse > > Second go at this. I've kept the changes to the test cases for them to be RFC compliant, and implemented your pseudo-code. I leave the closing of the connection to the calling code, but return a value of 1 to indicate the connection is not safe to reuse. Martin Index: bin/varnishtest/tests/r00776.vtc =================================================================== --- bin/varnishtest/tests/r00776.vtc (revision 5463) +++ bin/varnishtest/tests/r00776.vtc (working copy) @@ -6,6 +6,7 @@ rxreq txresp -nolen -hdr "Transfer-encoding: chunked" chunkedlen 4096 + send "\r\n" } -start varnish v1 \ Index: bin/varnishtest/tests/r00387.vtc =================================================================== --- bin/varnishtest/tests/r00387.vtc (revision 5463) +++ bin/varnishtest/tests/r00387.vtc (working copy) @@ -10,6 +10,7 @@ send "004\r\n1234\r\n" send "000000000000000000001\...@\r\n" send "00000000\r\n" + send "\r\n" } -start varnish v1 -vcl+backend {} -start Index: bin/varnishtest/tests/r00694.vtc =================================================================== --- bin/varnishtest/tests/r00694.vtc (revision 5463) +++ bin/varnishtest/tests/r00694.vtc (working copy) @@ -9,6 +9,7 @@ send "\r\n" # This is chunksize (128k) + 2 to force to chunks to be allocated chunkedlen 131074 + send "\r\n" } -start varnish v1 -vcl+backend { Index: bin/varnishtest/tests/b00007.vtc =================================================================== --- bin/varnishtest/tests/b00007.vtc (revision 5463) +++ bin/varnishtest/tests/b00007.vtc (working copy) @@ -10,6 +10,7 @@ send "\r\n" send "00000004\r\n1234\r\n" send "00000000\r\n" + send "\r\n" rxreq expect req.url == "/foo" @@ -19,6 +20,7 @@ send "00000004\r\n1234\r\n" chunked "1234" chunked "" + send "\r\n" } -start varnish v1 -vcl+backend {} -start Index: bin/varnishd/cache_fetch.c =================================================================== --- bin/varnishd/cache_fetch.c (revision 5463) +++ bin/varnishd/cache_fetch.c (working copy) @@ -207,6 +207,16 @@ q = bp = buf + v; } + /* Expect a CRLF to trail the chunks */ + i = HTC_Read(htc, buf, 1); + if (i == 1 && buf[0] == '\r') + i = HTC_Read(htc, buf, 1); + if (i != 1 || (i==1 && buf[0] != '\n')) { + WSP(sp, SLT_FetchError, + "chunked missing trailing crlf"); + return 1; /* Accept fetch, but do not reuse connection */ + } + if (st != NULL && st->len == 0) { VTAILQ_REMOVE(&sp->obj->store, st, list); STV_free(st); -- Martin Blix Grydeland Varnish Software AS -- Martin Blix Grydeland Varnish Software AS
_______________________________________________ varnish-dev mailing list [email protected] http://lists.varnish-cache.org/mailman/listinfo/varnish-dev
