-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tls: copy the Buffer object before using (backport to v4.x) #9016
Conversation
@@ -52,7 +52,7 @@ exports.convertNPNProtocols = function convertNPNProtocols(NPNProtocols, out) { | |||
|
|||
// If it's already a Buffer - store it | |||
if (NPNProtocols instanceof Buffer) { | |||
out.NPNProtocols = NPNProtocols; | |||
out.NPNProtocols = new Buffer(NPNProtocols); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not Buffer.from
, like in the original PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was being consistent with the rest of the file, change incoming
`convertNPNProtocols` and `convertALPNProtocols' uses the `protocols` buffer object as it is, and if it is modified outside of core, it might have an impact. This patch makes a copy of the buffer object, before using it. PR-URL: nodejs#8055 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
fe12577
to
2847490
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is a backport of #8055
There was quite a bit of difference between this and the original so I would like to get sign off from @thefourtheye or @jasnell that this is doing what is should be doing.