[Webkit-unassigned] [Bug 149873] [Content Extensions] Content blocking rules are not consulted for pings (<a ping>)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=149873

--- Comment #18 from Roopesh Chander <roop+webkit at roopc.net> ---
Comment on attachment 262988
  --> https://bugs.webkit.org/attachment.cgi?id=262988
Patch (fails one test)

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

>>> Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:173
>>> +    }
>> 
>> There are lots of places you use {} around single lines, which WebKit style doesn't like.  https://www.webkit.org/coding/coding-style.html Use Tools/Scripts/check-webkit-style to check this.

Sorry, I forgot to run check-webkit-style this time.

>>> Source/WebCore/loader/PingLoader.cpp:96
>>> +        request.setAllowCookies(false);
>> 
>> Why not call processContentExtensionRulesForLoad with this request instead of making a new function processContentExtensionRulesForPing?  I think that would be even cleaner.

Our options are:

 (a) Process content blocking rules on the ping URL before creating the request object.
     Then, if a request should be sent, call request.setAllowCookies(false) if applicable.

     (or)

 (b) Create a request object always, then call processContentExtensionRulesForLoad(request).
     If this results in BlockedStatus::Blocked, discard the request object.

Benjamin's earlier comment (https://bugs.webkit.org/show_bug.cgi?id=149873#c9) suggested that (a) is the preferred option, so that's what this patch implements. Implementing (b) would be a simpler patch because I wouldn't have to touch the ContentExtensionsBackend at all.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151014/79c2bd91/attachment-0001.html>