../../Source/WebCore/loader/cache/CachedScript.cpp:98:12: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses] return !parseContentTypeOptionsHeader(m_response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) == ContentTypeOptionsNosniff || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType()); ^ ~~ ../../Source/WebCore/loader/cache/CachedScript.cpp:98:12: note: add parentheses after the '!' to evaluate the comparison first return !parseContentTypeOptionsHeader(m_response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) == ContentTypeOptionsNosniff || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType()); ^ ( ) ../../Source/WebCore/loader/cache/CachedScript.cpp:98:12: note: add parentheses around left hand side expression to silence this warning return !parseContentTypeOptionsHeader(m_response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) == ContentTypeOptionsNosniff || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType()); ^ ( ) 1 warning generated. ---- It seems this warning found a real bug should be fixed.
Created attachment 253515 [details] Patch
Comment on attachment 253515 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253515&action=review > Source/WebCore/loader/cache/CachedScript.cpp:98 > + return !(parseContentTypeOptionsHeader(m_response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) == ContentTypeOptionsNosniff) || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType()); Shouldn't this just use != ?
Comment on attachment 253515 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253515&action=review >> Source/WebCore/loader/cache/CachedScript.cpp:98 >> + return !(parseContentTypeOptionsHeader(m_response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) == ContentTypeOptionsNosniff) || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType()); > > Shouldn't this just use != ? I agree that it should.
Also, we should test this feature. Now that we know it was broken.
Comment on attachment 253515 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253515&action=review >>> Source/WebCore/loader/cache/CachedScript.cpp:98 >>> + return !(parseContentTypeOptionsHeader(m_response.httpHeaderField(HTTPHeaderName::XContentTypeOptions)) == ContentTypeOptionsNosniff) || MIMETypeRegistry::isSupportedJavaScriptMIMEType(mimeType()); >> >> Shouldn't this just use != ? > > I agree that it should. Good point, I'll change it before landing.
(In reply to comment #4) > Also, we should test this feature. Now that we know it was broken. Source/WebCore/platform/network/HTTPParsers.h: ----------------------------------------------- ... enum ContentTypeOptionsDisposition { ContentTypeOptionsNone, ContentTypeOptionsNosniff }; ... It is already covered by testcases and this code isn't broken functionally, because ContentTypeOptionsNosniff gets 1 value in this enum and we were lucky and got the same results in both cases. X: ContentTypeOptionsNone (0) X: ContentTypeOptionsNosniff (1) !X==1 true false !(X==1) true false X!=1 true false
Created attachment 253991 [details] Patch for landing
( just a note, it is already fixed in blink near 2 years ago :) https://codereview.chromium.org/18753002 )
Comment on attachment 253991 [details] Patch for landing Clearing flags on attachment: 253991 Committed r185057: <http://trac.webkit.org/changeset/185057>
All reviewed patches have been landed. Closing bug.