From 7aa0118da123b37f8256de68e594792fa231e975 Mon Sep 17 00:00:00 2001 From: Alexander Sieg Date: Fri, 30 May 2025 16:22:54 +0200 Subject: [PATCH 1/3] fix: only response with html if client prefers media type The previous implemtantion has quite basic and didn't account for the q parameter a client can add to an entry in the accept header. This would fail with HTTPie, which send requests with "Accept: application/json, text/*;q=0.5". While one can argument that responding with HTML is in spec, it isn't a good DX. --- lib/plug/debugger.ex | 21 +++++++++++++++++++-- test/plug/debugger_test.exs | 27 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 4f69d7bf..16b8c95b 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -299,8 +299,25 @@ defmodule Plug.Debugger do defp accepts_html?(_accept_header = []), do: false - defp accepts_html?(_accept_header = [header | _]), - do: String.contains?(header, ["*/*", "text/*", "text/html"]) + defp accepts_html?(_accept_header = [header | _]) do + parse_accect_header(header) + |> Enum.any?(&accept_html_media_type?/1) + end + + defp parse_accect_header(accept_header) do + accept_header + |> String.split(",", trim: true) + |> Enum.map(&Plug.Conn.Utils.media_type/1) + |> Enum.filter(&match?({:ok, _, _, _}, &1)) + |> Enum.map(fn {:ok, type, subtype, params} -> + {type, subtype, Map.get(params, "q", 1)} + end) + end + + defp accept_html_media_type?({"text", "html", 1}), do: true + defp accept_html_media_type?({"text", "*", 1}), do: true + defp accept_html_media_type?({"*", "*", 1}), do: true + defp accept_html_media_type?({_type, _subtype, _q}), do: false defp maybe_fetch_session(conn) do if conn.private[:plug_session_fetch] do diff --git a/test/plug/debugger_test.exs b/test/plug/debugger_test.exs index b76388d0..c1a868a9 100644 --- a/test/plug/debugger_test.exs +++ b/test/plug/debugger_test.exs @@ -628,4 +628,31 @@ defmodule Plug.DebuggerTest do String.to_existing_atom("not_existing_atom_for_test_does_not_create_new_atom") end end + + test "render markdown with text/html;q=0.5" do + conn = + conn(:get, "/") + |> put_req_header("accept", "text/html;q=0.5") + |> render([], fn -> raise "oops" end) + + assert get_resp_header(conn, "content-type") == ["text/markdown; charset=utf-8"] + end + + test "render markdown with text/*;q=0.5" do + conn = + conn(:get, "/") + |> put_req_header("accept", "text/*;q=0.5") + |> render([], fn -> raise "oops" end) + + assert get_resp_header(conn, "content-type") == ["text/markdown; charset=utf-8"] + end + + test "render markdown with */*;q=0.5" do + conn = + conn(:get, "/") + |> put_req_header("accept", "*/*;q=0.5") + |> render([], fn -> raise "oops" end) + + assert get_resp_header(conn, "content-type") == ["text/markdown; charset=utf-8"] + end end From 19ddfcda18149e07c4715fcec7f8be25e61ce74b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 30 May 2025 22:35:29 +0200 Subject: [PATCH 2/3] Update lib/plug/debugger.ex --- lib/plug/debugger.ex | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 16b8c95b..3a53e432 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -300,24 +300,22 @@ defmodule Plug.Debugger do defp accepts_html?(_accept_header = []), do: false defp accepts_html?(_accept_header = [header | _]) do - parse_accect_header(header) - |> Enum.any?(&accept_html_media_type?/1) - end - - defp parse_accect_header(accept_header) do accept_header |> String.split(",", trim: true) |> Enum.map(&Plug.Conn.Utils.media_type/1) - |> Enum.filter(&match?({:ok, _, _, _}, &1)) - |> Enum.map(fn {:ok, type, subtype, params} -> - {type, subtype, Map.get(params, "q", 1)} + |> Enum.any?(fn + {:ok, type, subtype, params} -> + accept_html_media_type?(type, subtype, Map.get(params, "q", 1)) + + _ -> + false end) end - defp accept_html_media_type?({"text", "html", 1}), do: true - defp accept_html_media_type?({"text", "*", 1}), do: true - defp accept_html_media_type?({"*", "*", 1}), do: true - defp accept_html_media_type?({_type, _subtype, _q}), do: false + defp accept_html_media_type?("text", "html", 1), do: true + defp accept_html_media_type?("text", "*", 1), do: true + defp accept_html_media_type?("*", "*", 1), do: true + defp accept_html_media_type?(_type, _subtype, _q), do: false defp maybe_fetch_session(conn) do if conn.private[:plug_session_fetch] do From 9320fa22d202b03fb5743d1d01edee304f8ca64f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 30 May 2025 22:37:41 +0200 Subject: [PATCH 3/3] Update lib/plug/debugger.ex --- lib/plug/debugger.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 3a53e432..a05d37f9 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -300,7 +300,7 @@ defmodule Plug.Debugger do defp accepts_html?(_accept_header = []), do: false defp accepts_html?(_accept_header = [header | _]) do - accept_header + header |> String.split(",", trim: true) |> Enum.map(&Plug.Conn.Utils.media_type/1) |> Enum.any?(fn