-
Notifications
You must be signed in to change notification settings - Fork 437
fix: enable http3 panic #5671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: enable http3 panic #5671
Conversation
Signed-off-by: bitliu <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5671 +/- ##
==========================================
+ Coverage 65.20% 65.29% +0.08%
==========================================
Files 213 213
Lines 34108 34115 +7
==========================================
+ Hits 22241 22276 +35
+ Misses 10521 10500 -21
+ Partials 1346 1339 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
internal/xds/translator/listener.go
Outdated
if err := addXdsTLSInspectorFilter(xdsListener); err != nil { | ||
return err | ||
// Only add TLS inspector filter if the listener is not nil and is a TCP listener | ||
if xdsListener != nil && xdsListener.GetAddress().GetSocketAddress().GetProtocol() == corev3.SocketAddress_TCP { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also not add ServerNameMatch for UDP/HTTP3.
Is this related to #5005? |
Yes, |
Signed-off-by: bitliu <[email protected]>
@@ -511,14 +511,20 @@ func buildEarlyHeaderMutation(headers *ir.HeaderSettings) []*corev3.TypedExtensi | |||
} | |||
|
|||
func addServerNamesMatch(xdsListener *listenerv3.Listener, filterChain *listenerv3.FilterChain, hostnames []string) error { | |||
// Dont add a filter chain match if the hostname is a wildcard character. | |||
// Don't add a filter chain match if the hostname is a wildcard character or if the listener is UDP (HTTP3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be easier to read if the skip is moved to the top
if xdsListener == nil || xdsListener.GetAddress().GetSocketAddress().GetProtocol() != corev3.SocketAddress_UDP
return nil
Signed-off-by: Marc 'risson' Schmitt <[email protected]>
What type of PR is this?
What this PR does / why we need it:
These changes ensure that when HTTP/3 is enabled through a ClientTrafficPolicy, the TLS inspector filter is only added to TCP listeners, not to UDP (QUIC) listeners, which prevents the nil pointer dereference.
Which issue(s) this PR fixes:
Fixes #5660
Release Notes: Yes/No