From 9a440533d8f9862eb36d953f67b359ef802656ff Mon Sep 17 00:00:00 2001
From: marcin piotr miszczyk <mpmiszczyk@gmail.com>
Date: Thu, 16 Sep 2021 13:12:39 +0200
Subject: [PATCH] fix: do not fail tracer for malformed uri

Failing tracer will be detached which can brake tracing all together.
---
 lib/spandex_phoenix.ex                        | 10 +++++--
 .../phoenix/endpoint_test.exs                 | 26 +++++++++++++++++
 test/tracer_integration/plug/router_test.exs  | 29 +++++++++++++++++++
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/lib/spandex_phoenix.ex b/lib/spandex_phoenix.ex
index e771391..a08a728 100644
--- a/lib/spandex_phoenix.ex
+++ b/lib/spandex_phoenix.ex
@@ -196,7 +196,7 @@ defmodule SpandexPhoenix do
         method: method,
         query_string: conn.query_string,
         status_code: conn.status,
-        url: URI.decode(conn.request_path),
+        url: uri_decode(conn.request_path),
         user_agent: user_agent
       ],
       resource: method <> " " <> route,
@@ -262,11 +262,17 @@ defmodule SpandexPhoenix do
   end
 
   defp replace_path_param_with_name(path_params, path_component) do
-    decoded_component = URI.decode(path_component)
+    decoded_component = uri_decode(path_component)
 
     Enum.find_value(path_params, decoded_component, fn
       {param_name, ^decoded_component} -> ":#{param_name}"
       _ -> nil
     end)
   end
+
+  defp uri_decode(path_component) do
+    URI.decode(path_component)
+  rescue
+    _error -> "<malformed_uri>"
+  end
 end
diff --git a/test/tracer_integration/phoenix/endpoint_test.exs b/test/tracer_integration/phoenix/endpoint_test.exs
index 81c6c71..e3f0de9 100644
--- a/test/tracer_integration/phoenix/endpoint_test.exs
+++ b/test/tracer_integration/phoenix/endpoint_test.exs
@@ -29,6 +29,10 @@ defmodule TracerWithPhoenixEndpointTest do
   end
 
   defmodule ErrorView do
+    def render("400.html", _) do
+      "400 bad request"
+    end
+
     def render("404.html", _) do
       "404 not found"
     end
@@ -316,6 +320,28 @@ defmodule TracerWithPhoenixEndpointTest do
       assert Keyword.get(http, :url) == "/hello/+🤯"
     end
 
+    test "handles malformed URI" do
+      malicious_uri = "auth%%27%20AND%202*3*8=6*8%20AND%20%27zPT3%27!=%27zPT3%"
+
+      assert_raise Phoenix.Router.MalformedURIError, fn ->
+        call(Endpoint, :get, malicious_uri)
+      end
+
+      assert_receive {
+        :sent_trace,
+        %Spandex.Trace{
+          spans: [
+            %Spandex.Span{
+              resource: "GET /<malformed_uri>",
+              http: http
+            }
+          ]
+        }
+      }
+
+      assert Keyword.get(http, :url) == "<malformed_uri>"
+    end
+
     test "validates the options passed to the use macro" do
       Application.put_env(:spandex_phoenix, __MODULE__.EndpointWithInvalidFilterTraces, [])
 
diff --git a/test/tracer_integration/plug/router_test.exs b/test/tracer_integration/plug/router_test.exs
index 0359140..3c582ee 100644
--- a/test/tracer_integration/plug/router_test.exs
+++ b/test/tracer_integration/plug/router_test.exs
@@ -148,5 +148,34 @@ defmodule TracerWithPlugRouterTest do
       assert is_list(Keyword.get(error, :stacktrace))
       assert Keyword.get(error, :error?)
     end
+
+    test "is able to handle malformed URI" do
+      malicious_uri = "auth%%27%20AND%202*3*8=6*8%20AND%20%27zPT3%27!=%27zPT3%"
+      assert catch_error(call(Router, :get, malicious_uri))
+
+      assert_receive {
+        :sent_trace,
+        %Spandex.Trace{
+          spans: [
+            %Spandex.Span{
+              error: error,
+              http: http,
+              name: "request",
+              resource: "GET /<malformed_uri>"
+            }
+          ]
+        }
+      }
+
+      assert "GET" == Keyword.get(http, :method)
+      assert "<malformed_uri>" == Keyword.get(http, :url)
+
+      assert Keyword.get(error, :exception) == %Plug.Router.MalformedURIError{
+               message: "malformed URI \"#{malicious_uri}\"",
+               plug_status: 400
+             }
+
+      assert Keyword.get(error, :error?) == true
+    end
   end
 end