Issue 1115 - Some yahoo page not correctly shown on IE8 when ABP enabled

src/plugin/PluginWbPassThrough.cpp

Index: src/plugin/PluginWbPassThrough.cpp
diff --git a/src/plugin/PluginWbPassThrough.cpp b/src/plugin/PluginWbPassThrough.cpp
index 9069adf80a833d0bde42587ce55df2b2b75e0065..24d9c9fecdb197414f58877110e6f82544567142 100644
--- a/src/plugin/PluginWbPassThrough.cpp
+++ b/src/plugin/PluginWbPassThrough.cpp
@@ -87,6 +87,40 @@ namespace
     auto requestedWithHeader = ExtractHttpHeader<std::wstring>(additionalHeaders, L"X-Requested-With:", L"\n");
     return TrimString(requestedWithHeader) == L"XMLHttpRequest";
   }
+
+  int GetContentTypeFromString(const std::wstring& value)
+  {
+    auto lastDotIt = std::find(value.rbegin(), value.rend(), L'.').base();

[Eric, 2015/02/02 18:41:58]

It's much clearer just to call 'rfind':

    auto lastDotPos = value.rfind(L'.');

[sergei, 2015/02/12 14:44:06]

Good point!
fixed

+    if (lastDotIt == value.end())
+      return CFilter::contentTypeAny;
+
+    std::wstring ext(lastDotIt, value.end());
+    if (ext == L"jpg" || ext == L"gif" || ext == L"png" || ext == L"jpeg")
+    {
+      return CFilter::contentTypeImage;
+    }
+    else if (ext == L"css")
+    {
+      return CFilter::contentTypeStyleSheet;
+    }
+    else if (ext == L"js")
+    {
+      return CFilter::contentTypeScript;
+    }
+    else if (ext == L"xml")
+    {
+      return CFilter::contentTypeXmlHttpRequest;
+    }
+    else if (ext == L"swf")
+    {
+      return CFilter::contentTypeObject;
+    }
+    else if (ext == L"jsp" || ext == L"php" || ext == L"html")
+    {
+      return CFilter::contentTypeSubdocument;
+    }
+    return CFilter::contentTypeAny;
+  }
 }
 
 WBPassthruSink::WBPassthruSink()
@@ -129,44 +163,39 @@ int WBPassthruSink::GetContentTypeFromMimeType(const CString& mimeType)
 
 int WBPassthruSink::GetContentTypeFromURL(const std::wstring& src)
 {
-  CString srcLegacy = ToCString(src);
-  CString srcExt = srcLegacy;
-
-  int pos = 0;
-  if ((pos = srcLegacy.Find('?')) > 0)
-  {
-    srcExt = srcLegacy.Left(pos);
-  }
-
-  int lastDotIndex = srcExt.ReverseFind('.');
-  if (lastDotIndex < 0)
-    return CFilter::contentTypeAny;
-  CString ext = srcExt.Mid(lastDotIndex);
-  if (ext == L".jpg" || ext == L".gif" || ext == L".png" || ext == L".jpeg")
-  {
-    return CFilter::contentTypeImage;
-  }
-  else if (ext == L".css")
-  {
-    return CFilter::contentTypeStyleSheet;
-  }
-  else if (ext.Right(3) == L".js")
-  {
-    return CFilter::contentTypeScript;
-  }
-  else if (ext == L".xml")
+  // http://en.wikipedia.org/wiki/URI_scheme
+  auto schemeAndHierarchicalPartEndsAt = src.find(L'?');
+  if (schemeAndHierarchicalPartEndsAt == std::wstring::npos)
   {
-    return CFilter::contentTypeXmlHttpRequest;
+    schemeAndHierarchicalPartEndsAt = src.find(L'#');
   }
-  else if (ext == L".swf")
+  std::wstring schemeAndHierarchicalPart = src.substr(0, schemeAndHierarchicalPartEndsAt);
+  auto contentType = GetContentTypeFromString(schemeAndHierarchicalPart);
+  if (contentType != CFilter::contentTypeAny)
   {

[Eric, 2015/02/02 18:41:58]

As it's already been mentioned in the comments, it's hazardous to try to derive
a content type from the query string.
https://issues.adblockplus.org/ticket/1115#comment:7

Since this is only a problem on IE 8, we should add a version check here. If the
version is >= 9, we should always return here, even for 'contentTypeAny'

[sergei, 2015/02/12 14:44:06]

Actually, I'm not sure that it's a problem of only IE8, the execution flow gets
there in IE11 as well but I don't know concrete web site where it can derive the
content type from the query string parameter. However, I also agree that it can
be hazardous, so I've added it only for IE8.

-    return CFilter::contentTypeObject;
+    return contentType;
   }
-  else if (ext == L".jsp" || ext == L".php" || ext == L".html")
-  {
-    return CFilter::contentTypeSubdocument;
-  }
-  return CFilter::contentTypeAny;
+
+  ProcessQueryStringParameters(GetQueryString(src),
+    [&contentType](const std::wstring& name, const std::wstring& value)->bool
+    {
+      if (!value.empty())
+      {
+        contentType = GetContentTypeFromString(value);
+        if (contentType != CFilter::contentTypeAny)
+        {
+          return false;
+        }
+      }
+      contentType = GetContentTypeFromString(name);
+      if (contentType != CFilter::contentTypeAny)
+      {
+        return false;
+      }
+      return true;
+    });
+
+  return contentType;
 }
 
 int WBPassthruSink::GetContentType(const CString& mimeType, const std::wstring& domain, const std::wstring& src)