Url decode (#416)

* Fix URL decoding

* Decode URLs properly

CHANGELOG.md

@@ -18,6 +18,7 @@ All notable changes to this project will be documented in this file.
 - Do not error when activating an already activated account plausible/analytics#370
 - Ignore arrow keys when modifier keys are pressed plausible/analytics#363
 - Show correct stats when goal filter is combined with source plausible/analytics#374
+- Fix URL decoding in query parameters plausible/analytics#416
 
 ### Security
 - Do not run the plausible Docker container as root plausible/analytics#362

lib/plausible_web/controllers/api/external_controller.ex

@@ -71,7 +71,7 @@ defmodule PlausibleWeb.Api.ExternalController do
     if is_bot?(ua) do
       {:ok, nil}
     else
-      uri = params["url"] && URI.parse(URI.decode(params["url"]))
+      uri = params["url"] && URI.parse(params["url"])
       query = if uri && uri.query, do: URI.decode_query(uri.query), else: %{}
 
       ref = parse_referrer(uri, params["referrer"])
@@ -133,10 +133,11 @@ defmodule PlausibleWeb.Api.ExternalController do
   defp get_pathname(nil, _), do: "/"
 
   defp get_pathname(uri, hash_mode) do
-    pathname = uri.path || "/"
+    pathname = (uri.path || "/")
+               |> URI.decode
 
     if hash_mode && uri.fragment do
-      pathname <> "#" <> uri.fragment
+      pathname <> "#" <> URI.decode(uri.fragment)
     else
       pathname
     end

test/plausible_web/controllers/api/external_controller_test.exs

@@ -484,6 +484,25 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
     assert pageview.pathname == "/#page-a"
   end
 
+  test "decodes URL pathname, fragment and search", %{conn: conn} do
+    params = %{
+      n: "pageview",
+      u: "https://test.com/%EF%BA%9D%EF%BB%AD%EF%BA%8E%EF%BA%8B%EF%BA%AF-%EF%BB%AE%EF%BB%A4%EF%BA%B3%EF%BA%8E%EF%BA%92%EF%BB%97%EF%BA%8E%EF%BA%97?utm_source=%25balle%25",
+      d: "url-decode-test.com",
+      h: 1
+    }
+
+    conn
+    |> put_req_header("content-type", "text/plain")
+    |> post("/api/event", Jason.encode!(params))
+
+    pageview = get_event("url-decode-test.com")
+
+    assert pageview.hostname == "test.com"
+    assert pageview.pathname == "/ﺝﻭﺎﺋﺯ-ﻮﻤﺳﺎﺒﻗﺎﺗ"
+    assert pageview.utm_source == "%balle%"
+  end
+
   test "responds 400 when required fields are missing", %{conn: conn} do
     params = %{}
 

test/plausible_web/controllers/api/stats_controller/browsers_test.exs

@@ -20,10 +20,10 @@ defmodule PlausibleWeb.Api.StatsController.BrowsersTest do
 
     test "returns top browser versions by unique visitors", %{conn: conn, site: site} do
       filters = Jason.encode!(%{browser: "Chrome"})
-      conn = get(conn, "/api/stats/#{site.domain}/browser-versions?period=day&date=2019-01-01")
+      conn = get(conn, "/api/stats/#{site.domain}/browser-versions?period=day&date=2019-01-01&filters=#{filters}")
 
       assert json_response(conn, 200) == [
-               %{"name" => "78.0", "count" => 2, "percentage" => 100}
+               %{"name" => "78.0", "count" => 1, "percentage" => 100}
              ]
     end
   end

test/plausible_web/controllers/api/stats_controller/main_graph_test.exs

@@ -167,7 +167,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
         )
 
       res = json_response(conn, 200)
-      assert %{"name" => "Unique visitors", "count" => 2, "change" => 100} in res["top_stats"]
+      assert %{"name" => "Unique visitors", "count" => 3, "change" => 100} in res["top_stats"]
     end
 
     test "returns only visitors with specific screen size", %{conn: conn, site: site} do
@@ -180,7 +180,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
         )
 
       res = json_response(conn, 200)
-      assert %{"name" => "Unique visitors", "count" => 2, "change" => 100} in res["top_stats"]
+      assert %{"name" => "Unique visitors", "count" => 3, "change" => 100} in res["top_stats"]
     end
 
     test "returns only visitors with specific browser", %{conn: conn, site: site} do
@@ -193,7 +193,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
         )
 
       res = json_response(conn, 200)
-      assert %{"name" => "Unique visitors", "count" => 2, "change" => 100} in res["top_stats"]
+      assert %{"name" => "Unique visitors", "count" => 3, "change" => 100} in res["top_stats"]
     end
 
     test "returns only visitors with specific operating system", %{conn: conn, site: site} do
@@ -206,7 +206,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
         )
 
       res = json_response(conn, 200)
-      assert %{"name" => "Unique visitors", "count" => 2, "change" => 100} in res["top_stats"]
+      assert %{"name" => "Unique visitors", "count" => 3, "change" => 100} in res["top_stats"]
     end
   end
 end

test/plausible_web/controllers/api/stats_controller/operating_systems_test.exs

@@ -20,10 +20,10 @@ defmodule PlausibleWeb.Api.StatsController.OperatingSystemsTest do
 
     test "returns top OS versions by unique visitors", %{conn: conn, site: site} do
       filters = Jason.encode!(%{os: "Mac"})
-      conn = get(conn, "/api/stats/#{site.domain}/browser-versions?period=day&date=2019-01-01")
+      conn = get(conn, "/api/stats/#{site.domain}/operating-system-versions?period=day&date=2019-01-01&filters=#{filters}")
 
       assert json_response(conn, 200) == [
-               %{"name" => "10.15", "count" => 2, "percentage" => 100}
+               %{"name" => "10.15", "count" => 1, "percentage" => 100}
              ]
     end
   end

test/plausible_web/controllers/api/stats_controller/sources_test.exs

@@ -128,7 +128,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do
         )
 
       assert json_response(conn, 200) == %{
-               "total_visitors" => 3,
+               "total_visitors" => 4,
                "referrers" => [
                  %{"name" => "10words.com/page1", "url" => "10words.com", "count" => 2}
                ]
@@ -147,7 +147,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do
         )
 
       assert json_response(conn, 200) == %{
-               "total_visitors" => 3,
+               "total_visitors" => 4,
                "referrers" => [
                  %{
                    "name" => "10words.com/page1",

test/support/clickhouse_setup.ex

@@ -16,7 +16,8 @@ defmodule Plausible.Test.ClickhouseSetup do
         screen_size: "Desktop",
         referrer_source: "10words",
         referrer: "10words.com/page1",
-        timestamp: ~N[2019-01-01 00:00:00]
+        timestamp: ~N[2019-01-01 00:00:00],
+        session_id: @conversion_1_session_id,
       },
       %{
         name: "pageview",
@@ -161,7 +162,9 @@ defmodule Plausible.Test.ClickhouseSetup do
         country_code: "US",
         screen_size: "Desktop",
         browser: "Chrome",
+        browser_version: "78.0",
         operating_system: "Mac",
+        operating_system_version: "10.15",
         session_id: @conversion_1_session_id,
         is_bounce: true,
         duration: 100,