reverse tunnel: fix empty provider-backed formatters in handshake headers#46168
Merged
yanavlasov merged 1 commit intoJul 17, 2026
Merged
Conversation
…ders Handshake ``additional_headers`` values that use a ThreadLocal-backed substitution formatter (e.g. ``%FILE_CONTENT%``, or secret/SDS-backed formatters) resolved to an empty string on the worker thread that assembles the reverse-tunnel handshake request. The handshake formatters were built in the bootstrap extension constructor, which runs before the worker threads register with the ThreadLocal system. ThreadLocal::Slot::set() only propagates to threads that are already registered and is not replayed for threads that register later, so the formatter providers' thread-local slots were never populated on the workers, and format() returned an empty value there. Build the handshake formatters in onServerInitialized() instead, which runs after the workers register, so the providers' set() reaches every worker thread. This mirrors the pattern the extension already uses for its own thread-local slot in onWorkerThreadInitialized(). Adds a regression test that evaluates the handshake header value on a real worker thread; it fails without the fix and passes with it. Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
kanurag94
requested review from
agrawroh,
botengyao and
yanavlasov
as code owners
July 16, 2026 10:31
Member
Author
|
/retest |
yanavlasov
approved these changes
Jul 17, 2026
This was referenced Jul 23, 2026
kanurag94
added a commit
to kanurag94/envoy
that referenced
this pull request
Jul 25, 2026
The reverse-connection listen socket snapshots the handshake formatters when it's created, and that can happen before onServerInitialized() has built them. The snapshot ends up null and gets reused for every re-dial, so handshake additional_headers that use a formatter (%FILE_CONTENT%, SDS-backed secrets, etc.) went out as the raw template string instead of the resolved value. Read the formatters straight from the extension when we assemble the handshake instead of trusting that snapshot, falling back to the snapshot only when the extension hasn't built any. This is a follow-up to envoyproxy#46168, which fixed the same symptom in the extension constructor but missed the socket() -> io_handle -> wrapper snapshot path. Adds a regression test that goes through the real socket() path so it can't regress. Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Commit Message: reverse tunnel: fix empty provider-backed formatters in handshake headers
Additional Description:
Handshake
additional_headersvalues that use a ThreadLocal-backed substitution formatter(e.g.
%FILE_CONTENT%, or secret/SDS-backed formatters) resolved to an empty string on the workerthread that assembles the reverse-tunnel handshake request.
The handshake formatters were built in the bootstrap extension constructor, which runs before the
worker threads register with the ThreadLocal system.
ThreadLocal::Slot::set()only propagates tothreads that are already registered and is not replayed for threads that register later, so the
formatter providers' thread-local slots were never populated on the workers, and
format()returned an empty value there.
Build the handshake formatters in
onServerInitialized()instead, which runs after the workersregister, so the providers'
set()reaches every worker thread. This mirrors the pattern theextension already uses for its own thread-local slot in
onWorkerThreadInitialized().Risk Level: Low
Testing: Unit test added that evaluates the handshake header on a real worker thread (fails without
the change, passes with it); existing reverse_tunnel unit tests updated.
Docs Changes: N/A
Release Notes: Changelog entry added.
Platform Specific Features: N/A