Browse Source

Fix Websocket compression

The option for enabling Websocket compression has been
renamed. Previously it was shared with HTTP compression,
now it's specific to Websocket. The new option is named
'websocket_compress'.
Loïc Hoguin 8 years ago
parent
commit
97986df276
2 changed files with 24 additions and 25 deletions
  1. 22 23
      src/cowboy_websocket.erl
  2. 2 2
      test/ws_SUITE.erl

+ 22 - 23
src/cowboy_websocket.erl

@@ -76,17 +76,18 @@
 -spec upgrade(Req, Env, module(), any(), timeout(), run | hibernate)
 -spec upgrade(Req, Env, module(), any(), timeout(), run | hibernate)
 	-> {ok, Req, Env}
 	-> {ok, Req, Env}
 	when Req::cowboy_req:req(), Env::cowboy_middleware:env().
 	when Req::cowboy_req:req(), Env::cowboy_middleware:env().
-upgrade(Req, Env, Handler, HandlerState, Timeout, Hibernate) ->
-	State = #state{handler=Handler, timeout=Timeout,
-		hibernate=Hibernate =:= hibernate},
-	%% @todo We need to fail if HTTP/2.
-	try websocket_upgrade(State, Req) of
-		{ok, State2, Req2} ->
-			websocket_handshake(State2, Req2, HandlerState, Env)
+%% @todo Immediately crash if a response has already been sent.
+%% @todo Error out if HTTP/2.
+upgrade(Req0, Env, Handler, HandlerState, Timeout, Hibernate) ->
+	try websocket_upgrade(#state{handler=Handler, timeout=Timeout,
+			hibernate=Hibernate =:= hibernate}, Req0) of
+		{ok, State, Req} ->
+			websocket_handshake(State, Req, HandlerState, Env)
 	catch _:_ ->
 	catch _:_ ->
+		%% @todo Probably log something here?
 		%% @todo Test that we can have 2 /ws 400 status code in a row on the same connection.
 		%% @todo Test that we can have 2 /ws 400 status code in a row on the same connection.
 		%% @todo Does this even work?
 		%% @todo Does this even work?
-		{ok, cowboy_req:reply(400, Req), Env}
+		{ok, cowboy_req:reply(400, Req0), Env}
 	end.
 	end.
 
 
 -spec websocket_upgrade(#state{}, Req)
 -spec websocket_upgrade(#state{}, Req)
@@ -106,46 +107,44 @@ websocket_upgrade(State, Req) ->
 
 
 -spec websocket_extensions(#state{}, Req)
 -spec websocket_extensions(#state{}, Req)
 	-> {ok, #state{}, Req} when Req::cowboy_req:req().
 	-> {ok, #state{}, Req} when Req::cowboy_req:req().
-websocket_extensions(State, Req) ->
+websocket_extensions(State, Req=#{ref := Ref}) ->
 	%% @todo We want different options for this. For example
 	%% @todo We want different options for this. For example
-	%% @todo This should probably be configurable per handler, like timeout/hibernate.
 	%% * compress everything auto
 	%% * compress everything auto
 	%% * compress only text auto
 	%% * compress only text auto
 	%% * compress only binary auto
 	%% * compress only binary auto
 	%% * compress nothing auto (but still enabled it)
 	%% * compress nothing auto (but still enabled it)
 	%% * disable compression
 	%% * disable compression
-	Compress = maps:get(websocket_compress, Req, false),
-	Req2 = Req#{websocket_compress => false},
-	case {Compress, cowboy_req:parse_header(<<"sec-websocket-extensions">>, Req2)} of
+	Compress = maps:get(websocket_compress, ranch:get_protocol_options(Ref), false),
+	case {Compress, cowboy_req:parse_header(<<"sec-websocket-extensions">>, Req)} of
 		{true, Extensions} when Extensions =/= undefined ->
 		{true, Extensions} when Extensions =/= undefined ->
-			websocket_extensions(State, Req2, Extensions, []);
+			websocket_extensions(State, Req, Extensions, []);
 		_ ->
 		_ ->
-			{ok, State, Req2}
+			{ok, State, Req}
 	end.
 	end.
 
 
 websocket_extensions(State, Req, [], []) ->
 websocket_extensions(State, Req, [], []) ->
 	{ok, State, Req};
 	{ok, State, Req};
 websocket_extensions(State, Req, [], [<<", ">>|RespHeader]) ->
 websocket_extensions(State, Req, [], [<<", ">>|RespHeader]) ->
 	{ok, State, cowboy_req:set_resp_header(<<"sec-websocket-extensions">>, lists:reverse(RespHeader), Req)};
 	{ok, State, cowboy_req:set_resp_header(<<"sec-websocket-extensions">>, lists:reverse(RespHeader), Req)};
-websocket_extensions(State=#state{extensions=Extensions}, Req, [{<<"permessage-deflate">>, Params}|Tail], RespHeader) ->
+websocket_extensions(State=#state{extensions=Extensions}, Req=#{pid := Pid},
+		[{<<"permessage-deflate">>, Params}|Tail], RespHeader) ->
 	%% @todo Make deflate options configurable.
 	%% @todo Make deflate options configurable.
 	Opts = #{level => best_compression, mem_level => 8, strategy => default},
 	Opts = #{level => best_compression, mem_level => 8, strategy => default},
-	case cow_ws:negotiate_permessage_deflate(Params, Extensions, Opts) of
+	case cow_ws:negotiate_permessage_deflate(Params, Extensions, Opts#{owner => Pid}) of
 		{ok, RespExt, Extensions2} ->
 		{ok, RespExt, Extensions2} ->
-			Req2 = Req#{websocket_compress => true},
 			websocket_extensions(State#state{extensions=Extensions2},
 			websocket_extensions(State#state{extensions=Extensions2},
-				Req2, Tail, [<<", ">>, RespExt|RespHeader]);
+				Req, Tail, [<<", ">>, RespExt|RespHeader]);
 		ignore ->
 		ignore ->
 			websocket_extensions(State, Req, Tail, RespHeader)
 			websocket_extensions(State, Req, Tail, RespHeader)
 	end;
 	end;
-websocket_extensions(State=#state{extensions=Extensions}, Req, [{<<"x-webkit-deflate-frame">>, Params}|Tail], RespHeader) ->
+websocket_extensions(State=#state{extensions=Extensions}, Req=#{pid := Pid},
+		[{<<"x-webkit-deflate-frame">>, Params}|Tail], RespHeader) ->
 	%% @todo Make deflate options configurable.
 	%% @todo Make deflate options configurable.
 	Opts = #{level => best_compression, mem_level => 8, strategy => default},
 	Opts = #{level => best_compression, mem_level => 8, strategy => default},
-	case cow_ws:negotiate_x_webkit_deflate_frame(Params, Extensions, Opts) of
+	case cow_ws:negotiate_x_webkit_deflate_frame(Params, Extensions, Opts#{owner => Pid}) of
 		{ok, RespExt, Extensions2} ->
 		{ok, RespExt, Extensions2} ->
-			Req2 = cowboy_req:set_meta(websocket_compress, true, Req),
 			websocket_extensions(State#state{extensions=Extensions2},
 			websocket_extensions(State#state{extensions=Extensions2},
-				Req2, Tail, [<<", ">>, RespExt|RespHeader]);
+				Req, Tail, [<<", ">>, RespExt|RespHeader]);
 		ignore ->
 		ignore ->
 			websocket_extensions(State, Req, Tail, RespHeader)
 			websocket_extensions(State, Req, Tail, RespHeader)
 	end;
 	end;

+ 2 - 2
test/ws_SUITE.erl

@@ -39,14 +39,14 @@ init_per_group(Name = autobahn, Config) ->
 		_ ->
 		_ ->
 			{ok, _} = cowboy:start_clear(Name, 100, [{port, 33080}], #{
 			{ok, _} = cowboy:start_clear(Name, 100, [{port, 33080}], #{
 				env => #{dispatch => init_dispatch()},
 				env => #{dispatch => init_dispatch()},
-				compress => true %% @todo Use a separate option for HTTP and Websocket compression.
+				websocket_compress => true
 			}),
 			}),
 			Config
 			Config
 	end;
 	end;
 init_per_group(Name = ws, Config) ->
 init_per_group(Name = ws, Config) ->
 	cowboy_test:init_http(Name, #{
 	cowboy_test:init_http(Name, #{
 		env => #{dispatch => init_dispatch()},
 		env => #{dispatch => init_dispatch()},
-		compress => true %% @todo Use a separate option for HTTP and Websocket compression.
+		websocket_compress => true
 	}, Config).
 	}, Config).
 
 
 end_per_group(Listener, _Config) ->
 end_per_group(Listener, _Config) ->