Skip to content
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_legacy: do not read on OpenSSL's stack #4624

Closed
wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Jan 11, 2016

Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

R=@trevnorris

Let's land this change anyway, so it will be easier to work on our AsyncWrap stuff.

Not really going to introduce a test here, because this thing will fail once #4509 or any similar PR will land.

Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.
@trevnorris
Copy link
Contributor

Awesome. LGTM.

@trevnorris trevnorris added the tls Issues and PRs related to the tls subsystem. label Jan 11, 2016
@indutny
Copy link
Member Author

indutny commented Jan 11, 2016

@jasnell
Copy link
Member

jasnell commented Jan 11, 2016

LGTM

@indutny
Copy link
Member Author

indutny commented Jan 11, 2016

cc @nodejs/crypto just in case

@indutny
Copy link
Member Author

indutny commented Jan 11, 2016

Landed in 5a2445b, thank you everyone!

@indutny indutny closed this Jan 11, 2016
indutny added a commit that referenced this pull request Jan 11, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: #4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@indutny indutny deleted the land/gist-70b5 branch January 11, 2016 20:11
@MylesBorins
Copy link
Contributor

@indutny should this be rolled into v5.4.1 or should we wait for @nodejs/crypto to take a look?

@indutny
Copy link
Member Author

indutny commented Jan 12, 2016

It should be safe to roll it into v5.4.1

MylesBorins pushed a commit that referenced this pull request Jan 12, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: #4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 12, 2016
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: #4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: #4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: nodejs#4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: nodejs#4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Do not attempt to read data from the socket whilst on OpenSSL's stack,
weird things may happen, and this is most likely going to result in some
kind of error.

PR-URL: nodejs#4624
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants