[master] f4e3c27 Ensure we can always set a standard response code
Nils Goroll
nils.goroll at uplex.de
Mon Jul 4 13:42:07 CEST 2016
commit f4e3c278936c2db1d083d325b54d13476dc423e4
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Tue Jun 28 19:16:27 2016 +0200
Ensure we can always set a standard response code
The previous attempt to address 1990 was only half-hearted, the fundamental issue
was that we could still fail to set even a 503 response code because we required
workspace to stringify it.
For well known status codes, we now keep constant strings together with the
default reason phrases and emit those.
Add a vtc for the 1990 case.
Fixes #1990
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index ce56278..cbd741e 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -760,7 +760,7 @@ void VGZ_UpdateObj(const struct vfp_ctx *, struct vgz*, enum vgz_ua_e);
unsigned HTTP_estimate(unsigned nhttp);
void HTTP_Copy(struct http *to, const struct http * const fm);
struct http *HTTP_create(void *p, uint16_t nhttp);
-const char *http_Status2Reason(unsigned);
+const char *http_Status2Reason(unsigned, const char **);
unsigned http_EstimateWS(const struct http *fm, unsigned how);
void http_PutResponse(struct http *to, const char *proto, uint16_t status,
const char *response);
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 3772215..9983911 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -114,23 +114,27 @@ http_fail(const struct http *hp)
static struct http_msg {
unsigned nbr;
+ const char *status;
const char *txt;
} http_msg[] = {
-#define HTTP_RESP(n, t) { n, t},
+#define HTTP_RESP(n, t) { n, #n, t},
#include "tbl/http_response.h"
- { 0, NULL }
+ { 0, "0", NULL }
};
const char *
-http_Status2Reason(unsigned status)
+http_Status2Reason(unsigned status, const char **sstr)
{
struct http_msg *mp;
status %= 1000;
assert(status >= 100);
for (mp = http_msg; mp->nbr != 0 && mp->nbr <= status; mp++)
- if (mp->nbr == status)
+ if (mp->nbr == status) {
+ if (sstr)
+ *sstr = mp->status;
return (mp->txt);
+ }
return ("Unknown HTTP Status");
}
@@ -720,6 +724,8 @@ void
http_SetStatus(struct http *to, uint16_t status)
{
char buf[4];
+ const char *reason;
+ const char *sstr = NULL;
CHECK_OBJ_NOTNULL(to, HTTP_MAGIC);
/*
@@ -729,9 +735,15 @@ http_SetStatus(struct http *to, uint16_t status)
to->status = status;
status %= 1000;
assert(status >= 100);
- bprintf(buf, "%03d", status);
- http_PutField(to, HTTP_HDR_STATUS, buf);
- http_SetH(to, HTTP_HDR_REASON, http_Status2Reason(status));
+
+ reason = http_Status2Reason(status, &sstr);
+ if (sstr) {
+ http_SetH(to, HTTP_HDR_STATUS, sstr);
+ } else {
+ bprintf(buf, "%03d", status);
+ http_PutField(to, HTTP_HDR_STATUS, buf);
+ }
+ http_SetH(to, HTTP_HDR_REASON, reason);
}
/*--------------------------------------------------------------------*/
@@ -776,9 +788,8 @@ http_PutResponse(struct http *to, const char *proto, uint16_t status,
if (proto != NULL)
http_SetH(to, HTTP_HDR_PROTO, proto);
http_SetStatus(to, status);
- if (reason == NULL)
- reason = http_Status2Reason(status);
- http_SetH(to, HTTP_HDR_REASON, reason);
+ if (reason != NULL)
+ http_SetH(to, HTTP_HDR_REASON, reason);
}
/*--------------------------------------------------------------------
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index d3e832d..8a84e10 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -58,7 +58,7 @@ VRT_synth(VRT_CTX, unsigned code, const char *reason)
code = 503;
ctx->req->err_code = (uint16_t)code;
ctx->req->err_reason =
- reason ? reason : http_Status2Reason(ctx->req->err_code);
+ reason ? reason : http_Status2Reason(ctx->req->err_code, NULL);
}
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index a6934e7..b5273c6 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -454,14 +454,14 @@ HTTP1_DissectResponse(struct http_conn *htc, struct http *hp,
(int)(htc->rxbuf_e - htc->rxbuf_b), htc->rxbuf_b);
assert(retval >= 100 && retval <= 999);
assert(retval == 503);
- hp->status = retval;
- http_SetH(hp, HTTP_HDR_STATUS, "503");
- http_SetH(hp, HTTP_HDR_REASON, http_Status2Reason(retval));
+ http_SetStatus(hp, 503);
}
if (hp->hd[HTTP_HDR_REASON].b == NULL ||
- !Tlen(hp->hd[HTTP_HDR_REASON]))
- http_SetH(hp, HTTP_HDR_REASON, http_Status2Reason(hp->status));
+ !Tlen(hp->hd[HTTP_HDR_REASON])) {
+ http_SetH(hp, HTTP_HDR_REASON,
+ http_Status2Reason(hp->status, NULL));
+ }
htc->body_status = http1_body_status(hp, htc, 0);
diff --git a/bin/varnishtest/tests/r01990.vtc b/bin/varnishtest/tests/r01990.vtc
new file mode 100644
index 0000000..a430d63
--- /dev/null
+++ b/bin/varnishtest/tests/r01990.vtc
@@ -0,0 +1,44 @@
+varnishtest "workspace overflow with failed backend fetch"
+
+varnish v1 -vcl {
+ import debug;
+
+ backend default {
+ .host = "${bad_ip}";
+ .port = "9090";
+ }
+
+ sub vcl_backend_fetch {
+ # avoid LostHeader b Host: %s
+ set bereq.http.Host = "${bad_ip}";
+ debug.workspace_allocate(backend,
+ debug.workspace_free(backend));
+ }
+
+ sub vcl_backend_error {
+ # At this point we got no workspace to stringify
+ # the non-standard status 299, but we will leave
+ # the const "Unknown HTTP Status" reason
+ set beresp.status = 299;
+ }
+
+} -start
+
+logexpect l1 -v v1 -g raw {
+ expect * 1002 FetchError {^no backend connection}
+ expect * = BerespStatus {^503}
+ expect * = BerespReason {^Backend fetch failed}
+ expect * = Error {^out of workspace [(]Bo[)]}
+ expect * = LostHeader {^Date:}
+ expect * = Error {^out of workspace [(]Bo[)]}
+ expect * = LostHeader {^299}
+} -start
+
+client c1 {
+ txreq -url "/"
+ rxresp
+ expect resp.status == 503
+ expect resp.msg == "Unknown HTTP Status"
+} -run
+
+logexpect l1 -wait
More information about the varnish-commit
mailing list