Merge branch 'fix/streaming-termination-errors' into 'develop'

Fix two pseudo-errors in websocket handler

Closes #2131

See merge request pleroma/pleroma!2982
This commit is contained in:
rinpatch 2020-09-10 18:40:54 +00:00
parent 0465bdbd49
commit 608017b7df
2 changed files with 22 additions and 16 deletions

View file

@ -37,12 +37,12 @@ def init(%{qs: qs} = req, state) do
else else
{:error, :bad_topic} -> {:error, :bad_topic} ->
Logger.debug("#{__MODULE__} bad topic #{inspect(req)}") Logger.debug("#{__MODULE__} bad topic #{inspect(req)}")
{:ok, req} = :cowboy_req.reply(404, req) req = :cowboy_req.reply(404, req)
{:ok, req, state} {:ok, req, state}
{:error, :unauthorized} -> {:error, :unauthorized} ->
Logger.debug("#{__MODULE__} authentication error: #{inspect(req)}") Logger.debug("#{__MODULE__} authentication error: #{inspect(req)}")
{:ok, req} = :cowboy_req.reply(401, req) req = :cowboy_req.reply(401, req)
{:ok, req, state} {:ok, req, state}
end end
end end
@ -64,7 +64,9 @@ def websocket_handle(:pong, state) do
{:ok, %{state | timer: timer()}} {:ok, %{state | timer: timer()}}
end 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 def websocket_handle(frame, state) do
Logger.error("#{__MODULE__} received frame: #{inspect(frame)}") Logger.error("#{__MODULE__} received frame: #{inspect(frame)}")
{:ok, state} {:ok, state}
@ -98,6 +100,10 @@ def websocket_info(:tick, state) do
{:reply, :ping, %{state | timer: nil, count: 0}, :hibernate} {:reply, :ping, %{state | timer: nil, count: 0}, :hibernate}
end 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 def terminate(reason, _req, state) do
Logger.debug( Logger.debug(
"#{__MODULE__} terminating websocket connection for user #{ "#{__MODULE__} terminating websocket connection for user #{

View file

@ -99,30 +99,30 @@ test "accepts valid tokens", state do
test "accepts the 'user' stream", %{token: token} = _state do test "accepts the 'user' stream", %{token: token} = _state do
assert {:ok, _} = start_socket("?stream=user&access_token=#{token.token}") assert {:ok, _} = start_socket("?stream=user&access_token=#{token.token}")
assert capture_log(fn -> capture_log(fn ->
assert {:error, {401, _}} = start_socket("?stream=user") assert {:error, {401, _}} = start_socket("?stream=user")
Process.sleep(30) Process.sleep(30)
end) =~ ":badarg" end)
end end
test "accepts the 'user:notification' stream", %{token: token} = _state do test "accepts the 'user:notification' stream", %{token: token} = _state do
assert {:ok, _} = start_socket("?stream=user:notification&access_token=#{token.token}") assert {:ok, _} = start_socket("?stream=user:notification&access_token=#{token.token}")
assert capture_log(fn -> capture_log(fn ->
assert {:error, {401, _}} = start_socket("?stream=user:notification") assert {:error, {401, _}} = start_socket("?stream=user:notification")
Process.sleep(30) Process.sleep(30)
end) =~ ":badarg" end)
end end
test "accepts valid token on Sec-WebSocket-Protocol header", %{token: token} do test "accepts valid token on Sec-WebSocket-Protocol header", %{token: token} do
assert {:ok, _} = start_socket("?stream=user", [{"Sec-WebSocket-Protocol", token.token}]) assert {:ok, _} = start_socket("?stream=user", [{"Sec-WebSocket-Protocol", token.token}])
assert capture_log(fn -> capture_log(fn ->
assert {:error, {401, _}} = assert {:error, {401, _}} =
start_socket("?stream=user", [{"Sec-WebSocket-Protocol", "I am a friend"}]) start_socket("?stream=user", [{"Sec-WebSocket-Protocol", "I am a friend"}])
Process.sleep(30) Process.sleep(30)
end) =~ ":badarg" end)
end end
end end
end end