-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: renegotiate should take care of its own state #25997
tls: renegotiate should take care of its own state #25997
Conversation
In the initial version of this test there were two zero-length writes to force tls state to cycle. The second is not necessary, at least not now, but the first was. The renegotiate() API should ensure that packet exchange takes place, not its users, so move the zero-length write into tls. See: nodejs#14239 See: nodejs@b1909d3a70f9
I don't offer any justifiation for this change, its streams black magic. I'm curious to see if it passes CI. |
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.
Thanks for the ping :)
Btw, in the long run, it might make sense to move TLSWrap to a HTTP2-style model, where we check at the end of methods whether we have data to send or not, and if so, schedule a write.
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.
Cautious LGTM
Landed in 666beb0. |
In the initial version of this test there were two zero-length writes to force tls state to cycle. The second is not necessary, at least not now, but the first was. The renegotiate() API should ensure that packet exchange takes place, not its users, so move the zero-length write into tls. See: #14239 See: b1909d3a70f9 PR-URL: #25997 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In the initial version of this test there were two zero-length writes to force tls state to cycle. The second is not necessary, at least not now, but the first was. The renegotiate() API should ensure that packet exchange takes place, not its users, so move the zero-length write into tls. See: #14239 See: b1909d3a70f9 PR-URL: #25997 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In the initial version of this test there were two zero-length writes to force tls state to cycle. The second is not necessary, at least not now, but the first was. The renegotiate() API should ensure that packet exchange takes place, not its users, so move the zero-length write into tls. See: nodejs#14239 See: nodejs@b1909d3a70f9 PR-URL: nodejs#25997 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In the initial version of this test there were two zero-length writes to force tls state to cycle. The second is not necessary, at least not now, but the first was. The renegotiate() API should ensure that packet exchange takes place, not its users, so move the zero-length write into tls. See: #14239 See: b1909d3a70f9 Backport-PR-URL: #27938 PR-URL: #25997 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In the initial version of this test there were two zero-length writes to
force tls state to cycle. The second is not necessary, at least not now,
but the first was. The renegotiate() API should ensure that packet
exchange takes place, not its users, so move the zero-length write into
tls.
See: #14239
See: b1909d3a70f9
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes