Browse files

Merge pull request #23 from basho/bugfix/spurious_error_handling

Port the spurious 400 error fix forward to master
  • Loading branch information...
2 parents ade2a9b + 5a3d511 commit 4d3882181d0e0e507a05115782a2b091a1db2be4 @nickelization nickelization committed Feb 23, 2016
Showing with 63 additions and 2 deletions.
  1. +22 −2 src/mochiweb_http.erl
  2. +41 −0 test/mochiweb_http_tests.erl
View
24 src/mochiweb_http.erl
@@ -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)
@@ -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)
View
41 test/mochiweb_http_tests.erl
@@ -13,9 +13,16 @@ has_acceptor_bug_test_() ->
fun mochiweb_http:stop/1,
fun has_acceptor_bug_tests/1}.
+unexpected_msg_test_() ->
+ {setup,
+ fun start_server/0,
+ fun mochiweb_http:stop/1,
+ fun unexpected_msg_in_hdr_tests/1}.
+
start_server() ->
application:start(inets),
{ok, Pid} = mochiweb_http:start_link([{port, 0},
+ {acceptor_pool_size,1},
{loop, fun responder/1}]),
Pid.
@@ -43,3 +50,37 @@ has_bug(Port, Len) ->
{ok, {{"HTTP/1.1", 400, "Bad Request"}, _, []}} ->
false
end.
+
+-define(IN_METHOD, 3). % The space after the get.
+-define(IN_HEADER, 20). % The dash after user
+get_req() ->
+ <<"GET / HTTP/1.1\r\n"
+ "User-Agent: mochiweb_http_tests/0.0\r\n"
+ "Accept: */*\r\n\r\n">>.
+
+unexpected_msg(Server, MsgAt, Msg) ->
+ %% Set up with a single acceptor, dig out the pid - sensitive to change in mochiweb_socket_server
+ %% state record.
+ [Acceptor] = sets:to_list(element(14, sys:get_state(Server))),
+ Port = mochiweb_socket_server:get(Server, port),
+ <<Before:MsgAt/binary, After/binary>> = get_req(),
+ {ok, S} = gen_tcp:connect({127,0,0,1},Port,[binary,{active,false}]),
+ ok = gen_tcp:send(S, Before),
+ Acceptor ! Msg,
+ gen_tcp:send(S, After),
+ gen_tcp:recv(S, 0, 5000).
+
+
+unexpected_msg_in_hdr_tests(Server) ->
+ [{"should ignore a message in the middle of the request line",
+ ?_assertMatch({ok, <<"HTTP/1.1 200 OK", _Rest/binary>>},
+ unexpected_msg(Server, ?IN_METHOD, unexpected_msg_in_your_method))},
+ {"should ignore a message in the middle of a header",
+ ?_assertMatch({ok, <<"HTTP/1.1 200 OK", _Rest/binary>>},
+ unexpected_msg(Server, ?IN_HEADER, unexpected_msg_in_your_header))},
+ {"should close on a TCP error on the request line",
+ ?_assertMatch({error, closed},
+ unexpected_msg(Server, ?IN_METHOD, {tcp_error, your_port_you_dont_match_on, something_terrible}))},
+ {"should close on a TCP error in a header",
+ ?_assertMatch({error, closed},
+ unexpected_msg(Server, ?IN_HEADER, {tcp_error, your_port_you_dont_match_on, something_terrible}))}].

0 comments on commit 4d38821

Please sign in to comment.