Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(289)

Issue 23537027: WebCrypto: [refactor] Use "unsigned" rather than "size_t" for buffer sizes. (Closed)

Created:
7 years, 3 months ago by eroman
Modified:
7 years, 3 months ago
Reviewers:
abarth-chromium
CC:
blink-reviews, jamesr, dglazkov+blink, eae+blinkwatch, abarth-chromium, Bryan Eyler
Visibility:
Public.

Description

WebCrypto: [refactor] Use "unsigned" rather than "size_t" for buffer sizes in the public API. Justification: I have noticed some instances where the Chromium-side implementation is directly passing this "size_t" as the buffer length to NSS. NSS however expects buffer lengths as "unsigned int", and the C++ compiler implicitly converts the "size_t" to an "unsigned int". This conversion is dangerous, since theoretically it could result in truncation of the buffer length given to NSS. (Under LP64 datamodel for instance). Chromium's compilation settings don't warn about such issues. In practice this isn't a problem, and likely never will be, since in Blink ArrayBuffers are limited to lengths of 2^32-1. It is therefore a good idea to pass a narrower datatype in the API to make this obvious. Embedder shouldn't have to worry about dealing with buffer sizes in the full range of size_t. BUG=245025 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157532

Patch Set 1 #

Patch Set 2 : Add missing file #

Patch Set 3 : Add deprecated method to keep chromium compiling #

Patch Set 4 : Add ifdef so it compiles properly on 32-bit systems #

Patch Set 5 : add an ifdef to coordinate with chromium side #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -24 lines) Patch
M Source/core/platform/chromium/support/WebCrypto.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/crypto/SubtleCrypto.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/testing/runner/MockWebCrypto.h View 1 chunk +6 lines, -6 lines 0 comments Download
M Source/testing/runner/MockWebCrypto.cpp View 5 chunks +7 lines, -7 lines 0 comments Download
M public/platform/WebCrypto.h View 1 2 3 4 3 chunks +10 lines, -7 lines 0 comments Download
M public/platform/WebCryptoAlgorithmParams.h View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
eroman
7 years, 3 months ago (2013-09-09 18:11:16 UTC) #1
abarth-chromium
lgtm
7 years, 3 months ago (2013-09-09 18:39:52 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/23537027/2001
7 years, 3 months ago (2013-09-09 18:41:46 UTC) #3
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 3 months ago (2013-09-09 19:11:04 UTC) #4
eroman
(Added a deprecated version to keep chromium compiling)
7 years, 3 months ago (2013-09-09 19:31:10 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/23537027/11001
7 years, 3 months ago (2013-09-09 19:31:26 UTC) #6
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_lint, webkit_python_tests, webkit_tests, webkit_unit_tests, weborigin_unittests, wtf_unittests ...
7 years, 3 months ago (2013-09-09 20:32:43 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/23537027/24001
7 years, 3 months ago (2013-09-10 00:04:55 UTC) #8
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 3 months ago (2013-09-10 00:36:55 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/23537027/35001
7 years, 3 months ago (2013-09-10 16:56:45 UTC) #10
commit-bot: I haz the power
7 years, 3 months ago (2013-09-10 18:10:56 UTC) #11
Message was sent while issue was closed.
Change committed as 157532

Powered by Google App Engine
This is Rietveld 408576698