Bug 71604 – Implement the sandbox directive for CSP

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Implement the sandbox directive for CSP

[Adam Barth]

Created attachment 113749 [details]
Patch

[Adam Barth]

Comment on attachment 113749 [details]
Patch

This patch depends on the patch in Bug 71603.

[WebKit Review Bot]

Comment on attachment 113749 [details]
Patch

Attachment 113749 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10337215

New failing tests:
http/tests/security/contentSecurityPolicy/sandbox-empty-subframe.html
http/tests/security/contentSecurityPolicy/sandbox-empty.html

[Sam Weinig]

Is there any reporting that is supposed to associated with this flag?

[Adam Barth]

> Is there any reporting that is supposed to associated with this flag?

I wasn't planning to spec any reporting because there isn't any reporting for the HTML5 sandbox attribute.  We can certainly ask the working group for an opinion on that topic though.

[Sam Weinig]

(In reply to comment #5)
> > Is there any reporting that is supposed to associated with this flag?
> 
> I wasn't planning to spec any reporting because there isn't any reporting for the HTML5 sandbox attribute.  We can certainly ask the working group for an opinion on that topic though.

Ok.  As it currently stands, it seems a bit odd that there is no reporting, but not a show stopper.

[WebKit Review Bot]

Comment on attachment 113749 [details]
Patch

Rejecting attachment 113749 [details] from commit-queue.

New failing tests:
http/tests/security/contentSecurityPolicy/sandbox-empty.html
Full output: http://queues.webkit.org/results/10331334

[Adam Barth]

Created attachment 113795 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 113795 [details]
Patch for landing

Rejecting attachment 113795 [details] from commit-queue.

New failing tests:
http/tests/security/contentSecurityPolicy/sandbox-empty.html
Full output: http://queues.webkit.org/results/10331511

[Adam Barth]

That test doesn't fail for me locally.  I'm not sure why.

[Sam Weinig]

(In reply to comment #10)
> That test doesn't fail for me locally.  I'm not sure why.

Maybe something to do with is the patch for 71603 is landed yet?

[Adam Barth]

> Maybe something to do with is the patch for 71603 is landed yet?

That seems to have been landed for a while.  I'm going to land the patch and see what the post-commit bots think.

[Adam Barth]

Comment on attachment 113795 [details]
Patch for landing

Clearing flags on attachment: 113795

Committed r99383: <http://trac.webkit.org/changeset/99383>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.

[Sam Weinig]

(In reply to comment #12)
> > Maybe something to do with is the patch for 71603 is landed yet?
> 
> That seems to have been landed for a while.  I'm going to land the patch and see what the post-commit bots think.

That's what I would do :).

[Adam Barth]

Looks like the problem is that the testing harness isn't prepared to handle the case when script is disabled in the main frame:

	base::debug::StackTrace::StackTrace() [0x5aaace]
	base::(anonymous namespace)::StackDumpSignalHandler() [0x593ff9]
	0x7fca7ec08af0
	v8::Context::Enter() [0x63fecc]
	WebCoreTestSupport::resetInternalsObject() [0x554ae0]
	WebKit::WebTestingSupport::resetInternalsObject() [0x45269e]
	TestShell::resetTestController() [0x442d2a]
	runTest() [0x4252f8]
	main [0x4259a3]
	0x7fca7ebf3c4d
	0x41a009

Will fix.