[webkit-reviews] review denied: [Bug 71851] Implement script MIME restrictions for X-Content-Type-Options: nosniff : [Attachment 114373] Proposed patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 9 15:20:45 PST 2011


Adam Barth <abarth at webkit.org> has denied Thomas Sepez <tsepez at chromium.org>'s
request for review:
Bug 71851: Implement script MIME restrictions for X-Content-Type-Options:
nosniff
https://bugs.webkit.org/show_bug.cgi?id=71851

Attachment 114373: Proposed patch.
https://bugs.webkit.org/attachment.cgi?id=114373&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=114373&action=review


> Source/WebCore/dom/ScriptElement.cpp:52
> +#if ENABLE(NOSNIFF)
> +#include "Console.h"
> +#include "DOMWindow.h"
> +#endif

These can just be included unconditionally.

> Source/WebCore/dom/ScriptElement.cpp:300
> +	   String consoleMessage = "Refused to load script from " +
m_cachedScript->url().string() + " due to MIME type mismatch.\n";

Do we want to say something about nosniff here?

> Source/WebCore/dom/ScriptElement.cpp:301
> +	  
m_element->document()->domWindow()->console()->addMessage(JSMessageSource,
LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());

m_element->document()->domWindow()->console()->addMessage

can be much shorter:

m_element->document()->addConsoleMessage

> Source/WebCore/loader/cache/CachedScript.cpp:43
> +#if ENABLE(NOSNIFF)
> +#include "HTTPParsers.h"
> +#endif

Same here.

> Source/WebCore/loader/cache/CachedScript.cpp:167
> +	       permittedMimeTypes.add("application/ecmascript");
> +	       permittedMimeTypes.add("application/javascript");
> +	       permittedMimeTypes.add("application/x-javascript");
> +	       permittedMimeTypes.add("text/ecmascript");
> +	       permittedMimeTypes.add("text/javascript");
> +	       permittedMimeTypes.add("text/jscript");
> +	       permittedMimeTypes.add("text/x-javascript");
> +	       permittedMimeTypes.add("text/vbs");
> +	       permittedMimeTypes.add("text/vbscript");

Does this list of types exist somewhere already?

> Source/WebCore/platform/network/HTTPParsers.cpp:379
> +    if (contentTypeOptions.findIgnoringCase("nosniff") != notFound)

This will match xnosniffbob, which isn't right.


More information about the webkit-reviews mailing list