Skip to content

Ensure tcp-request doesn't reuse uncloned msg objects#5612

Merged
knolleary merged 2 commits intonode-red:masterfrom
hardillb:tcp-request-clone
Apr 10, 2026
Merged

Ensure tcp-request doesn't reuse uncloned msg objects#5612
knolleary merged 2 commits intonode-red:masterfrom
hardillb:tcp-request-clone

Conversation

@hardillb
Copy link
Copy Markdown
Member

@hardillb hardillb commented Apr 5, 2026

fixes #5609

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

Clones the input message before using it for the response, so if more than one response message is received then it doesn't override the first message.

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run npm run test to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

fixes node-red#5609

Clones the input message before using it for the response, so
if more than one response message is received then it doesn't
override the first message.
@dceejay
Copy link
Copy Markdown
Member

dceejay commented Apr 6, 2026

Fair enough to me - do we then also still need to clone in the new data on line 693 ?

@hardillb
Copy link
Copy Markdown
Member Author

hardillb commented Apr 6, 2026

do we then also still need to clone in the new data on line 693

Yes we still need that or we'd just be updating the ref for every chunk.

@hardillb
Copy link
Copy Markdown
Member Author

hardillb commented Apr 6, 2026

But we probably need clones on line 726 & 752 and 775. I will update the PR

EDIT line numbers are wrong as I was looking at the wrong branch

@dceejay
Copy link
Copy Markdown
Member

dceejay commented Apr 6, 2026

Final dumb question… would it be better to clone it on the way in (line 610) rather than the way out ?

@hardillb
Copy link
Copy Markdown
Member Author

hardillb commented Apr 6, 2026

No, because the whole problem is multiple response packets triggering outbound messages sharing the same msg instance, we need to clone on each output (before packets payload is added)

@dceejay
Copy link
Copy Markdown
Member

dceejay commented Apr 7, 2026

OK _ I'm happy for this to be merged.

@knolleary knolleary merged commit 7231e3d into node-red:master Apr 10, 2026
5 checks passed
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.

Strange behavior of TCP node

3 participants