test: remove potential race condition in https renegotiation test - #25601
test: remove potential race condition in https renegotiation test#25601Trott wants to merge 1 commit into
Conversation
Sorry, something went wrong.
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is set by the server and checked by the client, which is a separate process launched with child_process.spawn(). The boolean is not actually required by the client and might even be causing a race condition on some operating systems. Remove it.
|
@Trott build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2312/pipeline |
Sorry, something went wrong.
Sorry, something went wrong.
cjihrig
left a comment
There was a problem hiding this comment.
LGTM if the CI passes.
Sorry, something went wrong.
| @@ -77,7 +74,6 @@ function test(next) { | |||
| let renegs = 0; | |||
|
|
|||
| child.stderr.on('data', function(data) { | |||
There was a problem hiding this comment.
Using .on('data') without any sort of buffering might also be very brittle and something that we should fix…?
Sorry, something went wrong.
|
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/20275/ Will have to update the commit message because it's a bit wonky/wrong, but I'm looking forward to landing this one because stress tests show that it fixes the flaky pummel test! 🎉 Once it lands, we'll have to do the same for test-tls-ci-reneg-attack.js. Will open a PR for that now-ish.... |
Sorry, something went wrong.
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is causing a race condition on some operating systems. It is unnecessary. Remove it. PR-URL: nodejs#25601 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is causing a race condition on some operating systems. It is unnecessary. Remove it. PR-URL: #25601 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is causing a race condition on some operating systems. It is unnecessary. Remove it. PR-URL: #25601 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is causing a race condition on some operating systems. It is unnecessary. Remove it. PR-URL: #25601 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is causing a race condition on some operating systems. It is unnecessary. Remove it. PR-URL: #25601 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is
set by the server and checked by the client, which is a separate process
launched with child_process.spawn(). The boolean is not actually
required by the client and might even be causing a race condition on
some operating systems. Remove it.
Checklist