Bug 58014 - Implement CSP's options directive
Summary: Implement CSP's options directive
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 53572
  Show dependency tree / graph
 
Reported: 2011-04-06 23:32 PDT by Adam Barth
Modified: 2011-04-07 15:38 PDT (History)
4 users (show)

See Also:

Attachments
Patch (9.08 KB, patch)
2011-04-06 23:34 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch for landing (8.86 KB, patch)
2011-04-07 03:14 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch for landing (8.86 KB, patch)
2011-04-07 10:53 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-04-06 23:32:06 PDT 
Implement CSP's options directive
Comment 1 Adam Barth 2011-04-06 23:34:07 PDT 
Created attachment 88578 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-04-07 02:53:31 PDT 
Comment on attachment 88578 [details]
Patch

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

> Source/WebCore/page/ContentSecurityPolicy.cpp:454
> +        skipWhile<isOptionValueCharacter>(position, end);
> +
> +        String optionsValue(optionsValueBegin, position - optionsValueBegin);

I would almost put a comment here noting that position may be == end, but that we don't care.

> Source/WebCore/page/ContentSecurityPolicy.cpp:484
> +    return !m_scriptSrc || (m_options && m_options->disableXSSProtection());

Perhaps this m_options || m_options->disableXSSProtection check should be rolled into a single dispableXSSProtection() method then you don't need to repeat yourself.
Comment 3 Adam Barth 2011-04-07 03:14:51 PDT 
Created attachment 88601 [details]
Patch for landing
Comment 4 WebKit Commit Bot 2011-04-07 03:54:10 PDT 
Comment on attachment 88601 [details]
Patch for landing

Rejecting attachment 88601 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'build'..." exit_code: 2

Last 500 characters of output:
0
    setenv YACC /Developer/usr/bin/yacc
    /bin/sh -c /mnt/git/webkit-commit-queue/WebKitBuild/WebCore.build/Debug/WebCore.build/Script-5DF50887116F3077005202AB.sh

** BUILD FAILED **


The following build commands failed:
WebCore:
	CompileC /mnt/git/webkit-commit-queue/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/ContentSecurityPolicy.o /mnt/git/webkit-commit-queue/Source/WebCore/page/ContentSecurityPolicy.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2
(1 failure)


Full output: http://queues.webkit.org/results/8347540
Comment 5 Adam Barth 2011-04-07 10:53:16 PDT 
Created attachment 88662 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2011-04-07 13:51:42 PDT 
Comment on attachment 88662 [details]
Patch for landing

Clearing flags on attachment: 88662

Committed r83205: <http://trac.webkit.org/changeset/83205>
Comment 7 WebKit Commit Bot 2011-04-07 13:51:45 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 WebKit Review Bot 2011-04-07 15:38:40 PDT 
http://trac.webkit.org/changeset/83205 might have broken GTK Linux 32-bit Release and GTK Linux 64-bit Debug
The following tests are not passing:
media/context-menu-actions.html
media/media-fullscreen-inline.html
media/media-fullscreen-not-in-document.html