X Tutup
The Wayback Machine - https://web.archive.org/web/20220817093728/https://github.com/nodejs/node/pull/42559
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

Jump to bottom

crypto: do not add undefined hash in webcrypto normalizeAlgorithm #42559

Merged
merged 2 commits into from Apr 3, 2022
Merged

crypto: do not add undefined hash in webcrypto normalizeAlgorithm #42559

merged 2 commits into from Apr 3, 2022
+36 −8

Conversation

panva
Copy link
Member

@panva panva commented Apr 1, 2022 •
edited

Every now and then, when working with node's WebCryptoAPI, I noticed that the key's algorithm had an undefined hash property that wasn't present at the time of importing/deriving/generating. This, while not affecting the functionality of webcrypto, is not expected. This PR fixes that.

@panva panva added crypto webcrypto labels Apr 1, 2022
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Apr 1, 2022

Review requested:

@nodejs-github-bot nodejs-github-bot added the needs-ci label Apr 1, 2022
@tniessen
Copy link
Member

tniessen commented Apr 1, 2022

Looks like there are some related failures, could you take a look, please?

@panva
Copy link
Member Author

panva commented Apr 1, 2022

Looks like there are some related failures, could you take a look, please?

Yeah i'm trying to figure out how to best debug wpt failures right now.

@panva panva marked this pull request as draft Apr 1, 2022
@panva panva force-pushed the fix-undefined-hash-from-normalize branch from b09d864 to df21856 Compare Apr 1, 2022
@nodejs nodejs deleted a comment from nodejs-github-bot Apr 1, 2022
@panva panva marked this pull request as ready for review Apr 1, 2022
@nodejs nodejs deleted a comment from nodejs-github-bot Apr 1, 2022
@panva panva force-pushed the fix-undefined-hash-from-normalize branch from df21856 to 379b548 Compare Apr 1, 2022
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Apr 1, 2022

CI: https://ci.nodejs.org/job/node-test-pull-request/43259/

@panva panva removed the needs-ci label Apr 1, 2022
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Apr 1, 2022

CI: https://ci.nodejs.org/job/node-test-pull-request/43260/

@github-actions github-actions bot mentioned this pull request Apr 2, 2022
Open
9 tasks
@Trott Trott added the needs-ci label Apr 2, 2022
@Trott
Copy link
Member

Trott commented Apr 2, 2022

A bit of an aside, but I've re-added the needs-ci label.

It's perhaps a bit confusingly named, but the needs-ci label is there for the commit queue automation. It tells the automation that this is a PR that should land only if there is a passing Jenkins CI run. It's not a label letting other people know that we need a CI run. So it should generally not be manually removed (unless it was incorrectly placed on a PR in the first place.)

Refs: https://github.com/nodejs/node-core-utils/blob/919ec3b784a2589aa88a1b2629b98d57283555a4/lib/pr_checker.js#L431-L438

Trott
Trott approved these changes Apr 2, 2022
View changes
@panva panva added experimental author ready labels Apr 2, 2022
tniessen
tniessen approved these changes Apr 2, 2022
View changes
Copy link
Member

@tniessen tniessen left a comment

Tiny nit: since the file being test is called util.js, consider renaming the test to test-webcrypto-util.js (instead of test-webcrypto-utils.js).

test/parallel/test-webcrypto-utils.js Outdated Show resolved Hide resolved
test/parallel/test-webcrypto-utils.js Outdated Show resolved Hide resolved
lib/internal/crypto/util.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Apr 2, 2022

CI: https://ci.nodejs.org/job/node-test-pull-request/43266/

tniessen
tniessen approved these changes Apr 2, 2022
View changes
Copy link
Member

@tniessen tniessen left a comment

Thanks for updating :)

@github-actions github-actions bot mentioned this pull request Apr 3, 2022
Open
8 tasks
@panva panva added the commit-queue label Apr 3, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue label Apr 3, 2022
@nodejs-github-bot nodejs-github-bot merged commit 5d0eb10 into nodejs:master Apr 3, 2022
50 checks passed
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Apr 3, 2022

Landed in 5d0eb10

@panva panva deleted the fix-undefined-hash-from-normalize branch Apr 3, 2022
This was referenced Apr 4, 2022
Open
Open
juanarbol pushed a commit to juanarbol/node that referenced this issue Apr 5, 2022
@panva @juanarbol
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
fa7edb7 
PR-URL: nodejs#42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This was referenced Apr 5, 2022
Closed
Merged
@github-actions github-actions bot mentioned this pull request Apr 6, 2022
Open
11 tasks
juanarbol pushed a commit that referenced this issue Apr 6, 2022
@panva @juanarbol
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
a2f0738 
PR-URL: #42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@github-actions github-actions bot mentioned this pull request Apr 7, 2022
Open
17 tasks
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
@panva @xtx1130
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
256ca2e 
PR-URL: nodejs#42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
juanarbol pushed a commit that referenced this issue May 31, 2022
@panva @juanarbol
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
fd3a8bf 
PR-URL: #42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@juanarbol juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this issue Jun 27, 2022
@panva @danielleadams
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
963d7e0 
PR-URL: #42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this issue Jul 11, 2022
@panva @targos
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
e11e161 
PR-URL: #42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this issue Jul 31, 2022
@panva @targos
crypto: do not add undefined hash in webcrypto normalizeAlgorithm
297e374 
PR-URL: #42559
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@targos targos mentioned this pull request Aug 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott

@tniessen tniessen

Assignees
No one assigned
Labels
author ready crypto experimental needs-ci webcrypto
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@panva @nodejs-github-bot @tniessen @Trott
X Tutup