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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2418083002: Fix LinkStyle SRI check when against 0 sized resource. (Closed)

Created:
4 years, 2 months ago by kouhei (in TOK)
Modified:
4 years, 2 months ago
CC:
chromium-reviews, blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org, gavinp+prerender_chromium.org, Yoav Weiss
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix LinkStyle SRI check when against 0 sized resource. Before this CL, SRI check in LinkStyle::setCSSStyleSheet assumed that the target CSS resource had >0 size. However, it is possible that the CSS size is 0. This CL removes the assert that assumed resourceBuffer() != nullptr, which isn't true when the CSS empty. In addition, this CL also ensures SRI check on empty CSS resource, which doesn't affect user visible behaviour, but needed to emit SRI verification failure messages. Test by csharrison@chromium.org BUG=655807 Committed: https://crrev.com/3b89d127027ca441b6abfcde56aee65add6423ca Cr-Commit-Position: refs/heads/master@{#425611}

Patch Set 1 #

Total comments: 4

Patch Set 2 : review #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -10 lines) Patch
A third_party/WebKit/LayoutTests/fast/loader/sri-with-empty-response.html View 1 1 chunk +7 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/resources/empty.css View 1 0 chunks +-1 lines, --1 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLLinkElement.cpp View 1 1 chunk +14 lines, -11 lines 1 comment Download

Messages

Total messages: 16 (8 generated)
kouhei (in TOK)
PTAL
4 years, 2 months ago (2016-10-14 08:09:19 UTC) #2
jww
lgtm w/test change https://codereview.chromium.org/2418083002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html File third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html (right): https://codereview.chromium.org/2418083002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html#newcode1 third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html:1: <!DOCTYPE html> Can you please write ...
4 years, 2 months ago (2016-10-14 17:42:12 UTC) #3
kouhei (in TOK)
https://codereview.chromium.org/2418083002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html File third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html (right): https://codereview.chromium.org/2418083002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html#newcode1 third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-check-for-zero-size-css.html:1: <!DOCTYPE html> On 2016/10/14 17:42:11, jww wrote: > Can ...
4 years, 2 months ago (2016-10-17 01:23:39 UTC) #6
kouhei (in TOK)
4 years, 2 months ago (2016-10-17 01:23:56 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2418083002/40001
4 years, 2 months ago (2016-10-17 01:23:58 UTC) #11
yhirano
lgtm https://codereview.chromium.org/2418083002/diff/40001/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp File third_party/WebKit/Source/core/html/HTMLLinkElement.cpp (right): https://codereview.chromium.org/2418083002/diff/40001/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp#newcode449 third_party/WebKit/Source/core/html/HTMLLinkElement.cpp:449: checkResult = SubresourceIntegrity::CheckSubresourceIntegrity( How about defining |checkResult| here?
4 years, 2 months ago (2016-10-17 02:10:32 UTC) #12
commit-bot: I haz the power
Committed patchset #2 (id:40001)
4 years, 2 months ago (2016-10-17 02:42:29 UTC) #14
commit-bot: I haz the power
4 years, 2 months ago (2016-10-17 02:44:42 UTC) #16
Message was sent while issue was closed. 
Patchset 2 (id:??) landed as
https://crrev.com/3b89d127027ca441b6abfcde56aee65add6423ca
Cr-Commit-Position: refs/heads/master@{#425611}

Powered by Google App Engine
This is Rietveld 408576698