tls: use the most recently added matching SecureContext in default SNICallback. #34638
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addaleax
approved these changes
Aug 5, 2020
|
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reverse The reverse() method reverses an array in place. The first array element becomes the last, and the last array element becomes the first. Array.prototype.reverse() This method will change the original array, which produces unnecessary side effects and will cause a great performance loss. |
|
We should not use the Array.prototype.reverse() method. For the same servername, when the server.addContext and SNICallback method is used more than 2 times, an error will occur in half of the cases. |
|
Since we are solving the same problem, could you help me write a test for this pull request? |
|
Thank you @masx200 , good catch. Unfortunately, your PR contains numerous linting errors that you should be aware of, if you actually ran Regarding tests - below guide for writing tests should be helpful: |
bnoordhuis
approved these changes
Aug 6, 2020
|
It seems that there are two competing PRs for this? Which one should land? |
|
Well I reported the issue and stated I have a WIP fix. And the other one still has multiple unresolved ssues. |
|
Seems to have gotten a little stuck. What do we need to move further? |
|
@nodejs/crypto |
Refs: web-platform-tests/wpt#25987 PR-URL: nodejs#35636 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
It is already installed in the GitHub runners. PR-URL: nodejs#35638 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The other way is deprecated. PR-URL: nodejs#35638 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This adds support for DNS Certification Authority Authorization (RFC 8659) to Node.js. PR-URL: nodejs#35466 Fixes: nodejs#19239 Refs: nodejs#14713 Reviewed-By: Anna Henningsen <anna@addaleax.net>
0 is deprecated. Change to 1, which is experimental. PR-URL: nodejs#35672 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#35642 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Since the section refers to EventEmitter, instances in the example should be created of the same class EventEmitter. PR-URL: nodejs#33513 Reviewed-By: James M Snell <jasnell@gmail.com>
Currently we are returning the stringified error code while in other process methods we are throwin a UVException and only exclusion is in the CPUUsage. Converted it to follow the convention. PR-URL: nodejs#34762 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: nodejs#35086 Reviewed-By: Ruy Adorno <ruyadorno@github.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#35522 Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Use `common.mustCall` and `util.debuglog`. Remove unnecessary functions PR-URL: nodejs#32805 Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#35690 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#35653 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
PR-URL: nodejs#35657 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#35593 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This is in preparation for updating remark-preset-lint-node to 1.17.1. PR-URL: nodejs#35668 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Original commit message:
Fix alloc/dealloc size mismatch for v8::BackingStore
On newer compilers the {operator delete} with explicit {size_t}
argument would be instantiated for {v8::BackingStore} and used
in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
argument is wrong though, since the pointer actually points
to a {v8::internal::BackingStore} object.
The solution is to explicitly provide a {operator delete}, preventing
an implicitly generated {size_t} operator.
Bug:v8:11081
Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70916}
PR-URL: nodejs#35939
Fixes: nodejs#35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
A test was added for this in 6f34498 but because it was a test in the `internet` directory, it was not run on CI and it was not noticed that the test was failing. This fixes the error that was causing the test to fail. PR-URL: nodejs#35979 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#36017 Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#27413 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#32655 Fixes: nodejs#32559 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: nodejs#34543 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Harshitha K P <harshitha014@gmail.com> Reviewed-By: Pranshu Srivastava <rexagod@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com>
|
@mkrawczuk can you fix the linter issue please? |
aduh95
added
stalled
|
This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open. |
Refs: nodejs#34944 (comment) Co-authored-by: Anna Henningsen <anna@addaleax.net> PR-URL: nodejs#35000 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: nodejs#35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
In stream write encoding can be null. Fixes: nodejs#33715 PR-URL: nodejs#35372 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#35633 Fixes: nodejs#35600 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#35999 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe>
`EventTarget` is exposed on the global scope, there is no need to use `--expose-internals` flag in the tests. Refs: nodejs#35496 PR-URL: nodejs#36002 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs#35993 (comment) PR-URL: nodejs#35995 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#36024 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Make the linter recommend replacing `globalThis.Map` by `primordials.SafeMap`, and similar for `Set`, `WeakSet`, and `WeakMap`. PR-URL: nodejs#36026 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
PR-URL: nodejs#35679 Fixes: nodejs#35629 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#36012 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Don't write to a stream which already has a full buffer. Fixes: nodejs#35341 PR-URL: nodejs#35348 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Search the default installation path for NASM installed by a user without administrator privileges when not found on the Path or in the default system-wide installation path. PR-URL: nodejs#36014 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #34110
make -j4 test(UNIX), orvcbuild test(Windows) passes