Skip to content

Conversation

addaleax
Copy link
Member

Set the req.buffer property, which serves as a way of keeping
a Buffer alive that is being written to a stream, on the C++
side instead of the JS side.

This closes a hole where buffers that were temporarily created
in order to write strings with uncommon encodings (e.g. hex)
were passed to the native side without being set as req.buffer.

Fixes: #8251
PR-URL: #8252

(@thealphanerd my instinct has become to automatically assign v4.x backports to you, in part because that makes it very easy to discern them visually in PR lists… if you think that’s cool I’ll keep doing it?)

Set the `req.buffer` property, which serves as a way of keeping
a `Buffer` alive that is being written to a stream, on the C++
side instead of the JS side.

This closes a hole where buffers that were temporarily created
in order to write strings with uncommon encodings (e.g. `hex`)
were passed to the native side without being set as `req.buffer`.

Fixes: nodejs#8251
PR-URL: nodejs#8252
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. net Issues and PRs related to the net subsystem. v4.x lib / src Issues and PRs related to general changes in the lib or src directory. labels Aug 27, 2016
@addaleax
Copy link
Member Author

@jasnell
Copy link
Member

jasnell commented Aug 29, 2016

Some red in CI unfortunately.

@addaleax
Copy link
Member Author

@addaleax
Copy link
Member Author

It’s all mostly been buildbot failures so far, nothing related, but let’s see if we can get a green CI: https://ci.nodejs.org/job/node-test-commit/4833/

@jasnell
Copy link
Member

jasnell commented Aug 30, 2016

Green! LGTM!

@MylesBorins
Copy link
Contributor

LGTM

@MylesBorins
Copy link
Contributor

landed in bceac33

@MylesBorins MylesBorins closed this Sep 7, 2016
@addaleax addaleax deleted the backport-8252 branch September 7, 2016 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. net Issues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants