Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/jsc/bindings/webcrypto/JSJsonWebKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ template<> JsonWebKey convertDictionary<JsonWebKey>(JSGlobalObject& lexicalGloba
if (!ktyValue.isUndefined()) {
result.kty = convert<IDLDOMString>(lexicalGlobalObject, ktyValue);
RETURN_IF_EXCEPTION(throwScope, {});
} else {
throwRequiredMemberTypeError(lexicalGlobalObject, throwScope, "kty"_s, "JsonWebKey"_s, "DOMString"_s);
return {};
}
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
JSValue nValue;
if (isNullOrUndefined)
Expand Down
26 changes: 23 additions & 3 deletions test/js/web/crypto/web-crypto.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,26 @@
expect(isSigValid).toBe(true);
});

// W3C WebCrypto: JsonWebKey.kty is not a required dictionary member; each
// algorithm's import key operation rejects a missing/wrong kty with DataError.
describe("importKey jwk with missing kty rejects with DataError", () => {
const cases: Array<[string, object, object, KeyUsage[]]> = [
["AES-GCM", { k: "AAECAwQFBgcICQoLDA0ODw" }, { name: "AES-GCM" }, ["encrypt"]],
["HMAC", { k: "AAECAwQFBgcICQoLDA0ODw" }, { name: "HMAC", hash: "SHA-256" }, ["sign"]],
["RSA-OAEP", { n: "AQAB", e: "AQAB" }, { name: "RSA-OAEP", hash: "SHA-256" }, ["encrypt"]],
["ECDSA", { crv: "P-256", x: "", y: "" }, { name: "ECDSA", namedCurve: "P-256" }, ["verify"]],
["Ed25519", { crv: "Ed25519", x: "" }, { name: "Ed25519" }, ["verify"]],
];
it.each(cases)("%s", async (_name, jwk, alg, usages) => {
const err = await crypto.subtle.importKey("jwk", jwk as JsonWebKey, alg, true, usages).then(
() => null,
e => e,
);
expect(err).toBeInstanceOf(DOMException);
expect(err.name).toBe("DataError");
});
});

describe("unwrapKey JWK error handling", () => {
// Setup: AES-GCM key that can encrypt arbitrary bytes and also unwrap keys.
// We encrypt payloads that decrypt to invalid JWK data so the JWK parse path
Expand Down Expand Up @@ -134,7 +154,7 @@
expect(err.name).toBe("DataError");
});

// Previously this promise never settled: the TypeError from JsonWebKey
// Previously this promise never settled: the exception from JsonWebKey
// dictionary conversion escaped as an uncaught exception and the
// DeferredPromise was left in m_pendingPromises forever.
it("rejects when wrapped bytes are valid JSON but not a valid JWK", async () => {
Expand All @@ -142,11 +162,11 @@
const err = await crypto.subtle
.unwrapKey("jwk", wrapped, key, { name: "AES-GCM", iv }, { name: "AES-GCM" }, true, ["encrypt", "decrypt"])
.then(
() => null,
e => e,
);
expect(err).toBeInstanceOf(TypeError);
expect(err.message).toContain("kty");
expect(err).toBeInstanceOf(DOMException);
expect(err.name).toBe("DataError");

Check warning on line 169 in test/js/web/crypto/web-crypto.test.ts

View check run for this annotation

Claude / Claude Code Review

unwrapKey JWK regression tests no longer cover the scope.exception() path

Nit: with `kty` no longer required, the wrapped `{foo:"bar"}` payload in these two tests no longer throws inside `convertDictionary<JsonWebKey>`, so they no longer exercise the `scope.exception()` / `m_pendingPromises.remove(index)` branch at SubtleCrypto.cpp:1319-1323 they were written to guard — the rejection now comes from `CryptoKeyAES::importJwk` via the inner `exceptionCallback`, and the comment about "the exception from JsonWebKey dictionary conversion" is stale for this fixture. To keep
Comment thread
claude[bot] marked this conversation as resolved.
});

it("does not leak DeferredPromise in m_pendingPromises on JWK parse errors", async () => {
Expand Down
Loading