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

πŸ› Bug Report β€” Runtime APIs - nodejs compat - crypto - getRandomBytes() #2716

Closed
petebacondarwin opened this issue Sep 16, 2024 · 3 comments Β· Fixed by #2730
Closed

Comments

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Sep 16, 2024

The getRandomBytes() function will fail when called if it is not bound to the webcrypto object.
See cloudflare/workers-sdk#6721 for a test case.
Currently the calls fail with: TypeError: Illegal invocation: function called with incorrect this reference..

This has been fixed in unenv via unjs/unenv#309 but I think that the getRandomBytes() function should not require any particular this to work.

@kentonv
Copy link
Member

kentonv commented Sep 16, 2024

What's getRuntimeBytes()? Did you mean crypto.getRandomValues()?

Assuming so, it has the same behavior in Chrome:

Screenshot from 2024-09-16 15-47-53

@petebacondarwin
Copy link
Contributor Author

petebacondarwin commented Sep 16, 2024

To be more concrete. Using nodejs_compat (not v2, so no polyfills) running this Worker:

import { getBuiltinModule } from "node:process";

export default {
	async fetch(
		request: Request,
		env: Env,
		ctx: ExecutionContext
	): Promise<Response> {
		const results: string[] = [];
		const nodeCrypto = getBuiltinModule("crypto");
		const webcrypto = nodeCrypto.webcrypto;
		const getRandomValues = webcrypto.getRandomValues;
		results.push(crypto.getRandomValues(new Uint8Array(6)).toString()); // global
		results.push(webcrypto.getRandomValues(new Uint8Array(6)).toString()); // webcrypto
		results.push(nodeCrypto.getRandomValues(new Uint8Array(6)).toString()); // namespace import
		results.push(getRandomValues(new Uint8Array(6)).toString()); // free standing
		return Response.json(results);
	},
};

Results in exceptions on these lines:

nodeCrypto.getRandomValues(new Uint8Array(6)).toString(); // namespace import
getRandomValues(new Uint8Array(6)).toString(); // free standing

I could accept that the bare getRandomValues() call might not work. But I would expect the nodeCrypto.getRandomValues() call to work, which is the point of this issue.

@petebacondarwin petebacondarwin changed the title πŸ› Bug Report β€” Runtime APIs - nodejs compat - crypto - getRuntimeBytes πŸ› Bug Report β€” Runtime APIs - nodejs compat - crypto - getRandomBytes() Sep 17, 2024
@petebacondarwin
Copy link
Contributor Author

petebacondarwin commented Sep 17, 2024

Also given that this is for Node.js compatibility, I tested it on vanilla Node.js (18):

> const crypto = require("crypto")
undefined

> crypto.getRandomValues(new Uint8Array(6))
Uint8Array(6) [ 56, 123, 166, 65, 220, 221 ]

> crypto.webcrypto.getRandomValues(new Uint8Array(6))
Uint8Array(6) [ 89, 169, 107, 187, 129, 83 ]

> const getRandomValues = crypto.getRandomValues
undefined

> getRandomValues(new Uint8Array(6))
Uint8Array(6) [ 82, 122, 19, 15, 222, 155 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants