backend: http1 chunked encoding trailers support & other improvements #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://tools.ietf.org/html/rfc7230#section-4.1.1 A recipient MUST ignore unrecognized chunk extensions.
Ref: https://tools.ietf.org/html/rfc7230#section-4.1.2 There is no explicit "MUST ignore" here, but it is clear that trailer fields must at least be read.
== Usage / VCL ==
Trailer processing is enabled with ``set bereq.http.TE = "trailers";``
in ``vcl_backend_fetch{}``. Only then may the backend send a chunked
response with trailers. Any ``TE`` client header remains filtered, so
trailer processing is neither implicitly enabled nor would trailers
ever be sent downstream (except in pipe mode).
The above only allows the backend to send trailers, whether or not it
actually makes use of that capability (or if it even chooses chunked
encoding) is up to the backend.
For processing of the backend response, we enable trailer handling
only if a ``Trailers:`` backend response header is present - either
from the backend or set in ``vcl_backend_response{}``. ``Trailers:``
must contain header names to be accepted in the chunked trailer. In
addition, ``set beresp.http.Trailers = "*";`` can be used in vcl to
instruct varnish to accept any header in the chunked
trailer. ``Trailers: *`` is never accepted from a backend.
Any accepted trailer headers are copied into the object without
additional VCL control. In other words, any headers allowed by the
``Trailers`` header are accepted as-are. Additional filtering is
possible on the client side.
Trailer processing implicitly disables streaming. The backend response
is read into cache and trailer headers are delivered as normal
response headers.
== Implementation notes ==
The object API requires that the size of the attribute space is known
at object creation time, so we need to guess the amount of space to
reserve for trailers. For this initial implementation, the reserve is
hardcoded as 1KB and we'll probably want some VCL control later.
For trailer processing, the setting OA_HEADERS now needs happen after
reading the body, obviously. It is unclear if The Proprietary
Stevedore (tm) supports this.
To avoid code duplication for header dissection, http1_dissect_hdrs()
is now generalised and declared in cache.h as HTTP1_DissectHdrs()
For logging new/deleted headers from vbf, http_VSLH() and
http_VSLH_del() are now declared in cache_http.h
Note this does not consume additional space on 32bit and more as the enum gets aligned to sizeof(int). Checked with gdb on a 32bit binary: before:: (gdb) print (int)&((struct http*)0)->nhd $1 = 16 (gdb) print (int)&((struct http*)0)->logtag $2 = 20 after:: (gdb) print (int)&((struct http*)0)->nhd $1 = 16 (gdb) print (int)&((struct http*)0)->logtag $2 = 20 (gdb) print (int)&((struct http*)0)->thd $3 = 18
This was referenced Nov 23, 2017
nigoroll
added a commit
that referenced
this pull request
Nov 7, 2022
The panic code would panic again because a backend object which it no longer owned was still referenced in thread local storage (don't we all love abiguous acronyms?). Example: #5 0x0000563d8c20d595 in VAS_Fail (func=0x563d8c23fd13 "vsl_sanity", file=0x563d8c23f9f5 "cache/cache_shmlog.c", line=110, cond=0x563d8c23fd1e "(vsl->wlp) != 0", kind=VAS_ASSERT) at vas.c:67 #6 0x0000563d8c1603e9 in vsl_sanity (vsl=0x7fabd08001f0) at cache/cache_shmlog.c:110 ... #7 0x0000563d8c160298 in VSL_Flush (vsl=0x7fabd08001f0, overflow=0) at cache/cache_shmlog.c:314 #8 0x0000563d8c14dd44 in pan_ic (func=0x563d8c23c596 "child_signal_handler", file=0x563d8c23c1b4 "cache/cache_main.c", line=323, cond=0x7fabdf24a590 "Signal 6 (Aborted) received at 0x3e800015528 si_code -6", kind=VAS_WRONG) at cache/cache_panic.c:814 ... varnishcache#19 0x0000563d8c14bc07 in ObjBocDone (wrk=0x7fabdc9e54b8, oc=0x7fabd0024180, boc=0x7fabdc9e4778) at cache/cache_obj.c:368 varnishcache#20 0x0000563d8c13f25b in HSH_DerefBoc (wrk=0x7fabdc9e54b8, oc=0x7fabd0024180) at cache/cache_hash.c:1014 varnishcache#21 0x0000563d8c13158f in VBF_Fetch (wrk=0x7fabdc9e54b8, req=0x7fabd0008b20, oc=0x7fabd0024180, oldoc=0x0, mode=VBF_PASS) at cache/cache_fetch.c:1204 varnishcache#22 0x0000563d8c158f7d in cnt_pass (wrk=0x7fabdc9e54b8, req=0x7fabd0008b20) at cache/cache_req_fsm.c:742 varnishcache#23 0x0000563d8c156f53 in CNT_Request (req=0x7fabd0008b20) at cache/cache_req_fsm.c:1182 varnishcache#24 0x0000563d8c198f9a in HTTP1_Session (wrk=0x7fabdc9e54b8, req=0x7fabd0008b20) at http1/cache_http1_fsm.c:390 varnishcache#25 0x0000563d8c1984e0 in http1_req (wrk=0x7fabdc9e54b8, arg=0x7fabd0008b20) at http1/cache_http1_fsm.c:88 varnishcache#26 0x0000563d8c189592 in Pool_Work_Thread (pp=0x7fabdc400140, wrk=0x7fabdc9e54b8) at cache/cache_wrk.c:487 varnishcache#27 0x0000563d8c188ca7 in WRK_Thread (qp=0x7fabdc400140, stacksize=81920, thread_workspace=2048) at cache/cache_wrk.c:153
nigoroll
added a commit
that referenced
this pull request
Jan 31, 2024
in VRT_AddDirector, we create the new vcldir with an initial reference, which we need to drop if we can not add it. Compare: VRT_AddDirector() ... vdir->refcnt++; vcldir_free() ... AZ(vdir->refcnt); Noticed when testing other experimental changes while working on nigoroll/libvmod-dynamic#110 #5 0x000055820c8cb845 in VAS_Fail (func=0x55820c904559 "vcldir_free", file=0x55820c903a47 "cache/cache_vrt_vcl.c", line=150, cond=0x55820c90459a "(vdir->refcnt) == 0", kind=VAS_ASSERT) at vas.c:67 #6 0x000055820c83a442 in vcldir_free (vdir=0x7f662aa53140) at cache/cache_vrt_vcl.c:150 #7 0x000055820c839fe1 in VRT_AddDirector (ctx=0x7f662befe250, m=0x55820c965260 <vbe_methods_noprobe>, priv=0x7f662aa20780, fmt=0x55820c900f7f "%s") at cache/cache_vrt_vcl.c:219 #8 0x000055820c7c7c4d in VRT_new_backend_clustered (ctx=0x7f662befe250, vc=0x0, vrt=0x7f662befdd10, via=0x0) at cache/cache_backend.c:737 varnishcache#9 0x000055820c7c8632 in VRT_new_backend (ctx=0x7f662befe250, vrt=0x7f662befdd10, via=0x0) at cache/cache_backend.c:755
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
based upon varnishcache#2501
Usage / VCL
Trailer processing is enabled with
set bereq.http.TE = "trailers";invcl_backend_fetch{}. Only then may the backend send a chunked response with trailers. AnyTEclient header remains filtered, so trailer processing is neither implicitly enabled nor would trailers ever be sent downstream (except in pipe mode).In addition, trailers are only accepted if
std.fetch_body()is used. Otherwise, a backend fetch fails if the backend tries to actually set trailers.For processing of the backend response, we enable trailer handling only if a
Trailers:backend response header is present - either from the backend or set invcl_backend_response{}.Trailers:must contain header names to be accepted in the chunked trailer. In addition,set beresp.http.Trailers = "*";can be used in vcl to instruct varnish to accept any header in the chunked trailer.Trailers: *is never accepted from a backend.Trailers are visible to VCL as normal headers after the
std.fetch_body()call.Implementation notes
To avoid code duplication for header dissection,
http1_dissect_hdrs()is now generalised and declared in cache.h asHTTP1_DissectHdrs()For logging new/deleted headers from vbf,
http_VSLH()andhttp_VSLH_del()are now declared in cache_http.h