Bug 203383 - Add more information to SRI failure console messages
Summary: Add more information to SRI failure console messages
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-24 14:26 PDT by Alex Christensen
Modified: 2019-10-28 12:56 PDT (History)
12 users (show)

See Also:


Attachments
Patch (20.69 KB, patch)
2019-10-24 14:29 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews212 for win-future (13.90 MB, application/zip)
2019-10-24 16:38 PDT, EWS Watchlist
no flags Details
Patch (22.53 KB, patch)
2019-10-24 17:20 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-10-24 14:26:42 PDT
Add more information to SRI failure console messages
Comment 1 Alex Christensen 2019-10-24 14:29:36 PDT
Created attachment 381843 [details]
Patch
Comment 2 EWS Watchlist 2019-10-24 16:38:10 PDT
Comment on attachment 381843 [details]
Patch

Attachment 381843 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13174705

New failing tests:
js/dom/modules/module-fetch-failure-not-cached.html
Comment 3 EWS Watchlist 2019-10-24 16:38:12 PDT
Created attachment 381858 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 4 Geoffrey Garen 2019-10-24 16:43:25 PDT
Comment on attachment 381843 [details]
Patch

r=me but please update test result expectations before landing
Comment 5 Alex Christensen 2019-10-24 17:20:11 PDT
Created attachment 381861 [details]
Patch
Comment 6 Chris Dumez 2019-10-24 18:18:52 PDT
Comment on attachment 381861 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381861&action=review

> Source/WebCore/loader/SubresourceIntegrity.cpp:216
> +    builder.append(". Failed integrity metadata check. ");

appendLiteral

> Source/WebCore/loader/SubresourceIntegrity.cpp:217
> +    builder.append("Content length: ");

appendLiteral

> Source/WebCore/loader/SubresourceIntegrity.cpp:221
> +        builder.append("(no content)");

appendLiteral

> Source/WebCore/loader/SubresourceIntegrity.cpp:222
> +    builder.append(", Expected content length: ");

appendLiteral

> Source/WebCore/loader/SubresourceIntegrity.cpp:224
> +    builder.append(", Expected metadata: ");

appendLiteral

> Source/WebCore/loader/SubresourceIntegrity.cpp:225
> +    builder.append(integrityMetadata);

appendLiteral
Comment 7 WebKit Commit Bot 2019-10-24 19:49:57 PDT
The commit-queue encountered the following flaky tests while processing attachment 381861 [details]:

The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2019-10-24 19:50:05 PDT
The commit-queue encountered the following flaky tests while processing attachment 381861 [details]:

imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html bug 203394 (author: ysuzuki@apple.com)
The commit-queue is continuing to process your patch.
Comment 9 WebKit Commit Bot 2019-10-24 20:52:11 PDT
The commit-queue encountered the following flaky tests while processing attachment 381861 [details]:

The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2019-10-24 20:52:18 PDT
The commit-queue encountered the following flaky tests while processing attachment 381861 [details]:

imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html bug 203394 (author: ysuzuki@apple.com)
The commit-queue is continuing to process your patch.
Comment 11 Chris Dumez 2019-10-24 21:05:33 PDT
(In reply to WebKit Commit Bot from comment #10)
> The commit-queue encountered the following flaky tests while processing
> attachment 381861 [details]:
> 
> imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-
> element/module/integrity.html bug 203394 (author: ysuzuki@apple.com)
> The commit-queue is continuing to process your patch.

Given how related this test is, I would not be surprised if this patch was the cause of the flakiness.
Comment 12 WebKit Commit Bot 2019-10-24 22:06:01 PDT
Comment on attachment 381861 [details]
Patch

Clearing flags on attachment: 381861

Committed r251582: <https://trac.webkit.org/changeset/251582>
Comment 13 WebKit Commit Bot 2019-10-24 22:06:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2019-10-24 22:07:18 PDT
<rdar://problem/56607815>
Comment 15 Truitt Savell 2019-10-25 09:39:53 PDT
It looks like the changes in https://trac.webkit.org/changeset/251582/webkit

broke imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html

on Mac wk1

history:
https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fsemantics%2Fscripting-1%2Fthe-script-element%2Fmodule%2Fintegrity.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity-actual.txt
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 1: TypeError: Cannot load script http://localhost:8800/html/semantics/scripting-1/the-script-element/module/integrity-mismatches.js. Failed integrity metadata check. Content length: 93, Expected content length: -1, Expected metadata: sha384-doesnotmatch
+CONSOLE MESSAGE: line 1: TypeError: Cannot load script http://localhost:8800/html/semantics/scripting-1/the-script-element/module/integrity-mismatches.js. Failed integrity metadata check. Content length: 93, Expected content length: 93, Expected metadata: sha384-doesnotmatch
 
 PASS The integrity attribute must have no affect on inline module scripts 
 PASS The integrity attribute must be verified on the top-level of a module and allow it to execute when it matches
Comment 16 Alex Christensen 2019-10-25 10:47:25 PDT
http://trac.webkit.org/r251596 fixes that test
Comment 17 Truitt Savell 2019-10-28 10:25:46 PDT
It looks like this test is still flakey:
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html

History:
https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fsemantics%2Fscripting-1%2Fthe-script-element%2Fmodule%2Fintegrity.html&platform=ios&platform=mac

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity-actual.txt
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 1: TypeError: Cannot load script http://localhost:8800/html/semantics/scripting-1/the-script-element/module/integrity-mismatches.js. Failed integrity metadata check. Content length: 93, Expected content length: 93, Expected metadata: sha384-doesnotmatch
+CONSOLE MESSAGE: line 1: TypeError: Cannot load script http://localhost:8800/html/semantics/scripting-1/the-script-element/module/integrity-mismatches.js. Failed integrity metadata check. Content length: 93, Expected content length: -1, Expected metadata: sha384-doesnotmatch
 
 PASS The integrity attribute must have no affect on inline module scripts 
 PASS The integrity attribute must be verified on the top-level of a module and allow it to execute when it matches
Comment 18 Alex Christensen 2019-10-28 11:27:09 PDT
http://trac.webkit.org/r251660 makes it always pass.
Comment 19 Alexey Proskuryakov 2019-10-28 12:56:29 PDT
Is it understood why the expected content length became unreliable here?