From cb06e98da27994ac8034f3ba387b6eeaf8a2c48f Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 10 Sep 2020 13:47:53 +0300 Subject: [PATCH 1/4] websocket handler: Do not log client ping frames as errors --- lib/pleroma/web/mastodon_api/websocket_handler.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/mastodon_api/websocket_handler.ex b/lib/pleroma/web/mastodon_api/websocket_handler.ex index 94e4595d8..e6010bb4a 100644 --- a/lib/pleroma/web/mastodon_api/websocket_handler.ex +++ b/lib/pleroma/web/mastodon_api/websocket_handler.ex @@ -64,7 +64,9 @@ def websocket_handle(:pong, state) do {:ok, %{state | timer: timer()}} end - # We never receive messages. + # We only receive pings for now + def websocket_handle(:ping, state), do: {:ok, state} + def websocket_handle(frame, state) do Logger.error("#{__MODULE__} received frame: #{inspect(frame)}") {:ok, state} From e16e8f98169f822416c18778abfa8495a486c8f2 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 10 Sep 2020 13:48:24 +0300 Subject: [PATCH 2/4] Websocket handler: do not raise if handler is terminated before switching protocols Closes #2131 --- lib/pleroma/web/mastodon_api/websocket_handler.ex | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/pleroma/web/mastodon_api/websocket_handler.ex b/lib/pleroma/web/mastodon_api/websocket_handler.ex index e6010bb4a..5090d9622 100644 --- a/lib/pleroma/web/mastodon_api/websocket_handler.ex +++ b/lib/pleroma/web/mastodon_api/websocket_handler.ex @@ -100,6 +100,10 @@ def websocket_info(:tick, state) do {:reply, :ping, %{state | timer: nil, count: 0}, :hibernate} end + # State can be `[]` only in case we terminate before switching to websocket, + # we already log errors for these cases in `init/1`, so just do nothing here + def terminate(_reason, _req, []), do: :ok + def terminate(reason, _req, state) do Logger.debug( "#{__MODULE__} terminating websocket connection for user #{ From 01fa68fe4542286519e3520793c6b59103b050ff Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 10 Sep 2020 21:26:52 +0300 Subject: [PATCH 3/4] Websocket handler: fix never matching code on failed auth `:cowboy_req.reply` does not return tuples since 2.0, see https://ninenines.eu/docs/en/cowboy/2.4/manual/cowboy_req.reply/ --- lib/pleroma/web/mastodon_api/websocket_handler.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/websocket_handler.ex b/lib/pleroma/web/mastodon_api/websocket_handler.ex index 5090d9622..cf923ded8 100644 --- a/lib/pleroma/web/mastodon_api/websocket_handler.ex +++ b/lib/pleroma/web/mastodon_api/websocket_handler.ex @@ -37,12 +37,12 @@ def init(%{qs: qs} = req, state) do else {:error, :bad_topic} -> Logger.debug("#{__MODULE__} bad topic #{inspect(req)}") - {:ok, req} = :cowboy_req.reply(404, req) + req = :cowboy_req.reply(404, req) {:ok, req, state} {:error, :unauthorized} -> Logger.debug("#{__MODULE__} authentication error: #{inspect(req)}") - {:ok, req} = :cowboy_req.reply(401, req) + req = :cowboy_req.reply(401, req) {:ok, req, state} end end From 275602daa7c4a01dffb83759012554da2d9335fe Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 10 Sep 2020 21:28:47 +0300 Subject: [PATCH 4/4] Streaming integration tests: remove unexpected error assumption For some reason instead of fixing unexpected errors, we made tests assert they indeed trigger... Now that the errors are fixed these were failing --- test/integration/mastodon_websocket_test.exs | 26 ++++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/integration/mastodon_websocket_test.exs b/test/integration/mastodon_websocket_test.exs index ea17e9feb..76fbc8bda 100644 --- a/test/integration/mastodon_websocket_test.exs +++ b/test/integration/mastodon_websocket_test.exs @@ -99,30 +99,30 @@ test "accepts valid tokens", state do test "accepts the 'user' stream", %{token: token} = _state do assert {:ok, _} = start_socket("?stream=user&access_token=#{token.token}") - assert capture_log(fn -> - assert {:error, {401, _}} = start_socket("?stream=user") - Process.sleep(30) - end) =~ ":badarg" + capture_log(fn -> + assert {:error, {401, _}} = start_socket("?stream=user") + Process.sleep(30) + end) end test "accepts the 'user:notification' stream", %{token: token} = _state do assert {:ok, _} = start_socket("?stream=user:notification&access_token=#{token.token}") - assert capture_log(fn -> - assert {:error, {401, _}} = start_socket("?stream=user:notification") - Process.sleep(30) - end) =~ ":badarg" + capture_log(fn -> + assert {:error, {401, _}} = start_socket("?stream=user:notification") + Process.sleep(30) + end) end test "accepts valid token on Sec-WebSocket-Protocol header", %{token: token} do assert {:ok, _} = start_socket("?stream=user", [{"Sec-WebSocket-Protocol", token.token}]) - assert capture_log(fn -> - assert {:error, {401, _}} = - start_socket("?stream=user", [{"Sec-WebSocket-Protocol", "I am a friend"}]) + capture_log(fn -> + assert {:error, {401, _}} = + start_socket("?stream=user", [{"Sec-WebSocket-Protocol", "I am a friend"}]) - Process.sleep(30) - end) =~ ":badarg" + Process.sleep(30) + end) end end end