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

test/UtilProcessQueryStringParameters.cpp

Index: test/UtilProcessQueryStringParameters.cpp
diff --git a/test/UtilProcessQueryStringParameters.cpp b/test/UtilProcessQueryStringParameters.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..8ed93491fce7d9395f6ed4a297f63d5b302a5356
--- /dev/null
+++ b/test/UtilProcessQueryStringParameters.cpp
@@ -0,0 +1,216 @@
+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2015 Eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <fstream>
+#include <Windows.h>
+#include <gtest/gtest.h>
+
+#include "../src/shared/Utils.h"
+
+TEST(ProcessQueryStringParametersTest, EmptyParameterHandler)
+{
+  ProcessQueryStringParameters(L"some=qyery&string", std::function<bool(const std::wstring&, const std::wstring&)>());

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

Use ASSERT_NO_THROW here to clearly indicate the intent of the test.

Should probably be "query" instead of "qyery".

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

I've changed it, now it throws and there is ASSERT_THROW

+}
+
+TEST(ProcessQueryStringParametersTest, EmptyQueryString)
+{
+  ProcessQueryStringParameters(L"", [](const std::wstring&, const std::wstring&)->bool
+    {
+      EXPECT_FALSE(true); // it should not be called
+      return true;
+    });
+}
+
+TEST(ProcessQueryStringParametersTest, QueryStringWithOneNormalParameter)
+{
+  std::wstring parameterName, parameterValue;
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param=value",
+    [&parameterName, &parameterValue, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      EXPECT_EQ(1, ++callCounter); // fails on the second call
+      parameterName = name;
+      parameterValue = value;
+      return true;
+    });
+  EXPECT_EQ(L"param", parameterName);
+  EXPECT_EQ(L"value", parameterValue);
+
+  callCounter = 0;
+  ProcessQueryStringParameters(L"param=value",
+    [&parameterName, &parameterValue, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      EXPECT_EQ(1, ++callCounter); // fails on the second call
+      parameterName = name;
+      parameterValue = value;
+      return false;
+    });
+  EXPECT_EQ(L"param", parameterName);
+  EXPECT_EQ(L"value", parameterValue);
+}
+
+TEST(ProcessQueryStringParametersTest, QueryStringWithSeveralNormalParameters)
+{
+  std::pair<std::wstring, std::wstring> parameters[3];
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1=value1&param2=value2&param3=value3",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters[callCounter].first = name;
+      parameters[callCounter].second = value;
+      ++callCounter;
+      return true;
+    });

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

You've got some really horrible code duplication going on with this lambda. It
would be far better to define a function class whose body called push_back on a
vector of pairs.

You've also got minor variations in the behavior. You could avoid code
duplication there by having the test class provide a hook to its users that
allowed changing the return value.

  bool operator()(const std::wstring& name, const std::wstring& value)
  {
    if ( !hook(parameters.size(), name, value) )
    {
      return false;
    }
    parameters.push_back(make_pair(name,value));
    return true;
  }

For the hook functions:
  hookTrue(/*...*/) { return true; } // the default
  hookFalse(/*...*/) { return false; }
  hookOnlyOnce(/*...*/) { return count == 0; }
  hookLessThan3(/*...*/ { return count < 3; }

+  EXPECT_EQ(L"param1", parameters[0].first);
+  EXPECT_EQ(L"value1", parameters[0].second);
+  EXPECT_EQ(L"param2", parameters[1].first);
+  EXPECT_EQ(L"value2", parameters[1].second);
+  EXPECT_EQ(L"param3", parameters[2].first);
+  EXPECT_EQ(L"value3", parameters[2].second);
+  EXPECT_EQ(3, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, QueryStringWithEmptyParameterAtTheEnd)
+{
+  std::pair<std::wstring, std::wstring> parameters[2];
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1=value1&",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters[callCounter].first = name;
+      parameters[callCounter].second = value;
+      ++callCounter;
+      return true;
+    });

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

Since you're about to index a vector, you should first check that the vector
actually has the requisite elements.

    ASSERT_GE(2, callCounter)
    EXPECT_EQ(2, callCounter)

The first one terminates the test, preventing the reporting of spurious error
from indexing non-existent vector elements. If it were throw on operator[], it
would indicate a failure with the test implementation more than with the unit
under test.

The second one checks that you're have the number of elements expected.

+  EXPECT_EQ(L"param1", parameters[0].first);
+  EXPECT_EQ(L"value1", parameters[0].second);
+  EXPECT_EQ(L"", parameters[1].first);
+  EXPECT_EQ(L"", parameters[1].second);
+  EXPECT_EQ(2, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, QueryStringWithEmptyParameterAtTheBeginning)
+{
+  std::pair<std::wstring, std::wstring> parameters[2];
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"&param1=value1",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters[callCounter].first = name;
+      parameters[callCounter].second = value;
+      ++callCounter;
+      return true;
+    });
+  EXPECT_EQ(L"", parameters[0].first);
+  EXPECT_EQ(L"", parameters[0].second);
+  EXPECT_EQ(L"param1", parameters[1].first);
+  EXPECT_EQ(L"value1", parameters[1].second);
+  EXPECT_EQ(2, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, ParameterWithoutAssignSign)
+{
+  std::pair<std::wstring, std::wstring> parameters;
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters.first = name;
+      parameters.second = value;
+      ++callCounter;
+      return true;
+    });
+  EXPECT_EQ(L"param1", parameters.first);
+  EXPECT_EQ(L"", parameters.second);
+  EXPECT_EQ(1, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, ParametersWithoutAssignSign)
+{
+  std::pair<std::wstring, std::wstring> parameters[3];
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1&param2&param3",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters[callCounter].first = name;
+      parameters[callCounter].second = value;
+      ++callCounter;
+      return true;
+    });
+  EXPECT_EQ(L"param1", parameters[0].first);
+  EXPECT_EQ(L"", parameters[0].second);
+  EXPECT_EQ(L"param2", parameters[1].first);
+  EXPECT_EQ(L"", parameters[1].second);
+  EXPECT_EQ(L"param3", parameters[2].first);
+  EXPECT_EQ(L"", parameters[2].second);
+  EXPECT_EQ(3, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, ParameterWithSeveralAssignSigns)
+{
+  std::pair<std::wstring, std::wstring> parameters;
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1=value1.1=value1.2",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters.first = name;
+      parameters.second = value;
+      ++callCounter;
+      return true;
+    });
+  EXPECT_EQ(L"param1", parameters.first);
+  EXPECT_EQ(L"value1.1=value1.2", parameters.second);
+  EXPECT_EQ(1, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, ParameterWithEmptyValue)
+{
+  std::pair<std::wstring, std::wstring> parameters;
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1=",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters.first = name;
+      parameters.second = value;
+      ++callCounter;
+      return true;
+    });
+  EXPECT_EQ(L"param1", parameters.first);
+  EXPECT_EQ(L"", parameters.second);
+  EXPECT_EQ(1, callCounter);
+}
+
+TEST(ProcessQueryStringParametersTest, StopProcessingWhenHandlerReturnsFalse)
+{
+  std::pair<std::wstring, std::wstring> parameters[3];
+  int callCounter = 0;
+  ProcessQueryStringParameters(L"param1=value1&param2=value2&param3=value3",
+    [&parameters, &callCounter](const std::wstring& name, const std::wstring& value)->bool
+    {
+      parameters[callCounter].first = name;
+      parameters[callCounter].second = value;
+      ++callCounter;
+      return callCounter < 2;
+    });
+  EXPECT_EQ(L"param1", parameters[0].first);
+  EXPECT_EQ(L"value1", parameters[0].second);
+  EXPECT_EQ(L"param2", parameters[1].first);
+  EXPECT_EQ(L"value2", parameters[1].second);
+  EXPECT_EQ(L"", parameters[2].first);
+  EXPECT_EQ(L"", parameters[2].second);
+  EXPECT_EQ(2, callCounter);
+}