Browse Source

Fix crashes when creating the request

We now obtain the peer address before creating the Req object.
If an error occurs, then something went wrong, we close the connection
nicely directly.
Loïc Hoguin 12 years ago
parent
commit
ae401f7460
2 changed files with 19 additions and 11 deletions
  1. 12 6
      src/cowboy_protocol.erl
  2. 7 5
      src/cowboy_req.erl

+ 12 - 6
src/cowboy_protocol.erl

@@ -463,10 +463,16 @@ request(Buffer, State=#state{socket=Socket, transport=Transport,
 		req_keepalive=ReqKeepalive, max_keepalive=MaxKeepalive,
 		compress=Compress, onresponse=OnResponse},
 		Method, Path, Query, Fragment, Version, Headers, Host, Port) ->
-	Req = cowboy_req:new(Socket, Transport, Method, Path, Query, Fragment,
-		Version, Headers, Host, Port, Buffer, ReqKeepalive < MaxKeepalive,
-		Compress, OnResponse),
-	onrequest(Req, State).
+	case Transport:peername(Socket) of
+		{ok, Peer} ->
+			Req = cowboy_req:new(Socket, Transport, Peer, Method, Path,
+				Query, Fragment, Version, Headers, Host, Port, Buffer,
+				ReqKeepalive < MaxKeepalive, Compress, OnResponse),
+			onrequest(Req, State);
+		{error, _} ->
+			%% Couldn't read the peer address; connection is gone.
+			terminate(State)
+	end.
 
 %% Call the global onrequest callback. The callback can send a reply,
 %% in which case we consider the request handled and move on to the next
@@ -562,8 +568,8 @@ error_terminate(Code, State=#state{socket=Socket, transport=Transport,
 		{cowboy_req, resp_sent} -> ok
 	after 0 ->
 		_ = cowboy_req:reply(Code, cowboy_req:new(Socket, Transport,
-			<<"GET">>, <<>>, <<>>, <<>>, {1, 1}, [], <<>>, undefined,
-			<<>>, false, Compress, OnResponse)),
+			undefined, <<"GET">>, <<>>, <<>>, <<>>, {1, 1}, [], <<>>,
+			undefined, <<>>, false, Compress, OnResponse)),
 		ok
 	end,
 	terminate(State).

+ 7 - 5
src/cowboy_req.erl

@@ -42,7 +42,7 @@
 -module(cowboy_req).
 
 %% Request API.
--export([new/14]).
+-export([new/15]).
 -export([method/1]).
 -export([version/1]).
 -export([peer/1]).
@@ -179,15 +179,16 @@
 %%
 %% Since we always need to parse the Connection header, we do it
 %% in an optimized way and add the parsed value to p_headers' cache.
--spec new(inet:socket(), module(), binary(), binary(), binary(), binary(),
+-spec new(inet:socket(), module(),
+	undefined | {inet:ip_address(), inet:port_number()},
+	binary(), binary(), binary(), binary(),
 	cowboy_http:version(), cowboy_http:headers(), binary(),
 	inet:port_number() | undefined, binary(), boolean(), boolean(),
 	undefined | cowboy_protocol:onresponse_fun())
 	-> req().
-new(Socket, Transport, Method, Path, Query, Fragment,
+new(Socket, Transport, Peer, Method, Path, Query, Fragment,
 		Version, Headers, Host, Port, Buffer, CanKeepalive,
 		Compress, OnResponse) ->
-	{ok, Peer} = Transport:peername(Socket),
 	Req = #http_req{socket=Socket, transport=Transport, pid=self(), peer=Peer,
 		method=Method, path=Path, qs=Query, fragment=Fragment, version=Version,
 		headers=Headers, host=Host, port=Port, buffer=Buffer,
@@ -219,7 +220,8 @@ version(Req) ->
 
 %% @doc Return the peer address and port number of the remote host.
 -spec peer(Req)
-	-> {{inet:ip_address(), inet:port_number()}, Req} when Req::req().
+	-> {undefined | {inet:ip_address(), inet:port_number()}, Req}
+	when Req::req().
 peer(Req) ->
 	{Req#http_req.peer, Req}.