Skip to content

Commit

Permalink
Fix handling for spurious msgs and protocol errors
Browse files Browse the repository at this point in the history
This fix prevents us from sending back a 400 error in response to
unexpected messages that may come in. The intent in the original
catch-all clause seems to be to match on TCP or SSL errors, but if we
get some other unexpected message coming in from the application layer,
then that will also trigger the catch-all. This can have especially bad
consequences if keepalives are enabled, as we may end up sending both a
valid response AND a 400 on the same connection for a single request.

At the same time, not all TCP/SSL errors necessarily warrant a 400
response, so most errors will now simply trigger us to close the socket.
The exception is emsgsize errors, since this typically comes from the
packet decoder, suggesting that there is indeed actually a problem with
the client request itself. This is also the only such case that is
explicitly tested in the eunit test code, so that behavior is maintained
with no need to fix any tests.
  • Loading branch information
nickelization committed Feb 23, 2016
1 parent ade2a9b commit fe6c09b
Showing 1 changed file with 22 additions and 2 deletions.
24 changes: 22 additions & 2 deletions src/mochiweb_http.erl
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,17 @@ request(Socket, Body) ->
{ssl_closed, _} ->
mochiweb_socket:close(Socket),
exit(normal);
Msg = {ProtocolErr, _Socket, emsgsize} when
ProtocolErr =:= tcp_error ; ProtocolErr =:= ssl_error ->
handle_invalid_msg_request(Msg, Socket);
{ProtocolErr, _Socket, _Reason} when
ProtocolErr =:= tcp_error ; ProtocolErr =:= ssl_error ->
mochiweb_socket:close(Socket),
exit(normal);
Other ->
handle_invalid_msg_request(Other, Socket)
error_logger:warning_msg("Got unexpected (leftover) message: ~w (to pid=~w)~n",
[Other, self()]),
request(Socket, Body)
after ?REQUEST_RECV_TIMEOUT ->
mochiweb_socket:close(Socket),
exit(normal)
Expand Down Expand Up @@ -101,8 +110,19 @@ headers(Socket, Request, Headers, Body, HeaderCount) ->
{tcp_closed, _} ->
mochiweb_socket:close(Socket),
exit(normal);
Msg = {ProtocolErr, _Socket, emsgsize} when
ProtocolErr =:= tcp_error ; ProtocolErr =:= ssl_error ->
handle_invalid_msg_request(Msg, Socket);
Msg = {ProtocolErr, _Socket, _Reason} when
ProtocolErr =:= tcp_error ; ProtocolErr =:= ssl_error ->
error_logger:warning_msg("Got unexpected TCP error message: ~w (to pid=~w)~n",
[Msg, self()]),
mochiweb_socket:close(Socket),
exit(normal);
Other ->
handle_invalid_msg_request(Other, Socket, Request, Headers)
error_logger:warning_msg("Got unexpected (leftover) message: ~w (to pid=~w)~n",
[Other, self()]),
headers(Socket, Request, Headers, Body, HeaderCount)
after ?HEADERS_RECV_TIMEOUT ->
mochiweb_socket:close(Socket),
exit(normal)
Expand Down

0 comments on commit fe6c09b

Please sign in to comment.