Skip to content

[wasm] Use "node:crypto" to polyfill getRandomValues on older node - #78696

Merged
maraf merged 2 commits into
dotnet:mainfrom
maraf:WasmNodeCrypto
Nov 23, 2022
Merged

[wasm] Use "node:crypto" to polyfill getRandomValues on older node#78696
maraf merged 2 commits into
dotnet:mainfrom
maraf:WasmNodeCrypto

Conversation

@maraf

Copy link
Copy Markdown
Member
  • Use "node:crypto" when globalThis.crypto is undefined or doesn't have getRandomValues.
  • Polyfill getRandomValues from webcrypto when available
  • Polyfill getRandomValues using randomBytes on even older node

Fixes#77927

@marafmaraf added this to the 8.0.0 milestone Nov 22, 2022
@marafmaraf self-assigned this Nov 22, 2022
@ghost

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Use "node:crypto" when globalThis.crypto is undefined or doesn't have getRandomValues.
  • Polyfill getRandomValues from webcrypto when available
  • Polyfill getRandomValues using randomBytes on even older node

Fixes #77927

Author:maraf
Assignees:maraf
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone:8.0.0

@maraf

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing

Copy link
Copy Markdown
Member

removing the insecure polyfill in the test runner breaks v8

@maraf

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@maraf
maraf merged commit 4e8e5e5 into dotnet:mainNov 23, 2022
@maraf
maraf deleted the WasmNodeCrypto branch November 23, 2022 13:09
@maraf

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3532333494

@radical

Copy link
Copy Markdown
Member

@maraf you tested this on node v14, v17, and v19. Our tests should be running with v14 (from emsdk). Do we need to make sure that this doesn't break for 17/19/others? IOW, do we need to run a few tests with these different versions of node?

@maraf

maraf commented Dec 5, 2022

Copy link
Copy Markdown
MemberAuthor

@maraf you tested this on node v14, v17, and v19. Our tests should be running with v14 (from emsdk). Do we need to make sure that this doesn't break for 17/19/others? IOW, do we need to run a few tests with these different versions of node?

I think this is a first such thing.
Also, node v19 support fetch out of the box, so you could skip installing a polyfill for it.

@pavelsavara do you have an opinion?

@pavelsavara

Copy link
Copy Markdown
Member

I would stick to v14 for bulk of the testing for Net8.
But having alternate versions in runtime-extra-platforms would be nice I would go for LTS or Current (18.12 and 19.2).

Or should we make the Current version the main and have v14 just for extra platforms?
It would match what we do with latest V8, right ?

@maraf

maraf commented Dec 5, 2022

Copy link
Copy Markdown
MemberAuthor

Yeah, having Current as primary target makes sense. Than targeting these extras on v14.

@radical

Copy link
Copy Markdown
Member

@maraf Could you please open an issue to track this?

@ghostghost locked as resolved and limited conversation to collaborators Jan 4, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@maraf@lewing@radical@pavelsavara