lua filter: ensure active_request_ is verified before deletion Fixes #38017

[leocfig]

Commit Message:
Lua filter was removing ":status" and then setting a response body, which was causing envoy to terminate with a SIGSEGV. The issue occurred because "onEncodeComplete()" attempted to delete "activerequest" via "deferredDelete()", but "activerequest" had already been deleted earlier, leading to a segmentation fault. This fix ensures that "activerequest" is only deleted once, preventing double deletion. This is done by verifying that "activerequest" is not null before the existing check of "activerequest->remotecomplete".

Additional Description:
I compiled Envoy in release mode (using the -c flag) and also ran the new test in release mode to verify the fix.

Risk Level: Low

Testing:
I added a new integration test in "lua_integration_test.cc" file specifically for this bug. Without the fix, the test fails; with it applied, the test passes successfully.

Fixes #38017

[repokitteh-read-only]

Hi @leocfig, 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: #38863 was opened by leocfig.

see: more, trace.

[leocfig]

I believe the pipeline is failing because the tests are running in debug mode. The code eventually reaches a verification where ENVOY_BUG() forcefully aborts the program, when in debug mode. What my fix avoids is the program ending with a segmentation fault in release mode.

[KBaichoo]

Hey @leocfig ,

Thank you for working on this.

We do run CI (strangely under pre-check) with the release mode run of tests: https://github.com/envoyproxy/envoy/actions/runs/14021237617/job/39253502671

It seems like the test is still failing in release mode.

You can also try wrapping the test with a macro similar to

envoy/test/test_common/utility.h

Line 99 in 2533c82

#define EXPECT_ENVOY_BUG(statement, message) EXPECT_LOG_CONTAINS("error", message, statement)

to catch the error depending on DEBUG vs release build

/wait

[leocfig]

Thanks for the suggestion! I’ll first investigate why it’s still failing in release mode since it passed locally, then update the test to use EXPECT_ENVOY_BUG.

[leocfig]

I realized that my test was failing in the pipeline despite passing locally because my code was not up-to-date with a recent commit that modified the lua_integration_test.cc file. After reviewing the changes to this file, I updated my test accordingly.

Additionally, I added the #ifdef NDEBUG preprocessor directive to ensure the test only runs in release mode. This was necessary because in debug mode, the program aborts before reaching the segmentation fault, making the test irrelevant in that mode.

[leocfig]

I noticed that the //test/extensions/filters/http/dynamic_forward_proxy:legacy_proxy_filter_integration_test target is failing in the pipeline, even though it passes when I run it locally. However, I believe this issue is unrelated to my null verification.