Issue 2737343004: CSP: A new set of test with form resubmission.

Issue 2737343004: CSP: A new set of test with form resubmission. (Closed)

Created:
3 years, 9 months ago by arthursonzogni

Modified:
3 years, 9 months ago

Reviewers:
Mike West

CC:
alexmos, blink-reviews, chromium-reviews, clamy, nasko

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

CSP: A new set of test with form resubmission. The goal of this set of test is to document the current interactions between the "form-action" directive and form **re**submissions. It shows that the CSP that applies are the one of frame/window that is reloading the document, not the one that has initiated the reload. BUG=685074 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel Review-Url: https://codereview.chromium.org/2737343004 Cr-Commit-Position: refs/heads/master@{#458038} Committed: https://chromium.googlesource.com/chromium/src/+/01b5c7b1560e884426a2ef014c61acc74cd17a80

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed comments @mkwst #

Patch Set 3 : Re-add form submission into the main frame. #

Total comments: 2

Patch Set 4 : Use testharness for main page form resubmission. #

Created: 3 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+323 lines, -0 lines)			Patch
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html	View	1	1 chunk	+34 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child-expected.txt	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html	View	1	1 chunk	+57 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent-expected.txt	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html	View	1 2 3	1 chunk	+26 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page-expected.txt	View	1 2 3	1 chunk	+6 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child.html	View	1	1 chunk	+34 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child-expected.txt	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent.html	View	1	1 chunk	+58 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent-expected.txt	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none.html	View	1	1 chunk	+28 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none-and-reload.html	View	1	1 chunk	+23 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-callee.html	View	1 2 3	1 chunk	+20 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-caller.html	View	1 2 3	1 chunk	+13 lines, -0 lines	0 comments	Download

Messages

Total messages: 38 (24 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

arthursonzogni

Hi Mike, Here is the set of tests I was talking about yesterday. It shows ...

3 years, 9 months ago (2017-03-09 13:09:01 UTC) #5

Mike West

Documenting the existing behavior is great, even though I'm not sure it all makes sense. ...

3 years, 9 months ago (2017-03-10 13:44:41 UTC) #10

Documenting the existing behavior is great, even though I'm not sure it all
makes sense. :)

One comment about the way the tests are written, otherwise this looks good.

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html
(right):

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html:22:
testRunner.waitUntilDone();
We're generally writing layout tests in ways that allow us to upstream them to
the shared Web Platform Tests repository. Would you mind rewriting these tests
to use `testharness.js`? Something like:

```
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
  <iframe name="iframe"></iframe>
  <form method="POST" target="iframe" action="..."></form>
  <script>
    async_test(t => {
      window.addEventListener("message", t.step_func_done(e => {
        assert_equals(e.data, "securitypolicyviolation event fired");
      }));
    }, "This test tests a thing that should be tested.");
  </script>
</body>
```

Also, it would be good to serialize the error in the frame, send it up to this
document, and assert more things about its properties. The `effectiveDirective`
and `blockedURI` for instance.

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html
(right):

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html:7:
frame that has not defined any policies.
TODO(mkwst). This seems wrong.

arthursonzogni

Thanks Mike! I transformed the tests such that they are using testharness.js I droped form-action-resubmission-main-page.html, ...

3 years, 9 months ago (2017-03-13 13:25:55 UTC) #14

Thanks Mike!

I transformed the tests such that they are using testharness.js
I droped form-action-resubmission-main-page.html, I will probably add it latter.

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html
(right):

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html:22:
testRunner.waitUntilDone();
On 2017/03/10 13:44:41, Mike West (Slow.) wrote:
> We're generally writing layout tests in ways that allow us to upstream them to
> the shared Web Platform Tests repository. Would you mind rewriting these tests
> to use `testharness.js`? Something like:
> 
> ```
> <!DOCTYPE html>
> <script src="/resources/testharness.js"></script>
> <script src="/resources/testharnessreport.js"></script>
> <body>
>   <iframe name="iframe"></iframe>
>   <form method="POST" target="iframe" action="..."></form>
>   <script>
>     async_test(t => {
>       window.addEventListener("message", t.step_func_done(e => {
>         assert_equals(e.data, "securitypolicyviolation event fired");
>       }));
>     }, "This test tests a thing that should be tested.");
>   </script>
> </body>
> ```
> 
> Also, it would be good to serialize the error in the frame, send it up to this
> document, and assert more things about its properties. The
`effectiveDirective`
> and `blockedURI` for instance.

Thanks for the example. I had issues using it. I will transform all the tests.

FYI: the securitypolicyviolation event cannot be transmitted with
window.postMessage or serialized with JSON.stringify, that's why I had to build
a new object properties by properties.

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html
(right):

https://codereview.chromium.org/2737343004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html:7:
frame that has not defined any policies.
On 2017/03/10 13:44:41, Mike West (Slow.) wrote:
> TODO(mkwst). This seems wrong.

Okay, I will also add it to
form-action-resubmission-window-reload-from-parent.html.

arthursonzogni

Hi Mike, I added back the test that does a form resubmission in the main ...

3 years, 9 months ago (2017-03-15 17:06:56 UTC) #20

Mike West

LGTM. https://codereview.chromium.org/2737343004/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html File third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html (right): https://codereview.chromium.org/2737343004/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html#newcode7 third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html:7: TODO(mkwst). This might be wrong. Can you change ...

3 years, 9 months ago (2017-03-16 06:09:20 UTC) #23

arthursonzogni

https://codereview.chromium.org/2737343004/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html File third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html (right): https://codereview.chromium.org/2737343004/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html#newcode7 third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html:7: TODO(mkwst). This might be wrong. On 2017/03/16 06:09:20, Mike ...

3 years, 9 months ago (2017-03-16 09:32:52 UTC) #24