crypto: add CryptoKey Symbol.toStringTag by panva · Pull Request #46042 · nodejs/node

panva · 2023-01-01T11:53:43Z

Adds Symbol.toStringTag getter to CryptoKey to match other implementations behaviour.

Fixes: #45987

nodejs-github-bot · 2023-01-01T11:53:47Z

Review requested:

@nodejs/crypto

closes nodejs#45987

nodejs-github-bot · 2023-01-01T16:00:19Z

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

bnoordhuis · 2023-01-02T11:48:24Z

I'd hold off on merging until web-platform-tests/wpt#37716 is accepted (and maybe wait a few days in case it gets reverted again.)

panva · 2023-01-02T11:58:39Z

I'd hold off on merging until web-platform-tests/wpt#37716 is accepted (and maybe wait a few days in case it gets reverted again.)

Why? This is a QoL change that's not contradicting any existing requirement and aligns with other implementer's behaviour.

Even if the WPT wouldn't get updated it won't be for its incorrectness but rather its lack of being a requirement.

bnoordhuis · 2023-01-02T12:02:25Z

On the off chance that the WPT people raise valid concerns about the change. It's not a break-the-world kind of bug so I'd advise against rushing out a fix.

panva · 2023-01-02T12:19:51Z

On the off chance that the WPT people raise valid concerns about the change. It's not a break-the-world kind of bug so I'd advise against rushing out a fix.

Fine. I disagree with holding off just for the record because any valid concern would go against years of existing browser implementors having this behaviour and unlikely to be met with browser vendors' support to change.

juanarbol

LGTM

panva · 2023-01-18T09:08:31Z

All browsers, Deno, Bun, even widely used polyfills like PeculiarVentures/webcrypto yield the same result.

IMO there's no point in waiting for WPTs to be updated. Even if they weren't updated because for some reason this does not fall under what WPTs should be checking, we'd still have no reason for misalignment.

panva · 2023-01-18T17:56:25Z

WPT was merged protest-free.

nodejs-github-bot · 2023-01-18T18:07:28Z

Landed in 13f518f

closes #45987 PR-URL: #46042 Fixes: #45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

Notable changes: * crypto: * (SEMVER-MINOR) add CryptoKey Symbol.toStringTag (Filip Skokan) [#46042](#46042) * (SEMVER-MINOR) add KeyObject Symbol.toStringTag (Filip Skokan) [#46043](#46043) * http: * (SEMVER-MINOR) join authorization headers (Marco Ippolito) [#45982](#45982) * lib: * add webstreams to Duplex.from() (Debadree Chatterjee) [#46190](#46190) * stream: * implement finished() for ReadableStream and WritableStream (Debadree Chatterjee) [#46205](#46205) PR-URL: TBD

Notable changes: * crypto: * (SEMVER-MINOR) add CryptoKey Symbol.toStringTag (Filip Skokan) [#46042](#46042) * (SEMVER-MINOR) add KeyObject Symbol.toStringTag (Filip Skokan) [#46043](#46043) * http: * (SEMVER-MINOR) join authorization headers (Marco Ippolito) [#45982](#45982) * lib: * add webstreams to Duplex.from() (Debadree Chatterjee) [#46190](#46190) * stream: * implement finished() for ReadableStream and WritableStream (Debadree Chatterjee) [#46205](#46205) PR-URL: #46286

closes nodejs#45987 PR-URL: nodejs#46042 Fixes: nodejs#45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

closes nodejs#45987 PR-URL: nodejs#46042 Fixes: nodejs#45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Backport-PR-URL: nodejs#46340

closes #45987 PR-URL: #46042 Fixes: #45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

closes nodejs#45987 PR-URL: nodejs#46042 Backport-PR-URL: nodejs#46340 Fixes: nodejs#45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

closes #45987 PR-URL: #46042 Fixes: #45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

closes nodejs#45987 PR-URL: nodejs#46042 Fixes: nodejs#45987 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Backport-PR-URL: nodejs#47336

panva added crypto Issues and PRs related to the crypto subsystem. webcrypto labels Jan 1, 2023

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 1, 2023

panva added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2023

This comment was marked as outdated.

Sign in to view

crypto: add CryptoKey Symbol.toStringTag

e604769

closes nodejs#45987

panva force-pushed the add-cryptokey-toStringTag branch from 7ea1625 to e604769 Compare January 1, 2023 14:26

This comment was marked as outdated.

Sign in to view

aduh95 approved these changes Jan 1, 2023

View reviewed changes

aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 1, 2023

panva mentioned this pull request Jan 1, 2023

crypto: add KeyObject Symbol.toStringTag #46043

Merged

panva added the semver-minor PRs that contain new features and should be released in the next minor version. label Jan 1, 2023

tniessen approved these changes Jan 1, 2023

View reviewed changes

panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 1, 2023

github-actions bot mentioned this pull request Jan 2, 2023

CI Reliability 2023-01-02 nodejs/reliability#476

Open

22 tasks

panva removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 2, 2023

juanarbol approved these changes Jan 2, 2023

View reviewed changes

github-actions bot mentioned this pull request Jan 3, 2023

CI Reliability 2023-01-03 nodejs/reliability#477

Open

23 tasks

aduh95 added the blocked PRs that are blocked by other issues or PRs. label Jan 3, 2023

This was referenced Jan 4, 2023

CI Reliability 2023-01-04 nodejs/reliability#478

Open

CI Reliability 2023-01-05 nodejs/reliability#479

Open

panva added commit-queue Add this label to land a pull request using GitHub Actions. and removed blocked PRs that are blocked by other issues or PRs. labels Jan 18, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 18, 2023

nodejs-github-bot merged commit 13f518f into nodejs:main Jan 18, 2023

panva deleted the add-cryptokey-toStringTag branch January 18, 2023 18:07

RafaelGSS mentioned this pull request Jan 20, 2023

v19.5.0 proposal #46286

Merged

panva mentioned this pull request Jan 24, 2023

[v18.x backport] crypto: add CryptoKey Symbol.toStringTag #46340

Closed

panva added the backport-open-v18.x label Jan 24, 2023

panva added the dont-land-on-v16.x label Jan 24, 2023

juanarbol mentioned this pull request Jan 28, 2023

V18.14.0 proposal #46396

Merged

panva removed dont-land-on-v16.x labels Mar 31, 2023

panva mentioned this pull request Mar 31, 2023

[v16.x backport] WebCryptoAPI fixes #47336

Closed

panva added the backport-open-v16.x label Mar 31, 2023

panva removed the backport-open-v16.x label Jul 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

crypto: add CryptoKey Symbol.toStringTag#46042

crypto: add CryptoKey Symbol.toStringTag#46042
nodejs-github-bot merged 1 commit intonodejs:mainfrom
panva:add-cryptokey-toStringTag

panva commented Jan 1, 2023 •

edited by aduh95

Loading

Uh oh!

nodejs-github-bot commented Jan 1, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Jan 1, 2023

Uh oh!

bnoordhuis commented Jan 2, 2023

Uh oh!

panva commented Jan 2, 2023

Uh oh!

bnoordhuis commented Jan 2, 2023

Uh oh!

panva commented Jan 2, 2023

Uh oh!

juanarbol left a comment

Uh oh!

panva commented Jan 18, 2023

Uh oh!

panva commented Jan 18, 2023

Uh oh!

nodejs-github-bot commented Jan 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

panva commented Jan 1, 2023 • edited by aduh95 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 1, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Jan 1, 2023

Uh oh!

bnoordhuis commented Jan 2, 2023

Uh oh!

panva commented Jan 2, 2023

Uh oh!

bnoordhuis commented Jan 2, 2023

Uh oh!

panva commented Jan 2, 2023

Uh oh!

juanarbol left a comment

Choose a reason for hiding this comment

Uh oh!

panva commented Jan 18, 2023

Uh oh!

panva commented Jan 18, 2023

Uh oh!

nodejs-github-bot commented Jan 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

panva commented Jan 1, 2023 •

edited by aduh95

Loading