Escape domain when creating URL for favicon (#2942)

* Escape domain when constructing favicon URL

A domain may include a slash, and in that case the domain must be
escaped, before it is used as an attribute for the image tag.

* match with 'conn.request_path' instead + test

---------

Co-authored-by: Robert Joonas <robertjoonas16@gmail.com>

CHANGELOG.md

@@ -24,6 +24,7 @@ All notable changes to this project will be documented in this file.
 - Add error message in case a transfer to an invited (but not joined) user is requested plausible/analytics#2651
 - Fix bug with [showing property breakdown with a prop filter](https://github.com/plausible/analytics/issues/1789)
 - Fix bug when combining goal and prop filters plausible/analytics#2654
+- Fix broken favicons when domain includes a slash
 
 ### Changed
 - Treat page filter as entry page filter for `bounce_rate`

lib/plausible_web/plugs/favicon.ex

@@ -80,11 +80,11 @@ defmodule PlausibleWeb.Favicon do
 
   """
   def call(conn, favicon_domains: favicon_domains) do
-    case conn.path_info do
-      ["favicon", "sources", "placeholder"] ->
+    case conn.request_path do
+      "/favicon/sources/placeholder" ->
         send_placeholder(conn)
 
-      ["favicon", "sources", source] ->
+      "/favicon/sources/" <> source ->
         clean_source = URI.decode_www_form(source)
         domain = Map.get(favicon_domains, clean_source, clean_source)
 

test/plausible_web/plugs/favicon_test.exs

@@ -38,6 +38,24 @@ defmodule PlausibleWeb.FaviconTest do
     assert conn.resp_body == "favicon response body"
   end
 
+  test "requests favicon from DDG when domain contains a forward slash", %{plug_opts: plug_opts} do
+    expect(
+      Plausible.HTTPClient.Mock,
+      :get,
+      fn "https://icons.duckduckgo.com/ip3/site.com/subfolder.ico" ->
+        {:ok, %Finch.Response{status: 200, body: "favicon response body"}}
+      end
+    )
+
+    conn =
+      conn(:get, "/favicon/sources/site.com/subfolder")
+      |> Favicon.call(plug_opts)
+
+    assert conn.halted
+    assert conn.status == 200
+    assert conn.resp_body == "favicon response body"
+  end
+
   test "sets content-disposition and content-security-policy", %{plug_opts: plug_opts} do
     expect(
       Plausible.HTTPClient.Mock,