Browse Source

Reject invalid Connection header

LH: Small tweaks and added an HTTP/1.0 test.
Boris Pozdnyakov 2 years ago
parent
commit
e200272178
2 changed files with 43 additions and 9 deletions
  1. 21 9
      src/cowboy_http.erl
  2. 22 0
      test/rfc7230_SUITE.erl

+ 21 - 9
src/cowboy_http.erl

@@ -350,11 +350,17 @@ after_parse({request, Req=#{streamid := StreamID, method := Method,
 			TE = maps:get(<<"te">>, Headers, undefined),
 			Streams = [#stream{id=StreamID, state=StreamState,
 				method=Method, version=Version, te=TE}|Streams0],
-			State1 = case maybe_req_close(State0, Headers, Version) of
-				close -> State0#state{streams=Streams, last_streamid=StreamID, flow=Flow};
-				keepalive -> State0#state{streams=Streams, flow=Flow}
+			State1 = State0#state{streams=Streams, flow=Flow},
+			State2 = case maybe_req_close(State1, Headers, Version) of
+				close ->
+					State1#state{last_streamid=StreamID};
+				keepalive ->
+					State1;
+				bad_connection_header ->
+					error_terminate(400, State1, {connection_error, protocol_error,
+						'The Connection header is invalid. (RFC7230 6.1)'})
 			end,
-			State = set_timeout(State1, idle_timeout),
+			State = set_timeout(State2, idle_timeout),
 			parse(Buffer, commands(State, StreamID, Commands))
 	catch Class:Exception:Stacktrace ->
 		cowboy:log(cowboy_stream:make_error_log(init,
@@ -1353,17 +1359,23 @@ stream_call_terminate(StreamID, Reason, StreamState, #state{opts=Opts}) ->
 maybe_req_close(#state{opts=#{http10_keepalive := false}}, _, 'HTTP/1.0') ->
 	close;
 maybe_req_close(_, #{<<"connection">> := Conn}, 'HTTP/1.0') ->
-	Conns = cow_http_hd:parse_connection(Conn),
-	case lists:member(<<"keep-alive">>, Conns) of
-		true -> keepalive;
-		false -> close
+	try cow_http_hd:parse_connection(Conn) of
+		Conns ->
+			case lists:member(<<"keep-alive">>, Conns) of
+				true -> keepalive;
+				false -> close
+			end
+	catch _:_ ->
+		bad_connection_header
 	end;
 maybe_req_close(_, _, 'HTTP/1.0') ->
 	close;
 maybe_req_close(_, #{<<"connection">> := Conn}, 'HTTP/1.1') ->
-	case connection_hd_is_close(Conn) of
+	try connection_hd_is_close(Conn) of
 		true -> close;
 		false -> keepalive
+	catch _:_ ->
+		bad_connection_header
 	end;
 maybe_req_close(_, _, _) ->
 	keepalive.

+ 22 - 0
test/rfc7230_SUITE.erl

@@ -1512,6 +1512,28 @@ http10_no_connection_header_close(Config) ->
 	{_, <<"close">>} = lists:keyfind(<<"connection">>, 1, RespHeaders),
 	{error, closed} = raw_recv(Client, 0, 1000).
 
+connection_invalid(Config) ->
+	doc("HTTP/1.1 requests with an invalid Connection header "
+		"must be rejected with a 400 status code and the closing "
+		"of the connection. (RFC7230 6.1)"),
+	#{code := 400, client := Client} = do_raw(Config, [
+		"GET / HTTP/1.1\r\n"
+		"Host: localhost\r\n"
+		"Connection: jndi{ldap127\r\n"
+		"\r\n"]),
+	{error, closed} = raw_recv(Client, 0, 1000).
+
+http10_connection_invalid(Config) ->
+	doc("HTTP/1.0 requests with an invalid Connection header "
+		"must be rejected with a 400 status code and the closing "
+		"of the connection. (RFC7230 6.1)"),
+	#{code := 400, client := Client} = do_raw(Config, [
+		"GET / HTTP/1.0\r\n"
+		"Host: localhost\r\n"
+		"Connection: jndi{ldap127\r\n"
+		"\r\n"]),
+	{error, closed} = raw_recv(Client, 0, 1000).
+
 limit_requests_keepalive(Config) ->
 	doc("The maximum number of requests sent using a persistent connection "
 		"must be subject to configuration. The connection must be closed "