| Summary: | Fix logical-not-parentheses warning in CachedScript.cpp | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||
| Component: | New Bugs | Assignee: | Csaba Osztrogonác <ossy> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, darin, japhet, ossy | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 145121 | ||||||||
| Attachments: |
|
||||||||
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. |
../../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.