Issue 6299667012780032: Issues #696,1231,1264,1265 - Improve handling in PassthroughApp (Closed)
DescriptionSince the changes for these issues are quite coupled and they all are mainly in
PassthroughAPP we decided to put them in one codereview.
#696 Error page is displayed instead of blocked ad
- Solved by feeding IE the empty page. Sometimes it's a white rectangle,
sometimes it's hidden.
#1231 video doesn't play with ABP for IE
- object-subrequest was not handled at all, now we detect it but not completely
#1264 Figure out the best way to block subdomain requests
- For subdocument requests generate the empty html page, see #696
#1265 Figure out a method to detect the type of the request correctly
- We can obtain Accept header in BeginningTransaction and make a decision here.
- We also detect object-subrequest requests.
Changes:
- add OBJECT-SUBREQUEST to CPluginFilter
- add `CPluginDebug::DebugResultIgnoring(type, src, domain);` into `CPluginFilter::ShouldBlock`. There was already a call `CPluginDebug::DebugResultBlocking`, so this call is for consistency. On another hand that pair of calls is removed from `WBPassthruSink`, because it only duplicates blocked entries in the log.
- [PluginWbPassThrough.cpp] If cannot derive mime type from URL then use `CFilter::EContentType::contentTypeAny` without subtracting `CFilter::EContentType::contentTypeSubdocument`. It's indeed in many cases `subdocument`.
- add `handled` argument to our protocol Sink `OnStart` method. When `handled` is set to true the original (target) protocol::Start is not called. Before it was up to the return code, if it's successful then call it. But we need to return the successful code and don't need the call of the original protocol impl because it messes the workflow and crashes.
- rename `WBPassthruSink::Read` to `WBPassthruSink::OnRead`, because it's sink and its methods are some kind of slots and the proper names are `OnSomeEvent`, there is even already `OnStart`.
- get rid of `WBPassthruSink::m_lastDataReported` because it's not used anymore.
Add `WBPassthruSink::m_currentPositionOfSentPage` which is used by `WBPassthruSink::OnRead`.
- move `WBPassthruSink::m_shouldBlock` to `WBPassthru::m_shouldSupplyCustomContent`. When we send custom response we don't initiate any original protocol calls, thus we don't need to call its LockRequest/UnlockRequest. To support that logic WBPassthru::m_hasOriginalStartCalled is added. They (LockRequest/UnlockRequest) crash if original Start is not called. As well it cannot be in `WBPassthruSink` because it is destroyed before `UnlockRequest` is called.
Sometimes the mime type is not available in `WBPassthruSink::OnStart` and it's not possible to derive any information from the url. For such cases one can obtain `Accept` header in `BeginningTransaction` and make a decision here, if it's blocked we should return E_ABORT. It's possible via querying for `IWinInetHttpInfo` from the target protocol and calling `QueryInfo(HTTP_QUERY_RAW_HEADERS_CRLF | HTTP_QUERY_FLAG_REQUEST_HEADERS...)`. The latter call fails when it's done from our `OnStart` method, it seems that original target `protocol::Start` prepares the request and headers and then calls `BeginningTransaction`. To do it we need to store the url of the issuer of the request (`WBPassthruSink::m_boundDomain`) because it's needed by `CPluginFilter::ShouldBlock` in `BeginningTransaction`. As well as make `WBPassthruSink::m_contentType` which is used in `BeginningTransaction`, when the value is `CFilter::EContentType::contentTypeAny` it means that we could not derive the "type of request" in `OnStart` and we should try it here.
Patch Set 1 #
Total comments: 21
Patch Set 2 : Get rid of yoda style and rename blockedByABPPageSize local var #Patch Set 3 : rebase #
MessagesTotal messages: 9
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
