Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: src/plugin/PluginWbPassThrough.cpp

Issue 6299667012780032: Issues #696,1231,1264,1265 - Improve handling in PassthroughApp (Closed)
Patch Set: Created Sept. 19, 2014, 2:41 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/plugin/PluginWbPassThrough.h ('k') | src/plugin/SinkPolicy.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/plugin/PluginWbPassThrough.cpp
===================================================================
--- a/src/plugin/PluginWbPassThrough.cpp
+++ b/src/plugin/PluginWbPassThrough.cpp
@@ -9,14 +9,27 @@
#include "PluginSettings.h"
#include "PluginClass.h"
#include "PluginSystem.h"
-
+#include <WinInet.h>
#include "wtypes.h"
-EXTERN_C IMAGE_DOS_HEADER __ImageBase;
+namespace
+{
+ std::string g_blockedByABPPage = "<!DOCTYPE html>"
+ "<html>"
+ "<body>"
+ "<!-- blocked by AdblockPlus -->"
+ "</body>"
+ "</html>";
+}
+WBPassthruSink::WBPassthruSink()
+ : m_currentPositionOfSentPage(0)
Oleksandr 2014/09/21 23:30:40 I wouldn't be entirely sure that IE does not reuse
sergei 2014/09/22 16:18:05 It does not reuse them. If it happens we will be a
Oleksandr 2014/10/03 14:36:37 Even though I can't provide an example when this m
+ , m_contentType(CFilter::EContentType::contentTypeAny)
+ , m_blockedInTransaction(false)
+{
+}
-
-int WBPassthruSink::GetContentTypeFromMimeType(CString mimeType)
+int WBPassthruSink::GetContentTypeFromMimeType(const CString& mimeType)
{
if (mimeType.Find(L"image/") >= 0)
{
@@ -47,7 +60,7 @@
return CFilter::contentTypeAny;
}
-int WBPassthruSink::GetContentTypeFromURL(CString src)
+int WBPassthruSink::GetContentTypeFromURL(const CString& src)
{
CString srcExt = src;
@@ -85,14 +98,10 @@
{
return CFilter::contentTypeSubdocument;
}
- else
- {
- return CFilter::contentTypeAny & ~CFilter::contentTypeSubdocument;
- }
-
+ return CFilter::contentTypeAny;
}
-int WBPassthruSink::GetContentType(CString mimeType, CString domain, CString src)
+int WBPassthruSink::GetContentType(const CString& mimeType, const CString& domain, const CString& src)
{
// No referer or mime type
// BINDSTRING_XDR_ORIGIN works only for IE v8+
@@ -115,44 +124,48 @@
////////////////////////////////////////////////////////////////////////////////////////
HRESULT WBPassthruSink::OnStart(LPCWSTR szUrl, IInternetProtocolSink *pOIProtSink,
IInternetBindInfo *pOIBindInfo, DWORD grfPI, HANDLE_PTR dwReserved,
- IInternetProtocol* pTargetProtocol)
+ IInternetProtocol* pTargetProtocol, bool& handled)
{
-
m_pTargetProtocol = pTargetProtocol;
bool isBlocked = false;
- m_shouldBlock = false;
- m_lastDataReported = false;
- CString src;
- src.Append(szUrl);
+ CString src = szUrl;
DEBUG_GENERAL(src);
CPluginClient::UnescapeUrl(src);
- CString boundDomain;
+ // call the impl of the base class as soon as possible because it initializes the base class
+ // members, used by this method. It queries for the required interfaces.
+ HRESULT hr = BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
Oleksandr 2014/09/21 23:30:40 I still have my concerns if the requests are actua
sergei 2014/09/22 16:18:05 I have checked it before and now again, it works i
Oleksandr 2014/10/03 14:36:37 Not really. Just wanted to double check if you've
+ if (FAILED(hr))
+ {
+ return hr;
+ }
+
CString mimeType;
- LPOLESTR mime[10];
if (pOIBindInfo)
{
ULONG resLen = 0;
// Apparently IE will report random mime type if there's more then 1 in the list.
// So we get the whole list and just use the first one (top priority one)
+ LPOLESTR mime[10];
pOIBindInfo->GetBindString(BINDSTRING_ACCEPT_MIMES, mime, 10, &resLen);
if (mime && resLen > 0)
{
mimeType.SetString(mime[0]);
}
- LPOLESTR bindToObject = 0;
- pOIBindInfo->GetBindString(BINDSTRING_FLAG_BIND_TO_OBJECT, &bindToObject, 1, &resLen);
- LPOLESTR domainRetrieved = 0;
- if (resLen == 0 || wcscmp(bindToObject, L"FALSE") == 0)
- {
+ LPOLESTR bindString = nullptr;
+ pOIBindInfo->GetBindString(BINDSTRING_FLAG_BIND_TO_OBJECT, &bindString, 1, &resLen);
+ LPOLESTR domainRetrieved = nullptr;
+ if (resLen == 0 || wcscmp(bindString, L"FALSE") == 0)
+ {
HRESULT hr = pOIBindInfo->GetBindString(BINDSTRING_XDR_ORIGIN, &domainRetrieved, 1, &resLen);
-
if ((hr == S_OK) && domainRetrieved && (resLen > 0))
{
- boundDomain.SetString(domainRetrieved);
+ m_boundDomain = domainRetrieved;
}
}
+ // We can obtain IBindCtx* here, but IEnumString obtained via IBindCtx::EnumObjectParam
+ // does not return any parameter, so it's useless.
}
CString cookie;
@@ -160,12 +173,9 @@
ULONG len2 = 2048;
#ifdef SUPPORT_FILTER
- int contentType = CFilter::contentTypeAny;
-
CPluginTab* tab = CPluginClass::GetTab(::GetCurrentThreadId());
CPluginClient* client = CPluginClient::GetInstance();
-
if (tab && client)
{
CString documentUrl = tab->GetDocumentUrl();
@@ -176,153 +186,134 @@
}
else if (CPluginSettings::GetInstance()->IsPluginEnabled() && !client->IsWhitelistedUrl(std::wstring(documentUrl)))
{
- boundDomain = tab->GetDocumentUrl();
-
- contentType = CFilter::contentTypeAny;
-
+ m_boundDomain = tab->GetDocumentUrl();
+ m_contentType = CFilter::contentTypeAny;
#ifdef SUPPORT_FRAME_CACHING
- if ((tab != 0) && (tab->IsFrameCached(src)))
+ if (nullptr != tab && tab->IsFrameCached(src))
Oleksandr 2014/09/21 23:30:40 No yoda conditions, please if (tab != nullptr && .
{
- contentType = CFilter::contentTypeSubdocument;
+ m_contentType = CFilter::contentTypeSubdocument;
}
else
#endif // SUPPORT_FRAME_CACHING
- contentType = GetContentType(mimeType, boundDomain, src);
- if (client->ShouldBlock(src, contentType, boundDomain, true))
{
- isBlocked = true;
-
- DEBUG_BLOCKER("Blocker::Blocking Http-request:" + src);
+ m_contentType = GetContentType(mimeType, m_boundDomain, src);
}
}
- if (!isBlocked)
- {
- DEBUG_BLOCKER("Blocker::Ignoring Http-request:" + src)
- }
}
+ if (nullptr == tab)
Oleksandr 2014/09/21 23:30:40 No yoda conditions, please
+ {
+ m_contentType = GetContentType(mimeType, m_boundDomain, src);
+ }
- if (tab == NULL)
{
- contentType = GetContentType(mimeType, boundDomain, src);
- if (client->ShouldBlock(src, contentType, boundDomain, true))
+ // Here is the heuristic which detects the requests issued by Flash.ocx.
+ // It turned out that the implementation from ''Flash.ocx'' (tested version is 15.0.0.152)
+ // returns quite minimal configuration in comparison with the implementation from Microsofts'
+ // libraries (see grfBINDF and bindInfo.dwOptions). The impl from MS often includes something
+ // else.
+ ATL::CComPtr<IBindStatusCallback> bscb;
+ if (SUCCEEDED(QueryServiceFromClient(&bscb)) && !!bscb)
{
- isBlocked = true;
+ DWORD grfBINDF = 0;
+ BINDINFO bindInfo = {};
+ bindInfo.cbSize = sizeof(bindInfo);
+ if (SUCCEEDED(bscb->GetBindInfo(&grfBINDF, &bindInfo))
+ && (BINDF_ASYNCHRONOUS | BINDF_ASYNCSTORAGE| BINDF_PULLDATA) == grfBINDF
+ && (BINDINFO_OPTIONS_ENABLE_UTF8 | BINDINFO_OPTIONS_USE_IE_ENCODING) == bindInfo.dwOptions
+ )
Oleksandr 2014/09/21 23:30:40 This looks quite neat, however can you maybe test
sergei 2014/09/22 16:18:05 I will test it with different <IE, Flash> as far a
+ {
+ m_contentType = CFilter::EContentType::contentTypeObjectSubrequest;
+ }
}
}
-#ifdef _DEBUG
- CString type;
+ // The descision about EContentType::contentTypeAny is made later in
+ // WBPassthruSink::BeginningTransaction. Sometimes here we cannot detect the request type, but
+ // in WBPassthruSink::BeginningTransaction the header Accept is available which allows to
+ // obtain the "request type" in our terminology.
+ if (nullptr != client
+ && CFilter::EContentType::contentTypeAny != m_contentType
+ && client->ShouldBlock(src, m_contentType, m_boundDomain, true))
+ {
+ isBlocked = true;
+ }
- if (contentType == CFilter::contentTypeDocument) type = "DOCUMENT";
- else if (contentType == CFilter::contentTypeObject) type = "OBJECT";
- else if (contentType == CFilter::contentTypeImage) type = "IMAGE";
- else if (contentType == CFilter::contentTypeScript) type = "SCRIPT";
- else if (contentType == CFilter::contentTypeOther) type = "OTHER";
- else if (contentType == CFilter::contentTypeUnknown) type = "OTHER";
- else if (contentType == CFilter::contentTypeSubdocument) type = "SUBDOCUMENT";
- else if (contentType == CFilter::contentTypeStyleSheet) type = "STYLESHEET";
- else type = "OTHER";
-
- if (isBlocked)
+ // For IE6 and earlier there is iframe back button issue, so avoid it.
+ if (isBlocked && client->GetIEVersion() > 6)
{
- CPluginDebug::DebugResultBlocking(type, src, boundDomain);
- }
- else
- {
- CPluginDebug::DebugResultIgnoring(type, src, boundDomain);
- }
-#endif
-
- //Fixes the iframe back button issue
- if (client->GetIEVersion() > 6)
- {
- if ((contentType == CFilter::contentTypeImage) && (isBlocked))
+ handled = true;
+ if (CFilter::EContentType::contentTypeImage == m_contentType)
{
- m_shouldBlock = true;
- BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
-
+ // IE shows a cross that img is not loaded
Oleksandr 2014/09/21 23:30:40 Does this happen even if we "eat" the ReportResult
sergei 2014/09/22 16:18:05 Have not tested yet, also I'm afraid that sending
Oleksandr 2014/10/03 14:36:37 This can go into other review anyway. On 2014/09/2
return INET_E_REDIRECT_FAILED;
-
}
- if (((contentType == CFilter::contentTypeSubdocument))&& (isBlocked))
+ if (CFilter::EContentType::contentTypeSubdocument == m_contentType)
{
- m_shouldBlock = true;
- BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
-
+ PassthroughAPP::CustomSinkStartPolicy<WBPassthru, WBPassthruSink>::GetProtocol(this)->m_shouldSupplyCustomContent = true;
m_spInternetProtocolSink->ReportProgress(BINDSTATUS_MIMETYPEAVAILABLE, L"text/html");
-
- //Here we check if we are running on Windows 8 Consumer Preview.
- //For some reason on that environment the next line causes IE to crash
- if (CPluginSettings::GetInstance()->GetWindowsBuildNumber() != 8250)
- {
- m_spInternetProtocolSink->ReportResult(INET_E_REDIRECT_FAILED, 0, szUrl);
- }
-
- return INET_E_REDIRECT_FAILED;
+ m_spInternetProtocolSink->ReportData(BSCF_FIRSTDATANOTIFICATION, 0, static_cast<ULONG>(g_blockedByABPPage.size()));
+ return S_OK;
}
- if (((contentType == CFilter::contentTypeScript)) && (isBlocked))
+ if (CFilter::EContentType::contentTypeScript == m_contentType)
{
- m_shouldBlock = true;
- BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
m_spInternetProtocolSink->ReportProgress(BINDSTATUS_MIMETYPEAVAILABLE, L"text/javascript");
m_spInternetProtocolSink->ReportResult(INET_E_REDIRECTING, 301, L"data:");
return INET_E_REDIRECT_FAILED;
}
- if ((contentType == CFilter::contentTypeXmlHttpRequest) && (isBlocked))
+ if (CFilter::EContentType::contentTypeXmlHttpRequest == m_contentType)
{
- m_shouldBlock = true;
- BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
m_spInternetProtocolSink->ReportResult(INET_E_REDIRECTING, 301, L"data:");
return INET_E_REDIRECT_FAILED;
}
- if ((isBlocked))
+ if (CFilter::EContentType::contentTypeAny != m_contentType)
{
- m_shouldBlock = true;
- BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
- m_spInternetProtocolSink->ReportResult(S_FALSE, 0, L"");
-
+ m_spInternetProtocolSink->ReportResult(INET_E_REDIRECTING, 301, L"data:");
return INET_E_REDIRECT_FAILED;
}
}
#endif // SUPPORT_FILTER
- return isBlocked ? S_FALSE : BaseClass::OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, pTargetProtocol);
+ return isBlocked ? S_FALSE : hr;
}
+HRESULT WBPassthruSink::OnRead(void* pv, ULONG cb, ULONG* pcbRead)
+{
+ if (nullptr == pv)
Oleksandr 2014/09/21 23:30:40 if (pv == nullptr) looks better, imho
+ {
+ return E_POINTER;
+ }
+ if (nullptr == pcbRead)
Oleksandr 2014/09/21 23:30:40 Same as above
+ {
+ return E_POINTER;
+ }
+ *pcbRead = 0;
-HRESULT WBPassthruSink::Read(void *pv, ULONG cb, ULONG* pcbRead)
-{
- if (m_shouldBlock)
+ if (PassthroughAPP::CustomSinkStartPolicy<WBPassthru, WBPassthruSink>::GetProtocol(this)->m_shouldSupplyCustomContent)
{
- *pcbRead = 0;
- if (!m_lastDataReported)
+ ULONG myPageSize = static_cast<ULONG>(g_blockedByABPPage.size());
Oleksandr 2014/09/21 23:30:40 abpPageSize would be a better name, IMHO
sergei 2014/09/22 16:18:05 Sorry, forgot to change the name. During the playi
+ auto positionGrow = std::min<ULONG>(cb, static_cast<ULONG>(g_blockedByABPPage.size() - m_currentPositionOfSentPage));
+ if (0 == positionGrow) {
Oleksandr 2014/09/21 23:30:40 positionGrow == 0 seems more logical
+ return S_FALSE;
+ }
+ std::copy(g_blockedByABPPage.begin(), g_blockedByABPPage.begin() + positionGrow,
+ stdext::make_checked_array_iterator(static_cast<char*>(pv), cb));
Oleksandr 2014/09/21 23:30:40 So this will throw a runtime error if cb < positio
sergei 2014/09/22 16:18:05 - It's not possible to compile without checked ite
Oleksandr 2014/10/03 14:36:37 I suppose you could use memcpy here instead, but I
+ *pcbRead = positionGrow;
+ m_currentPositionOfSentPage += positionGrow;
+
+ if (m_spInternetProtocolSink)
{
- if (cb <= 1)
- {
- //IE must've gone nuts if this happened, but let's be cool about it and report we have no more data
- m_spInternetProtocolSink->ReportResult(S_FALSE, 0, NULL);
- return S_FALSE;
- }
- *pcbRead = 1;
- memcpy(pv, " ", 1);
-
- if (m_spInternetProtocolSink != NULL)
- {
- m_spInternetProtocolSink->ReportResult(S_OK, 0, NULL);
- }
- m_lastDataReported = true;
- m_shouldBlock = false;
- return S_OK;
+ m_spInternetProtocolSink->ReportData(BSCF_INTERMEDIATEDATANOTIFICATION,
+ static_cast<ULONG>(m_currentPositionOfSentPage), myPageSize);
+ }
+ if (myPageSize == m_currentPositionOfSentPage && m_spInternetProtocolSink)
+ {
+ m_spInternetProtocolSink->ReportData(BSCF_DATAFULLYAVAILABLE, myPageSize, myPageSize);
+ m_spInternetProtocolSink->ReportResult(S_OK, 0, nullptr);
}
return S_OK;
}
- else
- {
-
- return m_pTargetProtocol->Read(pv, cb, pcbRead);
- }
- return S_OK;
+ return m_pTargetProtocol->Read(pv, cb, pcbRead);
}
STDMETHODIMP WBPassthruSink::Switch(
/* [in] */ PROTOCOLDATA *pProtocolData)
@@ -351,14 +342,63 @@
return m_spInternetProtocolSink ? m_spInternetProtocolSink->Switch(pProtocolData) : E_UNEXPECTED;
}
-
-STDMETHODIMP WBPassthruSink::BeginningTransaction(LPCWSTR szURL, LPCWSTR szHeaders, DWORD dwReserved, LPWSTR *pszAdditionalHeaders)
+STDMETHODIMP WBPassthruSink::BeginningTransaction(LPCWSTR szURL, LPCWSTR szHeaders, DWORD dwReserved, LPWSTR* pszAdditionalHeaders)
{
if (pszAdditionalHeaders)
{
- *pszAdditionalHeaders = 0;
+ *pszAdditionalHeaders = nullptr;
}
+ CPluginClient* client = nullptr;
+ if (CFilter::EContentType::contentTypeAny == m_contentType && (client = CPluginClient::GetInstance()))
+ {
+ auto acceptHeader = [&]() -> std::string
+ {
+ // Despite there is HTTP_QUERY_ACCEPT and other query info flags, they don't work here,
+ // only HTTP_QUERY_RAW_HEADERS_CRLF | HTTP_QUERY_FLAG_REQUEST_HEADERS does dork.
+ ATL::CComPtr<IWinInetHttpInfo> winInetHttpInfo;
+ HRESULT hr = m_spTargetProtocol->QueryInterface(&winInetHttpInfo);
+ if(FAILED(hr))
+ {
+ return "";
+ }
+ DWORD size = 0;
+ DWORD flags = 0;
+ hr = winInetHttpInfo->QueryInfo(HTTP_QUERY_RAW_HEADERS_CRLF | HTTP_QUERY_FLAG_REQUEST_HEADERS,
+ /*buffer*/nullptr, /* get size */&size, &flags, /*reserved*/ 0);
+ if(FAILED(hr))
+ {
+ return "";
+ }
+ std::string buf(size, '\0');
+ hr = winInetHttpInfo->QueryInfo(HTTP_QUERY_RAW_HEADERS_CRLF | HTTP_QUERY_FLAG_REQUEST_HEADERS,
+ &buf[0], &size, &flags, 0);
+ if(FAILED(hr))
+ {
+ return "";
+ }
+ char acceptHeader[] = "Accept:";
+ auto acceptHeaderBeginsAt = buf.find(acceptHeader);
+ if (std::string::npos == acceptHeaderBeginsAt)
+ {
+ return "";
+ }
+ acceptHeaderBeginsAt += sizeof(acceptHeader);
+ auto acceptHeaderEndsAt = buf.find("\n", acceptHeaderBeginsAt);
+ if (std::string::npos == acceptHeaderEndsAt)
+ {
+ return "";
+ }
+ return buf.substr(acceptHeaderBeginsAt, acceptHeaderEndsAt - acceptHeaderBeginsAt);
+ }();
+ m_contentType = GetContentTypeFromMimeType(ATL::CString(acceptHeader.c_str()));
+ bool isBlocked = client->ShouldBlock(szURL, m_contentType, m_boundDomain, /*debug flag but must be set*/true);
+ if (isBlocked)
+ {
+ m_blockedInTransaction = true;
+ return E_ABORT;
+ }
+ }
CComPtr<IHttpNegotiate> spHttpNegotiate;
QueryServiceFromClient(&spHttpNegotiate);
return spHttpNegotiate ? spHttpNegotiate->BeginningTransaction(szURL, szHeaders,dwReserved, pszAdditionalHeaders) : S_OK;
@@ -382,6 +422,24 @@
return m_spInternetProtocolSink ? m_spInternetProtocolSink->ReportProgress(ulStatusCode, szStatusText) : S_OK;
}
+STDMETHODIMP WBPassthruSink::ReportResult(/* [in] */ HRESULT hrResult, /* [in] */ DWORD dwError, /* [in] */ LPCWSTR szResult)
+{
+ if (m_blockedInTransaction)
+ {
+ // Don't notify the client about aborting of the operation, thus don't call BaseClass::ReportResult.
+ // Current method is called by the original protocol implementation and we are intercepting the
+ // call here and eating it, we will call the proper ReportResult later by ourself.
+ return S_OK;
+ }
+ return BaseClass::ReportResult(hrResult, dwError, szResult);
+}
+
+
+WBPassthru::WBPassthru()
+ : m_shouldSupplyCustomContent(false)
+ , m_hasOriginalStartCalled(false)
+{
+}
STDMETHODIMP WBPassthru::Start(LPCWSTR szUrl, IInternetProtocolSink *pOIProtSink,
IInternetBindInfo *pOIBindInfo, DWORD grfPI, HANDLE_PTR dwReserved)
@@ -392,13 +450,29 @@
return E_UNEXPECTED;
}
- return OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI,
- dwReserved, m_spInternetProtocol);
+ return OnStart(szUrl, pOIProtSink, pOIBindInfo, grfPI, dwReserved, m_spInternetProtocol);
}
- STDMETHODIMP WBPassthru::Read( /* [in, out] */ void *pv,/* [in] */ ULONG cb,/* [out] */ ULONG *pcbRead)
- {
-
- WBPassthruSink* pSink = GetSink();
- return pSink->Read(pv, cb, pcbRead);
- }
+STDMETHODIMP WBPassthru::Read(/* [in, out] */ void *pv,/* [in] */ ULONG cb,/* [out] */ ULONG *pcbRead)
+{
+ WBPassthruSink* pSink = GetSink();
+ return pSink->OnRead(pv, cb, pcbRead);
+}
+
+STDMETHODIMP WBPassthru::LockRequest(/* [in] */ DWORD options)
+{
+ if (!m_hasOriginalStartCalled)
+ {
+ return S_OK;
+ }
+ return BaseClass::LockRequest(options);
+}
+
+STDMETHODIMP WBPassthru::UnlockRequest()
+{
+ if (!m_hasOriginalStartCalled)
+ {
+ return S_OK;
+ }
+ return BaseClass::UnlockRequest();
+}
« no previous file with comments | « src/plugin/PluginWbPassThrough.h ('k') | src/plugin/SinkPolicy.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld