[master] 601ee74 Ensure obj->response is set sensibly for errors
Kristian Lyngstøl
kristian at varnish-cache.org
Mon Oct 17 13:35:30 CEST 2011
commit 601ee74203b8d0088251c43c5e5de380dabdcba0
Author: Kristian Lyngstol <kristian at bohemians.org>
Date: Mon Oct 17 13:30:29 2011 +0200
Ensure obj->response is set sensibly for errors
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 commit 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.
This whole thing has been a collaboration between Martin and myself. I'll
leave it a mystery who wrote what line of code, which part of the comment
and contributed what to the test-case.
In all fairness, it's not a prefect solution, but a far step closer to one.
So it sort of, kinda, more or less, for now, until we get a better
solution:
Fixes: #1031
diff --git a/bin/varnishd/cache_center.c b/bin/varnishd/cache_center.c
index 47139c7..6bf0b0c 100644
--- a/bin/varnishd/cache_center.c
+++ b/bin/varnishd/cache_center.c
@@ -435,13 +435,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 dd55990..aaa8212 100644
--- a/bin/varnishd/cache_http.c
+++ b/bin/varnishd/cache_http.c
@@ -979,6 +979,9 @@ http_PutProtocol(struct worker *w, unsigned vsl_id, const struct http *to,
{
http_PutField(w, vsl_id, 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
@@ -995,6 +998,10 @@ http_PutResponse(struct worker *w, unsigned vsl_id, const struct http *to,
{
http_PutField(w, vsl_id, 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 "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+ }
+ sub vcl_error {
+ return(deliver);
+ }
+} -start
+
+client c1 {
+ txreq -req GET
+ rxresp
+ expect resp.status == 200
+ expect resp.msg == "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+} -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
More information about the varnish-commit
mailing list