From b16b6ed53f3153a1da79d5054f67fe552c2c5617 Mon Sep 17 00:00:00 2001 From: Walid Boudebouda Date: Thu, 16 Mar 2023 18:32:35 +0100 Subject: [PATCH 1/9] fix bug in abstract sockets with uds abstract sockets, sun_path should start with a NULL character followed by the socket's name. The name is not considered to be NULL terminated and can contain NULL bytes which have no special meaning. socklen is used to determine the length of name and must be set to the length of the struct sockaddr_un up to the last character of name, otherwise the 108 characters of sun_path will be treated as the name of the socket, including NULL bytes. --- bin/varnishtest/vtc_client.c | 4 +++- include/vus.h | 1 + lib/libvarnish/vus.c | 29 +++++++++++++++++++++++++++-- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/bin/varnishtest/vtc_client.c b/bin/varnishtest/vtc_client.c index fb7277d5c44..8fb3bdd3e6e 100644 --- a/bin/varnishtest/vtc_client.c +++ b/bin/varnishtest/vtc_client.c @@ -127,7 +127,9 @@ uds_open(void *priv, const struct sockaddr_un *uds) double *p; int s, i, tmo; struct pollfd fds[1]; - socklen_t sl = sizeof(*uds); + socklen_t sl; + + sl = VUS_socklen(uds); AN(priv); AN(uds); diff --git a/include/vus.h b/include/vus.h index 20d56fd15eb..c818f3939ae 100644 --- a/include/vus.h +++ b/include/vus.h @@ -36,6 +36,7 @@ int VUS_resolver(const char *path, vus_resolved_f *func, void *priv, const char **err); int VUS_bind(const struct sockaddr_un *uds, const char **errp); int VUS_connect(const char *path, int msec); +unsigned int VUS_socklen(const struct sockaddr_un *uds); static inline int VUS_is(const char *path) diff --git a/lib/libvarnish/vus.c b/lib/libvarnish/vus.c index c9920ef281e..a110a1ec127 100644 --- a/lib/libvarnish/vus.c +++ b/lib/libvarnish/vus.c @@ -86,6 +86,8 @@ VUS_resolver(const char *path, vus_resolved_f *func, void *priv, if (ret) return (ret); + assert(uds.sun_path[1] != '\0'); + if (func != NULL) ret = func(priv, &uds); return (ret); @@ -95,7 +97,9 @@ int VUS_bind(const struct sockaddr_un *uds, const char **errp) { int sd, e; - socklen_t sl = sizeof(*uds); + socklen_t sl; + + sl = VUS_socklen(uds); if (errp != NULL) *errp = NULL; @@ -133,13 +137,18 @@ VUS_connect(const char *path, int msec) int s, i; struct pollfd fds[1]; struct sockaddr_un uds; - socklen_t sl = (socklen_t) sizeof(uds); + socklen_t sl; if (path == NULL) return (-1); i = sun_init(&uds, path, NULL); if (i) return (i); + + assert(uds.sun_path[1] != '\0'); + + sl = VUS_socklen(&uds); + AN(sl); s = socket(PF_UNIX, SOCK_STREAM, 0); @@ -182,3 +191,19 @@ VUS_connect(const char *path, int msec) return (VTCP_connected(s)); } + +socklen_t +VUS_socklen(const struct sockaddr_un *uds) +{ + socklen_t sl; + char *p; + if (*uds->sun_path) + sl = sizeof(*uds); + else { + p = strchr(uds->sun_path + 1, '\0'); + assert(p != NULL); + sl = p - (const char*)uds; + } + assert(sl <= sizeof(*uds)); + return sl; +} From 5931b55e44063b4f915711fc8eea03ae4f70fe7c Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Mon, 20 Mar 2023 11:58:51 +0100 Subject: [PATCH 2/9] Constify (from flexelint) --- lib/libvarnish/vus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libvarnish/vus.c b/lib/libvarnish/vus.c index a110a1ec127..01187442f61 100644 --- a/lib/libvarnish/vus.c +++ b/lib/libvarnish/vus.c @@ -196,7 +196,7 @@ socklen_t VUS_socklen(const struct sockaddr_un *uds) { socklen_t sl; - char *p; + const char *p; if (*uds->sun_path) sl = sizeof(*uds); else { From eeeb8fad01926b188d8f9871f6d3b26107752f9b Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Mon, 3 Apr 2023 10:26:51 +0200 Subject: [PATCH 3/9] h2: Relax "no //" URLs requirement This requirement was dropped in the updated rfc 9113. Fixes: #3911 --- bin/varnishd/http2/cache_http2_hpack.c | 7 +++---- bin/varnishtest/tests/a02027.vtc | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c index 36570a751a2..8384856ddd6 100644 --- a/bin/varnishd/http2/cache_http2_hpack.c +++ b/bin/varnishd/http2/cache_http2_hpack.c @@ -135,10 +135,9 @@ h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len) n = HTTP_HDR_URL; disallow_empty = 1; - // rfc7540,l,3060,3071 - if (((len > 0 && *b != '/') || - (len > 1 && *(b+1) == '/')) && - (strncmp(b, "*", len) != 0)) { + // rfc9113,l,2693,2705 + if (len > 0 && *b != '/' && + strncmp(b, "*", len) != 0) { VSLb(hp->vsl, SLT_BogoHeader, "Illegal :path pseudo-header %.*s", (int)len, b); diff --git a/bin/varnishtest/tests/a02027.vtc b/bin/varnishtest/tests/a02027.vtc index ff34b00716c..e9dcf6619b7 100644 --- a/bin/varnishtest/tests/a02027.vtc +++ b/bin/varnishtest/tests/a02027.vtc @@ -22,8 +22,8 @@ client c1 { client c1 { stream 1 { txreq -noadd -hdr ":authority" "foo.com" -hdr ":path" "//foo" -hdr ":scheme" "http" -hdr ":method" "GET" - rxrst - expect rst.err == PROTOCOL_ERROR + rxresp + expect resp.status == 200 } -run } -run From 80f86806c089706c0923dbdaed13ae6004d7ea52 Mon Sep 17 00:00:00 2001 From: Lachie Date: Wed, 5 Apr 2023 12:10:29 +1000 Subject: [PATCH 4/9] bereq: Also unset a cached req body --- bin/varnishd/cache/cache_vrt_var.c | 6 ++++++ bin/varnishtest/tests/v00068.vtc | 33 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 bin/varnishtest/tests/v00068.vtc diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c index fe4f8a81e20..5069cadd650 100644 --- a/bin/varnishd/cache/cache_vrt_var.c +++ b/bin/varnishd/cache/cache_vrt_var.c @@ -35,6 +35,7 @@ #include #include "cache_varnishd.h" +#include "cache_objhead.h" #include "cache_transport.h" #include "common/heritage.h" @@ -595,6 +596,11 @@ VRT_u_bereq_body(VRT_CTX) { CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC); + if (ctx->bo->bereq_body != NULL) { + HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body, 0); + http_Unset(ctx->bo->bereq, H_Content_Length); + } + if (ctx->bo->req != NULL) { CHECK_OBJ(ctx->bo->req, REQ_MAGIC); ctx->bo->req = NULL; diff --git a/bin/varnishtest/tests/v00068.vtc b/bin/varnishtest/tests/v00068.vtc new file mode 100644 index 00000000000..4b54a5ba0f6 --- /dev/null +++ b/bin/varnishtest/tests/v00068.vtc @@ -0,0 +1,33 @@ +varnishtest "unset bereq.body with cached req body" + +server s1 { + rxreq + expect req.method == "GET" + expect req.http.Content-Length == + txresp + + rxreq + expect req.method == "GET" + txresp +} -start + +varnish v1 -vcl+backend { + import std; + + sub vcl_recv { + std.cache_req_body(2KB); + } + sub vcl_backend_fetch { + unset bereq.body; + } +} -start + +client c1 { + txreq -body "fine" + rxresp + expect resp.status == 200 + + txreq + rxresp + expect resp.status == 200 +} -run From f3fabdff3d84f731da0fc08ec7e9ab56ed051baa Mon Sep 17 00:00:00 2001 From: Poul-Henning Kamp Date: Fri, 12 May 2023 21:59:06 +0000 Subject: [PATCH 5/9] Fix a bug in our vgz-extension which is only triggered by vtest and only after 7565cdba7d10 --- lib/libvgz/deflate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/libvgz/deflate.c b/lib/libvgz/deflate.c index ed9ac5190e3..75277afd498 100644 --- a/lib/libvgz/deflate.c +++ b/lib/libvgz/deflate.c @@ -1810,6 +1810,9 @@ local block_state deflate_stored(s, flush) s->strm->total_out += len; } } while (last == 0); + if (last) + s->strm->stop_bit = + (s->strm->total_out + s->pending) * 8 + s->bi_valid; /* Update the sliding window with the last s->w_size bytes of the copied * data, or append all of the copied data to the existing window if less From 57b4152c8526d0a73230223dc77c056a6187b659 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Wed, 10 May 2023 16:58:19 +0200 Subject: [PATCH 6/9] vsl: Expose vsl_tag_is_masked --- bin/varnishd/cache/cache.h | 1 + bin/varnishd/cache/cache_shmlog.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index 55a86960a3f..2416196b27e 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -783,6 +783,7 @@ void VSLbs(struct vsl_log *, enum VSL_tag_e tag, const struct strands *s); void VSLb_ts(struct vsl_log *, const char *event, vtim_real first, vtim_real *pprev, vtim_real now); void VSLb_bin(struct vsl_log *, enum VSL_tag_e, ssize_t, const void*); +int VSL_tag_is_masked(enum VSL_tag_e tag); static inline void VSLb_ts_req(struct req *req, const char *event, vtim_real now) diff --git a/bin/varnishd/cache/cache_shmlog.c b/bin/varnishd/cache/cache_shmlog.c index b3b2e213df4..67150f69a48 100644 --- a/bin/varnishd/cache/cache_shmlog.c +++ b/bin/varnishd/cache/cache_shmlog.c @@ -130,6 +130,12 @@ vsl_tag_is_masked(enum VSL_tag_e tag) return (*bm & b); } +int +VSL_tag_is_masked(enum VSL_tag_e tag) +{ + return (vsl_tag_is_masked(tag)); +} + /*-------------------------------------------------------------------- * Lay down a header fields, and return pointer to the next record */ From 1295333832c00d17e46b40da2d0ca853ff1c6914 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Mon, 22 May 2023 10:57:09 +0200 Subject: [PATCH 7/9] h2_vsl_frame: Exit early if masked The VSB bits in here come with a very significant performance penalty for H2 request body processing. --- bin/varnishd/http2/cache_http2_proto.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index c1ced04b680..5b66795b04f 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -243,6 +243,10 @@ h2_vsl_frame(const struct h2_sess *h2, const void *ptr, size_t len) const char *p; unsigned u; + if (VSL_tag_is_masked(SLT_H2RxHdr) && + VSL_tag_is_masked(SLT_H2RxBody)) + return; + AN(ptr); assert(len >= 9); b = ptr; From 5636ad0b6b97dbb0499fb2609303d4dbc62ace75 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Wed, 12 Jul 2023 15:36:51 +0200 Subject: [PATCH 8/9] ESI: Use response status modulo 1000 Ref: 582ded6a2d6ae1a4467b1eb500f2725b42888016 Fixes #3958 --- bin/varnishd/cache/cache_esi_deliver.c | 7 ++++--- bin/varnishtest/tests/e00003.vtc | 29 ++++++++++++++++++-------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c index 56f4b82d271..100bbc3174d 100644 --- a/bin/varnishd/cache/cache_esi_deliver.c +++ b/bin/varnishd/cache/cache_esi_deliver.c @@ -856,6 +856,7 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody) { int i = 0; const char *p; + uint16_t status; struct ecx *ecx; struct ved_foo foo[1]; struct vrt_ctx ctx[1]; @@ -869,9 +870,9 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody) if (wantbody == 0) return; - if (!ecx->incl_cont && - req->resp->status != 200 && - req->resp->status != 204) { + status = req->resp->status % 1000; + + if (!ecx->incl_cont && status != 200 && status != 204) { req->top->topreq->vdc->retval = -1; req->top->topreq->doclose = req->doclose; return; diff --git a/bin/varnishtest/tests/e00003.vtc b/bin/varnishtest/tests/e00003.vtc index c03d9f48700..dad0d40abb9 100644 --- a/bin/varnishtest/tests/e00003.vtc +++ b/bin/varnishtest/tests/e00003.vtc @@ -8,8 +8,7 @@ server s1 { txresp -body { Before include - - After include + After include } rxreq @@ -21,9 +20,17 @@ server s1 { } -start varnish v1 -vcl+backend { + sub vcl_synth { + set resp.body = """ + """; + return (deliver); + } sub vcl_recv { if (req.esi_level > 0) { set req.url = req.url + req.esi_level; + if (req.url ~ "^/synth") { + return (synth(3200)); + } } else { set req.http.esi0 = "foo"; } @@ -64,7 +71,7 @@ logexpect l4 -v v1 -g request { } -start logexpect l5 -v v1 -g request { - expect * 1005 Begin "^req .* rxreq" + expect * 1006 Begin "^req .* rxreq" # Header bytes is 5 larger than in l1 due to two item X-Varnish hdr expect * = ReqAcct "^29 0 29 175 75 250$" expect 0 = End @@ -87,7 +94,7 @@ client c1 { } client c1 -run -varnish v1 -expect esi_req == 2 +varnish v1 -expect esi_req == 4 varnish v1 -expect esi_errors == 0 varnish v1 -expect MAIN.s_resp_bodybytes == 150 @@ -104,7 +111,7 @@ shell { cat >expected.txt <<-EOF 1001 c rxreq - 1005 c rxreq + 1006 c rxreq EOF diff -u expected.txt ncsa.txt } @@ -129,8 +136,10 @@ shell { cat >expected.txt <<-EOF 1001 c rxreq 1003 c esi - 1005 c rxreq - 1006 c esi + 1005 c esi + 1006 c rxreq + 1007 c esi + 1008 c esi EOF diff -u expected.txt ncsa.txt } @@ -145,8 +154,10 @@ shell { 1002 b fetch 1003 c esi 1004 b fetch - 1005 c rxreq - 1006 c esi + 1005 c esi + 1006 c rxreq + 1007 c esi + 1008 c esi EOF diff -u expected.txt ncsa.txt } From ad313fedc529c6f825230b04381c95903bc79bd9 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Tue, 11 Jul 2023 23:37:05 +0200 Subject: [PATCH 9/9] Fix a race between VBP_Remove() and vbp_thread() Suppose the following happens: vbp_task() finishes with vt->running = 0 and a heap insert. The vbp_cond is signaled under the lock, but now instead of vbp_thread() waking up first, VBP_Remove() gets the lock and reaches assert(vt->heap_idx == VBH_NOIDX) before the racing vbp_thread() deleted the heap. This is unlikely to happen with static backends, because for those, the probe is stopped via the vcl temperature before they get removed. Fixes https://github.com/nigoroll/libvmod-dynamic/issues/100 --- bin/varnishd/cache/cache_backend_probe.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/varnishd/cache/cache_backend_probe.c b/bin/varnishd/cache/cache_backend_probe.c index e4589dd3740..800bb6b67a5 100644 --- a/bin/varnishd/cache/cache_backend_probe.c +++ b/bin/varnishd/cache/cache_backend_probe.c @@ -710,8 +710,12 @@ VBP_Remove(struct backend *be) be->probe = NULL; vt->backend = NULL; if (vt->running) { + // task scheduled, it calls vbp_delete() vt->running = -1; vt = NULL; + } else if (vt->heap_idx != VBH_NOIDX) { + // task done, not yet rescheduled + VBH_delete(vbp_heap, vt->heap_idx); } Lck_Unlock(&vbp_mtx); if (vt != NULL) {