From bd63cf3a6dde652d771bd013a4f5dc5fb0901ef8 Mon Sep 17 00:00:00 2001 From: Mat Trudel Date: Thu, 2 Nov 2023 02:50:38 -0400 Subject: [PATCH] Upgrade to latest websock_adapter, handle upgrade failures & report 400 (#5621) --- lib/phoenix/transports/websocket.ex | 10 +++++++--- mix.lock | 4 ++-- test/phoenix/integration/websocket_socket_test.exs | 10 +++++++++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/phoenix/transports/websocket.ex b/lib/phoenix/transports/websocket.ex index 16be35bb54..f04cc4350d 100644 --- a/lib/phoenix/transports/websocket.ex +++ b/lib/phoenix/transports/websocket.ex @@ -57,9 +57,13 @@ defmodule Phoenix.Transports.WebSocket do case handler.connect(config) do {:ok, arg} -> - conn - |> WebSockAdapter.upgrade(handler, arg, opts) - |> halt() + try do + conn + |> WebSockAdapter.upgrade(handler, arg, opts) + |> halt() + rescue + e in WebSockAdapter.UpgradeError -> send_resp(conn, 400, e.message) + end :error -> send_resp(conn, 403, "") diff --git a/mix.lock b/mix.lock index 0dbfe8a02c..77ecea94ff 100644 --- a/mix.lock +++ b/mix.lock @@ -36,6 +36,6 @@ "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"}, "telemetry_metrics": {:hex, :telemetry_metrics, "0.6.1", "315d9163a1d4660aedc3fee73f33f1d355dcc76c5c3ab3d59e76e3edf80eef1f", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7be9e0871c41732c233be71e4be11b96e56177bf15dde64a8ac9ce72ac9834c6"}, "telemetry_poller": {:hex, :telemetry_poller, "1.0.0", "db91bb424e07f2bb6e73926fcafbfcbcb295f0193e0a00e825e589a0a47e8453", [:rebar3], [{:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b3a24eafd66c3f42da30fc3ca7dda1e9d546c12250a2d60d7b81d264fbec4f6e"}, - "websock": {:hex, :websock, "0.5.2", "b3c08511d8d79ed2c2f589ff430bd1fe799bb389686dafce86d28801783d8351", [:mix], [], "hexpm", "925f5de22fca6813dfa980fb62fd542ec43a2d1a1f83d2caec907483fe66ff05"}, - "websock_adapter": {:hex, :websock_adapter, "0.5.3", "4908718e42e4a548fc20e00e70848620a92f11f7a6add8cf0886c4232267498d", [:mix], [{:bandit, ">= 0.6.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "cbe5b814c1f86b6ea002b52dd99f345aeecf1a1a6964e209d208fb404d930d3d"}, + "websock": {:hex, :websock, "0.5.3", "2f69a6ebe810328555b6fe5c831a851f485e303a7c8ce6c5f675abeb20ebdadc", [:mix], [], "hexpm", "6105453d7fac22c712ad66fab1d45abdf049868f253cf719b625151460b8b453"}, + "websock_adapter": {:hex, :websock_adapter, "0.5.5", "9dfeee8269b27e958a65b3e235b7e447769f66b5b5925385f5a569269164a210", [:mix], [{:bandit, ">= 0.6.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "4b977ba4a01918acbf77045ff88de7f6972c2a009213c515a445c48f224ffce9"}, } diff --git a/test/phoenix/integration/websocket_socket_test.exs b/test/phoenix/integration/websocket_socket_test.exs index 5bc4c5599d..aa249f4138 100644 --- a/test/phoenix/integration/websocket_socket_test.exs +++ b/test/phoenix/integration/websocket_socket_test.exs @@ -4,7 +4,7 @@ defmodule Phoenix.Integration.WebSocketTest do use ExUnit.Case, async: true import ExUnit.CaptureLog - alias Phoenix.Integration.WebsocketClient + alias Phoenix.Integration.{HTTPClient, WebsocketClient} alias __MODULE__.Endpoint @moduletag :capture_log @@ -104,6 +104,14 @@ defmodule Phoenix.Integration.WebSocketTest do :ok end + test "handles invalid upgrade requests" do + capture_log(fn -> + path = String.replace_prefix(@path, "ws", "http") + assert {:ok, %{body: body, status: 400}} = HTTPClient.request(:get, path, %{}) + assert body =~ "'connection' header must contain 'upgrade'" + end) + end + test "refuses unallowed origins" do capture_log(fn -> headers = [{"origin", "https://example.com"}]