Do not serialize meta element containing Content-Security-Policy

Do not serialize meta element containing Content-Security-Policy

BUG=595476
TEST=test updated

Review-Url: https://codereview.chromium.org/2713663003
Cr-Commit-Position: refs/heads/master@{#453398}
Committed: https://chromium.googlesource.com/chromium/src/+/6c1dadffa7a8dcb5479c9b259f059de6d99af8b4

[jianli, 2017-02-22 21:53:36]

jianli@chromium.org changed reviewers:
+ mkwst@chromium.org

[jianli, 2017-02-23 21:37:54]

The CQ bit was checked by jianli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 21:38:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 23:04:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 23:04:09]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-02-24 08:21:49]

One question:

https://codereview.chromium.org/2713663003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2713663003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:149: // the saved MHTML
page, there is no need to carry the directives. If they
Hrm. I'm not sure I understand what this means. For example, if `<meta
http-equiv="Content-Security-Policy" content="style-src 'self'">` is present,
what happens to `<link href="https://evil.com/style.css"></link>`? Is it
ignored? Is it loaded and serialized as part of the packaging? Or are you really
just serializing the active document?

[jianli, 2017-02-24 21:38:19]

https://codereview.chromium.org/2713663003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2713663003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:149: // the saved MHTML
page, there is no need to carry the directives. If they
On 2017/02/24 08:21:49, Mike West (sloooooow) wrote:
> Hrm. I'm not sure I understand what this means. For example, if `<meta
> http-equiv="Content-Security-Policy" content="style-src 'self'">` is present,
> what happens to `<link href="https://evil.com/style.css"></link>`? Is it
> ignored? Is it loaded and serialized as part of the packaging? Or are you
really
> just serializing the active document?

For the page containing the above example code, the style will be blocked due to
that it violates the Content-Security-Policy directive. So when the page is
serialized to MHTML, the evil style will not be encapsulated into MHTML  since
it is not loaded in the active document. The MHTML is loaded in full sandboxing
mode. Though the MHTML still contains '<link
href="https://evil.com/style.css"></link>', it will not be requested because all
the requests for resources not embedded in the MHTML will be blocked. So we
don't have any problem when the Content-Security-Policy directive is removed in
the MHTML.

[Mike West, 2017-02-25 17:16:51]

LGTM

[jianli, 2017-02-27 21:47:48]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2017-02-27 21:48:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-27 23:53:51]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488232068295790, "parent_rev":
"4bf51e553baa31ca6b1ab7e45f8dffc9ca114641", "commit_rev":
"6c1dadffa7a8dcb5479c9b259f059de6d99af8b4"}

[commit-bot: I haz the power, 2017-02-27 23:54:39]

Description was changed from

==========
Do not serialize meta element containing Content-Security-Policy

BUG=595476
TEST=test updated
==========

to

==========
Do not serialize meta element containing Content-Security-Policy

BUG=595476
TEST=test updated

Review-Url: https://codereview.chromium.org/2713663003
Cr-Commit-Position: refs/heads/master@{#453398}
Committed:
https://chromium.googlesource.com/chromium/src/+/6c1dadffa7a8dcb5479c9b259f05...
==========

[commit-bot: I haz the power, 2017-02-27 23:54:40]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/6c1dadffa7a8dcb5479c9b259f05...