PeculiarVentures / webcrypto-core

A input validation layer for WebCrypto polyfills.
MIT License
28 stars 13 forks source link

`toStringTag` to support feature detection #30

Closed brettz9 closed 4 years ago

brettz9 commented 4 years ago

Hi,

Would you be open to a PR to add Symbol.toStringTag, e.g., as our project needs for CryptoKey?

    get [Symbol.toStringTag]() {
        return 'CryptoKey';
    }

This should behave more in line with the browser API, and allow feature detection via Object.prototype.toString.call(obj).slice(8, -1).

E.g., the following logs "CryptoKey" in Chrome or Firefox:

(async () => {
console.log(Object.prototype.toString.call(await crypto.subtle.generateKey(
  {
    name: "HMAC",
    hash: {name: "SHA-512"}
  },
  true,
  ["sign", "verify"]
)).slice(8, -1));
})();

Our project (typeson-registry) needs this to support auto-cloning of objects that may contain CryptoKey objects.

microshine commented 4 years ago

Would it be better to use instanceof for that? I don't like an idea to replace an original string tag.

const { Crypto, CryptoKey } = require("@peculiar/webcrypto");

async function main() {
    const crypto = new Crypto();
    const alg = { name: "HMAC", hash: "SHA-256" };
    const key = await crypto.subtle.generateKey(alg, false, ["sign", "verify"]);

    console.log(key instanceof CryptoKey); // true
}

main().catch(e => console.error(e));

For node-webcrypto-ossl you need to use CryptoKey from webcrypto-core module. It doesn't export CryptoKey like @peculiar/webcrypto.

const { Crypto } = require("node-webcrypto-ossl");
const { CryptoKey } = require("webcrypto-core");
brettz9 commented 4 years ago

Sure, instanceof can be used. However, the instanceof operator presents the problem of not working across frames in the browser, and our code is polyglot code for Node and the browser (and we can't add the toStringTag ourselves since the object is frozen). (The instanceof problem could be overcome if you were to add Symbol.hasInstance to the class instead and add some internal duck-typing or such with that method, but presumably that would be even more of an unexpected change), and as you point out, it requires an extra require in our case.

The original string tag for custom classes is just [object Object], and I wouldn't think anyone would be using this mechanism to determine that something is an object--if anyone wanted to detect that it is an object they can of course just use the more reliable obj && typeof obj === 'object'.

While we could perform the instanceof check conditionally when detecting we are in the Node environment, besides being unreliable, it seems this would be getting further away from the benefits of a polyfill, with this particular feature uniquely allowing providing a browser-compatible, cross-frame-compatible hint that can be used when trying to detect specific object type, and the toString result can also be helpful in debugging (either of which may be reasons why HTML keeps defining new string tags for new object types).

microshine commented 4 years ago

@brettz9 I published a new version + webcrypto-core@1.1.5. Please try @peculiar/webcrypto or node-webcrypto-ossl with the latest version of webcrypto-core

brettz9 commented 4 years ago

Thanks very much, looks great to me in source...However, the changes don't seem to have resulted in their being present within the main file: build/webcrypto-core.js

brettz9 commented 4 years ago

Oh, nevermind, sorry--though I saw the version was updated, I must have overwritten--will double-check...

brettz9 commented 4 years ago

Ok, now I do have the right version 😛

However, getting this error:

ReferenceError: CryptoKey is not defined
      at SubtleCrypto.checkCryptoKey (node_modules/webcrypto-core/build/webcrypto-core.js:911:30)
      at SubtleCrypto.exportKey (node_modules/webcrypto-core/build/webcrypto-core.js:836:14)
      at Context.<anonymous> (test/test.js:738:45)

Apparently those CryptoKey imports were needed after all (for instanceof checks).

microshine commented 4 years ago

@brettz9 Please try one more. I've published a new version of webcrypto-core

I tested it using @peculiar/webcrypto. It works fine

brettz9 commented 4 years ago

Working like a charm, thanks so much!