RESOLVED FIXED 145254
Fix logical-not-parentheses warning in CachedScript.cpp
https://bugs.webkit.org/show_bug.cgi?id=145254
Summary Fix logical-not-parentheses warning in CachedScript.cpp
Csaba Osztrogonác
Reported 2015-05-21 03:23:23 PDT
../../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.
Attachments
Patch (1.54 KB, patch)
2015-05-21 03:25 PDT, Csaba Osztrogonác
no flags
Patch for landing (1.51 KB, patch)
2015-06-01 03:47 PDT, Csaba Osztrogonác
no flags
Csaba Osztrogonác
Comment 1 2015-05-21 03:25:35 PDT
Alexey Proskuryakov
Comment 2 2015-05-23 18:48:37 PDT
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 != ?
Darin Adler
Comment 3 2015-05-26 14:53:12 PDT
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.
Darin Adler
Comment 4 2015-05-26 14:53:35 PDT
Also, we should test this feature. Now that we know it was broken.
Csaba Osztrogonác
Comment 5 2015-06-01 02:54:12 PDT
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.
Csaba Osztrogonác
Comment 6 2015-06-01 03:41:30 PDT
(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
Csaba Osztrogonác
Comment 7 2015-06-01 03:47:30 PDT
Created attachment 253991 [details] Patch for landing
Csaba Osztrogonác
Comment 8 2015-06-01 03:58:44 PDT
( just a note, it is already fixed in blink near 2 years ago :) https://codereview.chromium.org/18753002 )
WebKit Commit Bot
Comment 9 2015-06-01 04:37:04 PDT
Comment on attachment 253991 [details] Patch for landing Clearing flags on attachment: 253991 Committed r185057: <http://trac.webkit.org/changeset/185057>
WebKit Commit Bot
Comment 10 2015-06-01 04:37:08 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.