Hi,<br><br>Attached is code proposed to resolve ticket #780 on <a href="http://vanish-cache.org">vanish-cache.org</a>. This ticket is about the final CRLF in a chunked fetch not being read by varnish. I have added the code to read 2 more bytes after finishing with the chunks and test that these are in fact CRLF. If they are not, it returns a fetch failed (-1).<br>
<br>I was a bit puzzled that this behaviour could exist for so long in varnish without it being noticed. The reason for that is probably that in most cases this would still work as the code in http_splitline skips over leading CRLF when looking at the next response on a reused backend. This would skip the lingering CRLF in the byte stream at that moment.<br>
<br>Because of the reading of CRLF in http_splitline, I have been unable to create a test case that would reliable fail without the fix. There was however a number of test cases that produced non-compliant chunked encoding data that would fail with the fix applied. (These tests were not adding the mandatory final CRLF to the byte stream). I have updated these test cases in the patch to add a final CRLF.<br>
<br>Discussion of trailing headers: With this patch, if a backend was to send us trailing headers the connection will fail. Without the patch, I would expect the next connection to fail instead, as the http response line processing would not match. I think that is worse behaviour than failing on the connection that has the traling headers. The patch could be extended to skip over trailing headers to ignore them but keep the connection reusable and don't fail the fetch, but I am not sure if it is worth the bother as noone is actually using trailing headers.<br>
<br>Regards,<br>Martin Blix Grydeland<br><br>The patch:<br><br>Index: varnish-cache/bin/varnishtest/tests/r00776.vtc<br>===================================================================<br>--- varnish-cache/bin/varnishtest/tests/r00776.vtc    (revision 5430)<br>
+++ varnish-cache/bin/varnishtest/tests/r00776.vtc    (working copy)<br>@@ -6,6 +6,7 @@<br>     rxreq<br>     txresp -nolen -hdr "Transfer-encoding: chunked"<br>     chunkedlen 4096<br>+    send "\r\n"<br>
 } -start<br> <br> varnish v1 \<br>Index: varnish-cache/bin/varnishtest/tests/r00387.vtc<br>===================================================================<br>--- varnish-cache/bin/varnishtest/tests/r00387.vtc    (revision 5430)<br>
+++ varnish-cache/bin/varnishtest/tests/r00387.vtc    (working copy)<br>@@ -10,6 +10,7 @@<br>     send "004\r\n1234\r\n"<br>     send "000000000000000000001\r\n@\r\n"<br>     send "00000000\r\n"<br>
+    send "\r\n"<br> } -start<br> <br> varnish v1 -vcl+backend {} -start<br>Index: varnish-cache/bin/varnishtest/tests/r00694.vtc<br>===================================================================<br>--- varnish-cache/bin/varnishtest/tests/r00694.vtc    (revision 5430)<br>
+++ varnish-cache/bin/varnishtest/tests/r00694.vtc    (working copy)<br>@@ -9,6 +9,7 @@<br>     send "\r\n"<br>     # This is chunksize (128k) + 2 to force to chunks to be allocated<br>     chunkedlen 131074<br>
+    send "\r\n"<br> } -start<br> <br> varnish v1 -vcl+backend {<br>Index: varnish-cache/bin/varnishtest/tests/b00007.vtc<br>===================================================================<br>--- varnish-cache/bin/varnishtest/tests/b00007.vtc    (revision 5430)<br>
+++ varnish-cache/bin/varnishtest/tests/b00007.vtc    (working copy)<br>@@ -10,6 +10,7 @@<br>     send "\r\n"<br>     send "00000004\r\n1234\r\n"<br>     send "00000000\r\n"<br>+    send "\r\n"<br>
 <br>     rxreq <br>     expect req.url == "/foo"<br>@@ -19,6 +20,7 @@<br>     send "00000004\r\n1234\r\n"<br>     chunked "1234"<br>     chunked ""<br>+    send "\r\n"<br>
 } -start<br> <br> varnish v1 -vcl+backend {} -start<br>Index: varnish-cache/bin/varnishd/cache_fetch.c<br>===================================================================<br>--- varnish-cache/bin/varnishd/cache_fetch.c    (revision 5430)<br>
+++ varnish-cache/bin/varnishd/cache_fetch.c    (working copy)<br>@@ -207,6 +207,11 @@<br>         q = bp = buf + v;<br>     }<br> <br>+    /* Expect a CRLF to trail the chunks */<br>+    i = HTC_Read(htc, buf, 2);<br>+    if (i != 2 || buf[0] != '\r' || buf[1] != '\n')<br>
+        return -1;<br>+<br>     if (st != NULL && st->len == 0) {<br>         VTAILQ_REMOVE(&sp->obj->store, st, list);<br>         STV_free(st);<br><br clear="all"><br>-- <br>Martin Blix Grydeland<br>
Varnish Software AS<br><br>