assert,crypto: make KeyObject and CryptoKey testable for equality

[panva]

This makes CryptoKey and KeyObject instances testable using assert.deepStrictEqual and assert.deepEqual.

The state before was that

• deepEqual did not check extractable, algorithm, usages
• neither deepEqual or deepStrict equal compared the key material, so two different keys with the other properties matching evaluated as equal

[panva]

cc @nodejs/crypto

[nodejs-github-bot]

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

[panva]

cc @nodejs/crypto

[aduh95]

I think the use case makes sense, and I'm not able to able to suggest a better implementation, so let's roll with this.

[aduh95]

nit: I find Buffer.from easier to understand (I had to go read the docs to get what the second argument of Buffer.alloc was for)

Suggested change

	consta=crypto.createSecretKey(Buffer.alloc(1,0));
	constb=crypto.createSecretKey(Buffer.alloc(1,1));
	consta=crypto.createSecretKey(Buffer.from([0]));
	constb=crypto.createSecretKey(Buffer.from([1]));

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/50897 ✔ Done loading data for nodejs/node/pull/50897 ----------------------------------- PR info ------------------------------------ Title assert,crypto: make KeyObject and CryptoKey testable for equality (#50897) Author Filip Skokan (@panva) Branch panva:crypto-objects-equal -> nodejs:main Labels crypto, assert, util, needs-ci, review wanted, webcrypto, commit-queue-rebase Commits 2 - crypto: update CryptoKey symbol properties - assert,crypto: make KeyObject and CryptoKey testable for equality Committers 1 - Filip Skokan PR-URL: https://github.com/nodejs/node/pull/50897 Reviewed-By: Antoine du Hamel ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/50897 Reviewed-By: Antoine du Hamel -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 24 Nov 2023 16:50:43 GMT ✔ Approvals: 1 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/50897#pullrequestreview-1785408839 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-11-25T19:19:00Z: https://ci.nodejs.org/job/node-test-pull-request/55920/ ⚠ Commits were pushed after the last Full PR CI run: ⚠ - crypto: update CryptoKey symbol properties ⚠ - assert,crypto: make KeyObject and CryptoKey testable for equality - Querying data for job/node-test-pull-request/55920/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/7237974363

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 154afbe...0afe731

[chharvey]

Not sure if this is the right place to comment but I noticed a regression from v18 to v20, and this PR seems vaguely related.

consta=['a'];assert.deepStrictEqual(newSet([a,['b']]),newSet([a,['b']]));

• v18.20.2: pass — no error as expected
• v20.0.0: fail — unexpected error:

AssertionError [ERR_ASSERTION]: Values have same structure but are not reference-equal:

Set(2){[ 'a' ], [ 'b' ] } 

The bug seems to occur only when comparing Set objects, with a mix of elements where some elements are compared by reference (===) and other elements are compared by deep equality.

[aduh95]

@chharvey can you open a new issue to track this please? (FWIW I'm able to reproduce)

[chharvey]

@aduh95 thanks for your reply. I think I found the culprit and left a comment: #46593 (comment)

[panva]

@chharvey can you open a new issue to track this please?

[chharvey]

@aduh95@panva I opened this issue: #53423