Skip to content

Conversation

@yueshangzuo
Copy link

Commit Message:

golang: add nil guard in stream complete and destroy callbacks

Add nil check in envoyGoFilterOnHttpStreamComplete and
envoyGoFilterOnHttpDestroy to prevent panic when a Go filter
terminates the stream early (e.g., via SendLocalReply).

Without this, subsequent Go filters in the same chain may receive
callbacks on a cleared request context, causing nil pointer dereference.

Additional Description:

If a Go HTTP filter terminates the stream early (e.g., by calling SendLocalReply), Envoy may still call OnStreamComplete and OnDestroy on later Go filters in the chain.

At that point, the request context has already been cleared, so getRequest() returns nil. Without a nil check, the callback panics when trying to use the request.

This change adds a nil guard to both callbacks, so they safely return early if the request is gone.

Risk Level: Low

Testing: N/A
Docs Changes: N/A
Release Notes: Fixed a potential crash in Go HTTP filters when an early-terminating filter causes subsequent Go filters to be invoked on a cleared request context.
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Add nil guard in envoyGoFilterOnHttpStreamComplete and
envoyGoFilterOnHttpDestroy to prevent panic when Go filter
terminates stream early (e.g., via SendLocalReply).

Without this, subsequent callbacks on cleared request context
cause nil pointer dereference.

Signed-off-by: Yueshang zuo <[email protected]>
@repokitteh-read-only
Copy link

Hi @yueshangzuo, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #42965 was opened by yueshangzuo.

see: more, trace.

Copy link
Member

@doujiang24 doujiang24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for your fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants