Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2159313002:
Miscelaneous updates to WebCrypto to match latest spec. (Closed)
|
Created:
4 years, 5 months ago by eroman Modified:
4 years, 5 months ago Reviewers:
Ryan Sleevi CC:
chromium-reviews, blink-reviews, tfarina, haraken Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMiscelaneous updates to WebCrypto to match latest spec.
* Matches version 2ac0c945d78d8b66246ce47c5a366a1fd6464aa5 of
https://github.com/w3c/webcrypto
* Adds comments referencing the spec procedure text
* Renames "algorithm" --> "normalizedAlgorithm" to match spec text
* Re-orders an algorithm normalization step
* Changes DataError --> TypeError for an importKey() case
BUG=628413
Committed: https://crrev.com/c28c47588a5093e8383b497c6bc8732f088762b3
Cr-Commit-Position: refs/heads/master@{#406467}
Patch Set 1 #
Total comments: 17
Patch Set 2 : double check spec text -- corrected one numbering, added 3 more notes, and moved up one parsing #Patch Set 3 : spelling fix cal --> call #
Messages
Total messages: 23 (13 generated)
The CQ bit was checked by eroman@chromium.org to run a CQ dry run
eroman@chromium.org changed reviewers: + rsleevi@chromium.org
https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp (right): https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:152: WebCryptoAlgorithm normalizedAlgorithm; renames algorithm --> normalizedAlgorithm; no other changes in logic here. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:186: WebCryptoAlgorithm normalizedAlgorithm; renames algorithm --> normalizedAlgorithm; no other changes in logic here. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:220: WebCryptoAlgorithm normalizedAlgorithm; renames algorithm --> normalizedAlgorithm; no other changes in logic here. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:254: WebCryptoAlgorithm normalizedAlgorithm; renames algorithm --> normalizedAlgorithm; no other changes in logic here. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:292: WebCryptoAlgorithm normalizedAlgorithm; renames algorithm --> normalizedAlgorithm; no other changes in logic here. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:318: WebCryptoAlgorithm normalizedAlgorithm; renames algorithm --> normalizedAlgorithm; no other changes in logic here. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:344: WebCryptoAlgorithm normalizedAlgorithm; Two changes: (1) renames algorithm --> normalizedAlgorithm (2) Moves algorithm normalization to happen before reading key data -- this is an observable change not covered by current tests. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:352: result->completeWithError(WebCryptoErrorTypeType, "Key data must be a buffer for non-JWK formats"); intentional change (crbug.com/628413). Will be clarified with comments when I address the TODO above. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:427: WebCryptoAlgorithm normalizedAlgorithm; rename wrapAlgorithm --> normalizedAlgorithm https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:438: if (!wrappingKey->canBeUsedForAlgorithm(normalizedAlgorithm, WebCryptoKeyUsageWrapKey, result)) This was moved to happen before extractability check (step 12). Otherwise no logic change. https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:488: WebCryptoAlgorithm normalizedAlgorithm; renamed unwrapAlgorithm --> normalizedAlgorithm https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:495: WebCryptoAlgorithm normalizedKeyAlgorithm; renamed unwrappedKeyAlgorithm --> normalizedKeyAlgorithm https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:528: WebCryptoAlgorithm normalizedAlgorithm; renamed algorithm --> normalizedAlgorithm; no other change in logic https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:563: WebCryptoAlgorithm normalizedAlgorithm; renamed algorithm --> normalizedAlgorithm; no other change in logic https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:569: WebCryptoAlgorithm normalizedDerivedKeyAlgorithm; Two changes: (1) renamed importAlgorithm --> normalizedDerivedKeyAlgorithm (2) Moved normalizedDerivedKeyAlgorithm to happen before key usage check
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp (right): https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:437: // then throw an InvalidAccessError. Typo: You copied 14.3.11.9 as 14.3.11.10 - but .10 is the usages check
Oh, and LGTM % nit
https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp (right): https://codereview.chromium.org/2159313002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/crypto/SubtleCrypto.cpp:437: // then throw an InvalidAccessError. On 2016/07/19 18:27:52, Ryan Sleevi (extremely slow) wrote: > Typo: You copied 14.3.11.9 as 14.3.11.10 - but .10 is the usages check Good spot! Ugh, embarrassing. I will cross check all of these and make sure I don't have other errors. (Looks like the URL I pasted for wrapKey() above doesn't work too so something went wrong here)
The CQ bit was unchecked by eroman@chromium.org
On 2016/07/19 19:17:13, eroman wrote: > I will cross check all of these and make sure I don't have other errors. I did, you didn't > (Looks like the URL I pasted for wrapKey() above doesn't work too so something > went wrong here) If you're copy/pasting from Rietveld, it adds a newline as whitespace which gets encoded in the URL as %20. In the actual source file, the URL is fine and loads.
The CQ bit was checked by eroman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I did another pass and corrected one numbering, the rest looked good.
The CQ bit was checked by eroman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by eroman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rsleevi@chromium.org Link to the patchset: https://codereview.chromium.org/2159313002/#ps40001 (title: "spelling fix cal --> call")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Miscelaneous updates to WebCrypto to match latest spec. * Matches version 2ac0c945d78d8b66246ce47c5a366a1fd6464aa5 of https://github.com/w3c/webcrypto * Adds comments referencing the spec procedure text * Renames "algorithm" --> "normalizedAlgorithm" to match spec text * Re-orders an algorithm normalization step * Changes DataError --> TypeError for an importKey() case BUG=628413 ========== to ========== Miscelaneous updates to WebCrypto to match latest spec. * Matches version 2ac0c945d78d8b66246ce47c5a366a1fd6464aa5 of https://github.com/w3c/webcrypto * Adds comments referencing the spec procedure text * Renames "algorithm" --> "normalizedAlgorithm" to match spec text * Re-orders an algorithm normalization step * Changes DataError --> TypeError for an importKey() case BUG=628413 Committed: https://crrev.com/c28c47588a5093e8383b497c6bc8732f088762b3 Cr-Commit-Position: refs/heads/master@{#406467} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/c28c47588a5093e8383b497c6bc8732f088762b3 Cr-Commit-Position: refs/heads/master@{#406467} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
