Skip to content

decoder state leakage across multiple messages#44860

Open
jmestwa-coder wants to merge 1 commit into
envoyproxy:mainfrom
jmestwa-coder:sip-decoder-reset-state-between-messages
Open

decoder state leakage across multiple messages#44860
jmestwa-coder wants to merge 1 commit into
envoyproxy:mainfrom
jmestwa-coder:sip-decoder-reset-state-between-messages

Conversation

@jmestwa-coder
Copy link
Copy Markdown

Summary

Fix state leakage in SIP decoder where clen and full_msg_len were not reset between iterations in Decoder::reassemble().


Problem

When multiple SIP messages are present in a single buffer:

  • Values from the previous message are reused
  • Subsequent messages may not be processed

Fix

Move clen and full_msg_len inside the loop so they are reinitialized per message.


Test

Adds a regression test that:

  • Sends two SIP messages in one buffer
  • Verifies both are processed (newDecoderEventHandler called twice)

The test fails before the fix and passes after.

@repokitteh-read-only
Copy link
Copy Markdown

Hi @jmestwa-coder, 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: #44860 was opened by jmestwa-coder.

see: more, trace.

@zuercher
Copy link
Copy Markdown
Member

zuercher commented May 5, 2026

You'll need to fix the DCO before this will be accepted. See CONTRIBUTING.md for instructions.

@zuercher
Copy link
Copy Markdown
Member

zuercher commented May 5, 2026

/wait

Signed-off-by: jmestwa-coder <jmestwa@gmail.com>
@jmestwa-coder jmestwa-coder force-pushed the sip-decoder-reset-state-between-messages branch from fea3bd3 to 66b289f Compare May 5, 2026 17:13
@jmestwa-coder jmestwa-coder requested a deployment to external-contributors May 5, 2026 17:13 — with GitHub Actions Waiting
@jmestwa-coder
Copy link
Copy Markdown
Author

Added DCO sign-off and updated the commit. Please let me know if anything else is needed.

@kyessenov
Copy link
Copy Markdown
Contributor

/wait
@durd07 please review as the code owner

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants