Bug 171678 – Cleanup: Extract CachedScript::mimeTypeAllowedByNosniff() into a common function

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

CachedScript::mimeType() is only used by CachedScript::mimeTypeAllowedByNosniff(). Moreover, it is unnecessary to lowercase the MIME type before querying MIMETypeRegistry as MIMETypeRegistry performs lookup case-insensitively. We should remove the lowercase conversion and inline CachedScript::mimeType() into CachedScript::mimeTypeAllowedByNosniff().

[Daniel Bates]

Even better, we should extract CachedScript::mimeTypeAllowedByNosniff() into a common function that can be shared by LoadableClassicScript and WorkerScriptLoader.

[Daniel Bates]

Created attachment 309075 [details]
Patch

[Daniel Bates]

I am open to suggestions on the name and placement of isScriptAllowedByNosniff(). When I wrote the patch (attachment #309075 [details]) I put this function in ResourceResponseBase.{cpp, h} because it operates on a ResourceResponse.

[Andy Estes]

Comment on attachment 309075 [details]
Patch

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

> Source/WebCore/platform/network/ResourceResponseBase.cpp:47
> +    String mimeType = extractMIMETypeFromMediaType(response.httpHeaderField(HTTPHeaderName::ContentType));
> +    return parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) != ContentTypeOptionsNosniff || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType);

Seems like this could be broken up a bit. For instance, if parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) is true then you don't need to compute the mimeType from the ContentType header.

[Andy Estes]

(In reply to Andy Estes from comment #4)
> Comment on attachment 309075 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309075&action=review
> 
> > Source/WebCore/platform/network/ResourceResponseBase.cpp:47
> > +    String mimeType = extractMIMETypeFromMediaType(response.httpHeaderField(HTTPHeaderName::ContentType));
> > +    return parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) != ContentTypeOptionsNosniff || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType);
> 
> Seems like this could be broken up a bit. For instance, if
> parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::
> XContentTypeOptions)) is true

... is not ContentTypeOptionsNosniff, that is.

[Daniel Bates]

(In reply to Andy Estes from comment #4)
> Comment on attachment 309075 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309075&action=review
> 
> > Source/WebCore/platform/network/ResourceResponseBase.cpp:47
> > +    String mimeType = extractMIMETypeFromMediaType(response.httpHeaderField(HTTPHeaderName::ContentType));
> > +    return parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) != ContentTypeOptionsNosniff || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType);
> 
> Seems like this could be broken up a bit. For instance, if
> parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::
> XContentTypeOptions)) is true then you don't need to compute the mimeType
> from the ContentType header.

Wow, I need more sleep.

Will update code before landing to implement isScriptAllowedByNosniff() as follows:

bool isScriptAllowedByNosniff(const ResourceResponse& response)
{
    if (parseContentTypeOptionsHeader(response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) != ContentTypeOptionsNosniff)
        return true;
    String mimeType = extractMIMETypeFromMediaType(response.httpHeaderField(HTTPHeaderName::ContentType));
    return MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType);
}

[Daniel Bates]

Committed r216199: <http://trac.webkit.org/changeset/216199>