[master] 08bf3c8 Fail if ESI is attempted on partial (206) objects. Ignore gzipery on same.
Poul-Henning Kamp
phk at FreeBSD.org
Mon Feb 26 08:26:08 UTC 2018
commit 08bf3c8cc14495e02d20997fc16e4b5f166172e1
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon Feb 26 08:24:32 2018 +0000
Fail if ESI is attempted on partial (206) objects. Ignore gzipery on same.
Document workaround for ESI (also works for gzipery).
Fixes #2554
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 9faf4ee..d5e2548 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -501,7 +501,6 @@ vbf_stp_fetchbody(struct worker *wrk, struct busyobj *bo)
static int
vbf_figure_out_vfp(struct busyobj *bo)
{
- unsigned is_partial;
/*
* The VCL variables beresp.do_g[un]zip tells us how we want the
@@ -514,8 +513,22 @@ vbf_figure_out_vfp(struct busyobj *bo)
* no Content-Encoding --> object is not gzip'ed.
* anything else --> do nothing wrt gzip
*
+ * On partial responses (206 on pass), we fail if do_esi is
+ * requested because it could leak partial esi-directives, and
+ * ignore gzipery, because it makes no sense.
+ *
*/
+ if (http_GetStatus(bo->beresp) == 206) {
+ if (bo->do_esi) {
+ VSLb(bo->vsl, SLT_VCL_Error,
+ "beresp.do_esi on partial response");
+ return (-1);
+ }
+ bo->do_gzip = bo->do_gunzip = 0;
+ return (0);
+ }
+
/* No body or no GZIP support -> done */
if (bo->htc->body_status == BS_NONE ||
bo->htc->content_length == 0 ||
@@ -526,7 +539,6 @@ vbf_figure_out_vfp(struct busyobj *bo)
return (0);
}
- is_partial = http_GetStatus(bo->beresp) == 206;
bo->is_gzip = http_HdrIs(bo->beresp, H_Content_Encoding, "gzip");
bo->is_gunzip = !http_GetHdr(bo->beresp, H_Content_Encoding, NULL);
assert(bo->is_gzip == 0 || bo->is_gunzip == 0);
@@ -555,7 +567,7 @@ vbf_figure_out_vfp(struct busyobj *bo)
if (bo->do_gzip)
return (VFP_Push(bo->vfc, &VFP_gzip) == NULL ? -1 : 0);
- if (bo->is_gzip && !bo->do_gunzip && !is_partial)
+ if (bo->is_gzip && !bo->do_gunzip)
return (VFP_Push(bo->vfc, &VFP_testgunzip) == NULL ? -1 : 0);
return (0);
diff --git a/bin/varnishtest/tests/r02554.vtc b/bin/varnishtest/tests/r02554.vtc
new file mode 100644
index 0000000..672b766
--- /dev/null
+++ b/bin/varnishtest/tests/r02554.vtc
@@ -0,0 +1,58 @@
+varnishtest "VFP'ing partial responses"
+
+server s1 -repeat 4 {
+ rxreq
+ txresp -status 206 -body "<A><esi:remove>foobar</esi:remove>"
+} -start
+
+varnish v1 -vcl+backend {
+ sub vcl_recv { return(pass); }
+ sub vcl_backend_response {
+ if (bereq.url == "/a") {
+ set beresp.do_esi = True;
+ }
+ if (bereq.url == "/b") {
+ set beresp.do_gzip = True;
+ }
+ if (bereq.url == "/c") {
+ set beresp.do_esi = True;
+ set beresp.do_gzip = True;
+ set beresp.status = 1206;
+ }
+ }
+} -start
+
+client c1 {
+ txreq -url /a
+ rxresp
+ expect resp.status == 503
+} -run
+
+varnish v1 -vsl_catchup
+
+client c1 {
+ txreq -url /b -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.bodylen == 34
+ expect resp.status == 206
+} -run
+
+varnish v1 -vsl_catchup
+
+client c1 {
+ txreq -url /b
+ rxresp
+ expect resp.bodylen == 34
+ expect resp.status == 206
+} -run
+
+varnish v1 -vsl_catchup
+
+client c1 {
+ txreq -url /c -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 206
+ expect resp.bodylen == 32
+ gunzip
+ expect resp.bodylen == 3
+} -run
diff --git a/doc/sphinx/users-guide/esi.rst b/doc/sphinx/users-guide/esi.rst
index 0f16405..afa9236 100644
--- a/doc/sphinx/users-guide/esi.rst
+++ b/doc/sphinx/users-guide/esi.rst
@@ -4,8 +4,8 @@ Content composition with Edge Side Includes
-------------------------------------------
Varnish can create web pages by assembling different pages, called `fragments`,
-together into one page. These `fragments` can have individual cache policies. If you
-have a web site with a list showing the five most popular articles on
+together into one page. These `fragments` can have individual cache policies.
+If you have a web site with a list showing the five most popular articles on
your site, this list can probably be cached as a `fragment` and included
in all the other pages.
@@ -14,8 +14,9 @@ in all the other pages.
Used properly this strategy can dramatically increase
your hit rate and reduce the load on your servers.
-In Varnish we've only so far implemented a small subset of ESI. As of version 2.1 we
-have three ESI statements::
+In Varnish we've only implemented a small subset of ESI, because most of
+the rest of the ESI specifications facilities are easier and better done
+with VCL::
esi:include
esi:remove
@@ -81,10 +82,55 @@ For example::
<esi:include src="http://example.com/LICENSE" />
-->
+Footnotes about ESI
+-------------------
+
Doing ESI on JSON and other non-XML'ish content
------------------------------------------------
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Varnish will peek at the first byte of an object and if it is not
+a "<" Varnish assumes you didn't really mean to ESI process.
+You can alter this behaviour by::
+
+ param.set feature +esi_disable_xml_check
+
+Ignoring BOM in ESI objects
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If you backend spits out a Unicode Byte-Order-Mark as the first
+bytes of the reponse, the "<" check will fail unless you set::
+
+ param.set feature +esi_remove_bom
+
+ESI on invalid XML
+~~~~~~~~~~~~~~~~~~
+
+The ESI parser expects the XML to be reasonably well formed, but
+this may fail if you are ESI including non-XML files. You can
+make the ESI parser disrecard anything but ESI tags by setting:
+
+ param.set feature +esi_ignore_other_elements
+
+ESI includes with HTTPS protocol
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If ESI:include tags specify HTTPS protocol, it will be ignored
+by default, because varnish has no way to fetch it encryption
+enabled. If you want to treat HTTPS in ESI:include tags as if
+it were HTTP, set::
+
+ param.set feature +esi_ignore_https
+
+ESI on partial responses (206)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Varnish can ``pass`` range requests but it is ESI processing a partial
+response makes no sense, so the fetch fails if you do ask for that.
+If you really know what you are doing, you can use this workaround::
-Please note that Varnish will peek at the included content. If it
-doesn't start with a "<" Varnish assumes you didn't really mean to
-include it and disregard it. You can alter this behaviour by setting
-the 'esi_syntax' parameter (see ref:`ref-varnishd`).
+ sub vcl_backend_response {
+ if (beresp.status == 206 && beresp.http.secret == "swordfish") {
+ set beresp.do_esi = True;
+ set beresp.status = 1206;
+ }
+ }
More information about the varnish-commit
mailing list