Promisifying Crypto API #1
Open
Conversation
This file contains hidden or 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
boneskull
reviewed
Jun 4, 2020
boneskull
reviewed
Jun 4, 2020
lib/crypto.js
Outdated
There was a problem hiding this comment.
In current master, the promises in fs are defined as:
let promises = null;then below in the get() method:
if (promises === null) {
// ..
}
boneskull
reviewed
Jun 4, 2020
promises.js
Outdated
boneskull
reviewed
Jun 4, 2020
test/parallel/test-crypto-random.js
Outdated
There was a problem hiding this comment.
seems good, but would expect at least a happy-path test. I assume that the promise implementations aren't generally tested as thoroughly as the callback implementations given they are usually just wrappers?
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due to the application already being signed. This commit updates the test to handle that condition. Refs: nodejs#33944 PR-URL: nodejs#34331 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
The messaging code uses `Object.defineProperty()`, which accesses `value` on `Object.prototype` by default, so some calls to the getter here would actually be expected. Instead, make the list of accessed properties more specific to the tested source map code to avoid flakiness. Refs: https://twitter.com/addaleax/status/1276289101671608320 Refs: nodejs#34057 PR-URL: nodejs#34446 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
This test has not been working correctly since at least a555be2. Since it tests internals, any replacement test might become invalid in a similar way. Refs: nodejs#34057 PR-URL: nodejs#34445 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Enable `NodeEventTarget` as a base class for `MessagePort`, by enabling special processing of events for Node.js listeners, and removing implicit constructors/private properties so that classes can be made subclasses of `NodeEventTarget` after they are created. PR-URL: nodejs#34057 Refs: https://twitter.com/addaleax/status/1276289101671608320 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Use `NodeEventTarget` to provide a mixed `EventEmitter`/`EventTarget` API interface. PR-URL: nodejs#34057 Refs: https://twitter.com/addaleax/status/1276289101671608320 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
For consistency with the rest of our docs and our style guide, use sentence-case rather than headline-case in the headers in quic.md. PR-URL: nodejs#34453 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Move benchmark CI to native suite since it requires building an addon. Refs: nodejs#34427 (comment) PR-URL: nodejs#34433 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34494 Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
The index ToC says "About these docs" but the document itself says "About this documentation" which I think is better. Use that. PR-URL: nodejs#34449 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Although most of the time openStream will be able to create the stream immediately, when a stream is opened before the handshake is complete we have to wait for the handshake to be complete before continuing. PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Removing no longer needed code PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
qlog files are diagnostic files that are being used to verify the quic implementation. Make sure they don't get checked in. PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
This one was a bit of a rabbit hole... but, with this set of changes, `QuicStream` should now work with autoDestroy, supports a promisified `close()`, and fixes a number of other internal bugs that were spotted trying to get it to work. PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34351 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34317 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Load self referential modules from the repl and using the preload flag `-r`. In both cases the base path used for resolution is the current `process.cwd()`. Also fixes an internal cycle bug in the REPL exports resolution. PR-URL: nodejs#32261 Fixes: nodejs#31595 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com>
This GitHub action takes quite a bit of time to build and is regularly flaky. Removing the OSX and Windows target from this action to avoid having red actions CI runs. Refs: nodejs#34123 PR-URL: nodejs#34440 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: nodejs#34471 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
PR-URL: nodejs#34471 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Remove checks in pummel/test-timers that are already checked in parallel/test-timers-clear-null-does-not-throw-error. PR-URL: nodejs#34473 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Refs: nodejs#34295 (comment) PR-URL: nodejs#34426 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <Michael_Dawson@ca.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Callout some practices explicitly, so that the process is followed in a similar manner PR-URL: nodejs#34455 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
PR-URL: nodejs#34782 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Harshitha K P <harshitha014@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: nodejs#34070 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
As best as I can tell, ERR_V8BREAKITERATOR is unused anywhere in our code base and dependencies. Move to legacy errors. PR-URL: nodejs#34792 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
In test-http-destroyed-socket-write2, the assert.strictEqual() in the default case of the switch statement will always fail because it checks for a value that is already accounted for in one of the switch cases. Convert it to assert.fail(). PR-URL: nodejs#34793 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
The text for the legacy API sends mixed signals. It's legacy, but still supported, so not deprecated, but not recommended. Let's begin to clarify this by removing "not recommended". If we want to not-recommend it, let's doc-deprecate it properly, or at least include an explanation as to why it's not recommended. PR-URL: nodejs#34697 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Documentation-only: Recommend people use the static methods on crypto.Certificate() and not the legacy API constructor. PR-URL: nodejs#34697 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Running outside of the main Node.js context prevents us from upgrading the WPT harness because new versions more aggressively check the identity of globals like error constructors. Instead of exposing globals used by the tests on vm sandboxes, use worker threads to run everything. PR-URL: nodejs#34796 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
There doesn't seem to be a reason for this test to have to stay in sequential. It appears to have been placed there out of caution. PR-URL: nodejs#34755 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#34800 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Harshitha K P <harshitha014@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ricky Zhou <0x19951125@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: nodejs#34829 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Refs: nodejs#34765 PR-URL: nodejs#34795 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#34786 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
In other api docs, it seems that deprecated classes and methods are listed along with others, and marked as deprecated. In tls docs, deprecations were listed at the bottom of the document. This commit reorders them to what seems to be the standard, and corrects some links in doc/api/deprecations.md PR-URL: nodejs#34687 Reviewed-By: Rich Trott <rtrott@gmail.com>
Signed-off-by: Richard Lau <riclau@uk.ibm.com> PR-URL: nodejs#34810 Refs: nodejs#34787 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Change format of logged messages when run on GitHub Actions (i.e. when the `GITHUB_ACTIONS` environment variable is true) so that broken links are highlighted inline in pull requests. Signed-off-by: Richard Lau <riclau@uk.ibm.com> PR-URL: nodejs#34810 Refs: nodejs#34787 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
The `module` core module is available for both CJS and ESM users, it deserves its own page. PR-URL: nodejs#34747 Refs: nodejs/modules#539 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Some stylistic changes to bring this more in line with what our tests currently look like, and add a note about general flakiness. PR-URL: nodejs#34685 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
This commit adds validation to the IP address returned by the net module's custom DNS lookup() function. PR-URL: nodejs#34813 Fixes: nodejs#34812 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#34584 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com>
PR-URL: nodejs#34768 Fixes: nodejs#34767 Fixes: nodejs#34447 Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Harshitha K P <harshitha014@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#34809 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Uses x64 node executable for running .js files in arm64 cross-compilation scenarios. MSI can now be created by running `vcbuild.bat release msi arm64` Refs: nodejs#25998 Refs: nodejs#32582 PR-URL: nodejs#34009 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com>
Fixes an issue on Windows, where Unicode in NODE_OPTIONS was not parsed correctly. Fixes: nodejs#34399 PR-URL: nodejs#34476 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Denys Otrishko <shishugi@gmail.com>
PR-URL: nodejs#34798 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Mary Marchini <oss@mmarchini.me>
Keep references sorted in ASCII order in module.md. PR-URL: nodejs#34848 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Remove/add periods as appropriate in bulleted lists in BUILDING.md. PR-URL: nodejs#34849 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
test-cluster-net-listen-relative-path fails if run from the root directory on POSIX because the socket filename isn't quite long enough. Increase it by 2 so that the path length always exceeds 100 characters. PR-URL: nodejs#34820 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
PR-URL: nodejs#34816 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
joesepi
force-pushed
the
promisify-crypto
branch
from
a7f2c0f to
1a034ff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes