[PATCH] Make http_PutProtocol() and http_PutResponse() never fail, and increase make the synthetic object's workspace be param->http_resp_size

[Poul-Henning Kamp]

Works for me:  Commit it.

In message <1318541712-25077-2-git-send-email-martin at varnish-software.com>, Mar
tin Blix Grydeland writes:
>The http_PutProtocol() and http_PutResponse() would in the case of
>workspace overflow leave the headers as NULL and log a
>SLT_LostHeader. This would make Varnish assert correctly later when
>writing to the wire, as these are mandated by HTTP. This patch changes
>them to set the fields to static strings instead ("HTTP/1.1" and "Lost
>Response") when failing to write them to the workspace. This leaves
>enough information to complete the protocol in the case of overflow.
>
>The patch also increases the synthetic object's workspace from static
>1024 to param->http_resp_size. This leaves more (and configurable)
>room for manipulating the headers of the synthetic object in
>vcl_error.
>
>Test case by Kristian
>
>Fixes: #1031
>---
> bin/varnishd/cache_center.c      |    8 ++++----
> bin/varnishd/cache_http.c        |    6 ++++++
> bin/varnishtest/tests/r01031.vtc |   30 ++++++++++++++++++++++++++++++
> 3 files changed, 40 insertions(+), 4 deletions(-)
> create mode 100644 bin/varnishtest/tests/r01031.vtc
>
>diff --git a/bin/varnishd/cache_center.c b/bin/varnishd/cache_center.c
>index 39a2104..4d94d88 100644
>--- a/bin/varnishd/cache_center.c
>+++ b/bin/varnishd/cache_center.c
>@@ -436,13 +436,13 @@ cnt_error(struct sess *sp)
> 	w = sp->wrk;
> 	if (sp->obj == NULL) {
> 		HSH_Prealloc(sp);
>-		/* XXX: 1024 is a pure guess */
> 		EXP_Clr(&w->exp);
>-		sp->obj = STV_NewObject(sp, NULL, 1024, &w->exp,
>-		     (uint16_t)params->http_max_hdr);
>+		sp->obj = STV_NewObject(sp, NULL, params->http_resp_size,
>+		     &w->exp, (uint16_t)params->http_max_hdr);
> 		if (sp->obj == NULL)
> 			sp->obj = STV_NewObject(sp, TRANSIENT_STORAGE,
>-			    1024, &w->exp, (uint16_t)params->http_max_hdr);
>+			     params->http_resp_size , &w->exp,
>+			     (uint16_t)params->http_max_hdr);
> 		if (sp->obj == NULL) {
> 			sp->doclose = "Out of objects";
> 			sp->director = NULL;
>diff --git a/bin/varnishd/cache_http.c b/bin/varnishd/cache_http.c
>index f77ab74..5ce1b6a 100644
>--- a/bin/varnishd/cache_http.c
>+++ b/bin/varnishd/cache_http.c
>@@ -981,6 +981,9 @@ http_PutProtocol(struct worker *w, int fd, const struct http *to,
> {
> 
> 	http_PutField(w, fd, to, HTTP_HDR_PROTO, protocol);
>+	if (to->hd[HTTP_HDR_PROTO].b == NULL)
>+		http_SetH(to, HTTP_HDR_PROTO, "HTTP/1.1");
>+	Tcheck(to->hd[HTTP_HDR_PROTO]);
> }
> 
> void
>@@ -997,6 +1000,9 @@ http_PutResponse(struct worker *w, int fd, const struct http *to,
> {
> 
> 	http_PutField(w, fd, to, HTTP_HDR_RESPONSE, response);
>+	if (to->hd[HTTP_HDR_RESPONSE].b == NULL)
>+		http_SetH(to, HTTP_HDR_RESPONSE, "Lost Response");
>+	Tcheck(to->hd[HTTP_HDR_RESPONSE]);
> }
> 
> void
>diff --git a/bin/varnishtest/tests/r01031.vtc b/bin/varnishtest/tests/r01031.vtc
>new file mode 100644
>index 0000000..435f973
>--- /dev/null
>+++ b/bin/varnishtest/tests/r01031.vtc
>@@ -0,0 +1,30 @@
>+varnishtest "Test overflowing the response through sp->err_reason"
>+
>+varnish v1 -vcl {
>+	backend blatti {
>+		.host = "127.0.0.1";
>+	}
>+
>+	sub vcl_recv {
>+		error 200 "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
>+	}
>+	sub vcl_error {
>+		return(deliver);
>+	}
>+} -start
>+
>+client c1 {
>+	txreq -req GET
>+	rxresp
>+	expect resp.status == 200
>+	expect resp.msg == "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
>+} -run
>+
>+varnish v1 -cliok "param.set http_resp_size 256"
>+
>+client c2 {
>+	txreq -req GET
>+	rxresp
>+	expect resp.status == 200
>+	expect resp.msg == "Lost Response"
>+} -run
>-- 
>1.7.4.1
>
>
>_______________________________________________
>varnish-dev mailing list
>varnish-dev at varnish-cache.org
>https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
>

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.