Bug 155466

Summary: Web Inspector: Display Content Security Policy hash in details sidebar for script and style elements
Product: WebKit Reporter: Daniel Bates <dbates>
Component: Web InspectorAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, bfulgham, commit-queue, graouts, joepeck, keith_miller, mark.lam, mattbaker, mkwst, msaboff, nvasilyev, saam, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch and Layout Tests
joepeck: review+
[Screenshot] Example UI none

Daniel Bates
Reported 2016-03-14 14:35:44 PDT
We should display a Content Security Policy hash expression in the node sidebar when an HTML style element or HTML script element for a non-external JavaScript script is selected. This will help a person expedite the process of transitioning an existing web page with inline JavaScript scripts and inline stylesheets to a web page that functions as expected with a Content Security Policy.
Attachments
Patch and Layout Tests (24.66 KB, patch)
2016-03-14 14:47 PDT, Daniel Bates 		joepeck: review+
DetailsFormatted DiffDiff
[Screenshot] Example UI (59.02 KB, image/png)
2016-03-14 14:50 PDT, Daniel Bates 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2016-03-14 14:36:43 PDT
<rdar://problem/25152480>
Daniel Bates
Comment 2 2016-03-14 14:47:14 PDT
Created attachment 274031 [details] Patch and Layout Tests
Daniel Bates
Comment 3 2016-03-14 14:50:07 PDT
Created attachment 274032 [details] [Screenshot] Example UI A screenshot that illustrates the appearance of a CSP hash in the sidebar when an HTML script element (for an inline JavaScript script) is selected.
Joseph Pecoraro
Comment 4 2016-03-14 15:37:45 PDT
Comment on attachment 274031 [details] Patch and Layout Tests View in context: https://bugs.webkit.org/attachment.cgi?id=274031&action=review r=me! Awesome. > Source/WebCore/inspector/InspectorDOMAgent.cpp:1284 > + return makeString("'sha256-", base64Encode(digest.data(), digest.size()), '\''); Are these single-quotes necessary? > Source/WebCore/inspector/InspectorFrontendHost.h:41 > +class Element; Is this needed? I don't see it used. > LayoutTests/inspector/dom/csp-hash.html:1 > +<html> We normally add a DOCTYPE. Would that affect things here? > LayoutTests/inspector/dom/csp-hash.html:48 > + WebInspector.domTreeManager.querySelector(documentNode.id, "#script-with-uncode-code-point-00C5", function(nodeId) { Typo: "uncode" => "unicode". Here and a few other places. > LayoutTests/inspector/dom/csp-hash.html:62 > + WebInspector.domTreeManager.querySelector(documentNode.id, "#script-with-uncode-code-point-212B", function(nodeId) { > + let domNode = WebInspector.domTreeManager.nodeForId(nodeId); > + let expectedHash = "'sha256-YcKgriaBGkU6FsWZXgDLv4Wo5UZ5Qe5hNp6Psb3RJOE='"; // Same hash as for script #script-with-uncode-code-point-00C5. > + InspectorTest.log(""); > + InspectorTest.expectThat(domNode, "Got DOMNode for #script-with-uncode-code-point-212B"); > + InspectorTest.expectThat(domNode.contentSecurityPolicyHash() === expectedHash, `DOMNode has hash ${expectedHash}`); > + }); An easier way to write this file could be a set of test that you loop through. Since it seems the contents of all of these functions are nearly identical. // Nit: Style this however you wish. let testCases = [ {selector: '#stylesheet-without-whitespace", hash: "'sha256-NW7+Fm6YV404pkklaopT0jgCBCmfOAn0K+NtIfyPN4A='"}, {selector: '#stylesheet-with-whitespace", hash: "'sha256-NW7+Fm6YV404pkklaopT0jgCBCmfOAn0K+NtIfyPN4A='"}, ... ]; for (let {selector, hash} of testCases) { WebInspector.domTreeManager.querySelector(documentNode.id, test.selector, function(nodeId) { let domNode = WebInspector.domTreeManager.nodeForId(nodeId); InspectorTest.log(""); InspectorTest.expectThat(domNode, `Got DOMNode for ${selector}`); InspectorTest.expectThat(domNode.contentSecurityPolicyHash() === hash, `DOMNode has hash ${hash}`); } } InspectorTest.completeTest(); > LayoutTests/inspector/dom/csp-hash.html:90 > + <p>Test for Content Security Policy hash support.</p> Please add " on DOM.DOMNode". > LayoutTests/inspector/dom/csp-hash.html:113 > + <!-- Hash of this script should be equivalent to hash of script script-with-uncode-code-point-00C5. --> Typo: "uncode" => "unicode"
Timothy Hatcher
Comment 5 2016-03-14 16:03:19 PDT
Comment on attachment 274031 [details] Patch and Layout Tests View in context: https://bugs.webkit.org/attachment.cgi?id=274031&action=review >> Source/WebCore/inspector/InspectorDOMAgent.cpp:1284 >> + return makeString("'sha256-", base64Encode(digest.data(), digest.size()), '\''); > > Are these single-quotes necessary? They are needed when putting the hash in the CSP header, but they clutter the UI. Lets drop them. A developer will know to add them if they use this. > Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js:41 > + this._identityNodeContentSecurityPolicyHashRow = new WebInspector.DetailsSectionSimpleRow(WebInspector.UIString("Content Security Policy Hash")); This is a long label. I think we can just say "CSP Hash", most developers will know what that is if they need it.
Daniel Bates
Comment 6 2016-03-14 16:52:21 PDT
(In reply to comment #4) > > Source/WebCore/inspector/InspectorDOMAgent.cpp:1284 > > + return makeString("'sha256-", base64Encode(digest.data(), digest.size()), '\''); > > Are these single-quotes necessary? > As mentioned in Timothy Hatcher's remark in comment #5, the single quotes are needed "when putting the hash in the CSP header". Timothy Hatcher suggested in the same comment that we omit these single quotes. > > Source/WebCore/inspector/InspectorFrontendHost.h:41 > > +class Element; > > Is this needed? I don't see it used. > No, it is not needed. Will remove. > > LayoutTests/inspector/dom/csp-hash.html:1 > > +<html> > > We normally add a DOCTYPE. Will add DOCTYPE tag. > Would that affect things here? > No. > > LayoutTests/inspector/dom/csp-hash.html:48 > > + WebInspector.domTreeManager.querySelector(documentNode.id, "#script-with-uncode-code-point-00C5", function(nodeId) { > > Typo: "uncode" => "unicode". Here and a few other places. > Will fix throughout this file. > > LayoutTests/inspector/dom/csp-hash.html:62 > > + WebInspector.domTreeManager.querySelector(documentNode.id, "#script-with-uncode-code-point-212B", function(nodeId) { > > + let domNode = WebInspector.domTreeManager.nodeForId(nodeId); > > + let expectedHash = "'sha256-YcKgriaBGkU6FsWZXgDLv4Wo5UZ5Qe5hNp6Psb3RJOE='"; // Same hash as for script #script-with-uncode-code-point-00C5. > > + InspectorTest.log(""); > > + InspectorTest.expectThat(domNode, "Got DOMNode for #script-with-uncode-code-point-212B"); > > + InspectorTest.expectThat(domNode.contentSecurityPolicyHash() === expectedHash, `DOMNode has hash ${expectedHash}`); > > + }); > > An easier way to write this file could be a set of test that you loop > through. Since it seems the contents of all of these functions are nearly > identical. > > // Nit: Style this however you wish. > let testCases = [ > {selector: '#stylesheet-without-whitespace", hash: > "'sha256-NW7+Fm6YV404pkklaopT0jgCBCmfOAn0K+NtIfyPN4A='"}, > {selector: '#stylesheet-with-whitespace", hash: > "'sha256-NW7+Fm6YV404pkklaopT0jgCBCmfOAn0K+NtIfyPN4A='"}, > ... > ]; > > for (let {selector, hash} of testCases) { > WebInspector.domTreeManager.querySelector(documentNode.id, > test.selector, function(nodeId) { > let domNode = WebInspector.domTreeManager.nodeForId(nodeId); > InspectorTest.log(""); > InspectorTest.expectThat(domNode, `Got DOMNode for ${selector}`); > InspectorTest.expectThat(domNode.contentSecurityPolicyHash() === > hash, `DOMNode has hash ${hash}`); > } > } > > InspectorTest.completeTest(); > Will use your idea to simplify the code in this file (cap-hash.html) and LayoutTests/inspector/dom/csp-big5-hash.html. > > LayoutTests/inspector/dom/csp-hash.html:90 > > + <p>Test for Content Security Policy hash support.</p> > > Please add " on DOM.DOMNode". > Will add. > > LayoutTests/inspector/dom/csp-hash.html:113 > > + <!-- Hash of this script should be equivalent to hash of script script-with-uncode-code-point-00C5. --> > > Typo: "uncode" => "unicode" Will fix throughout this file.
Daniel Bates
Comment 7 2016-03-14 16:53:14 PDT
(In reply to comment #5) > >> Source/WebCore/inspector/InspectorDOMAgent.cpp:1284 > >> + return makeString("'sha256-", base64Encode(digest.data(), digest.size()), '\''); > > > > Are these single-quotes necessary? > > They are needed when putting the hash in the CSP header, but they clutter > the UI. Lets drop them. A developer will know to add them if they use this. > Will omit the single quotes. > > Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js:41 > > + this._identityNodeContentSecurityPolicyHashRow = new WebInspector.DetailsSectionSimpleRow(WebInspector.UIString("Content Security Policy Hash")); > > This is a long label. I think we can just say "CSP Hash", most developers > will know what that is if they need it. Will change label to read "CSP Hash".
Daniel Bates
Comment 8 2016-03-14 17:39:48 PDT
Committed r198178: <http://trac.webkit.org/changeset/198178>
Note You need to log in before you can comment on or make changes to this bug.