From 6250b22384dae437892d256b50c2e91df169b7aa Mon Sep 17 00:00:00 2001 From: Hunter Morris Date: Fri, 30 Sep 2011 18:05:02 +0100 Subject: Fix byte-by-byte Websocket handling If the websocket frame handling code in cowboy_http_websocket receives only 1 byte at a time, it fails with a badmatch in cowboy_http_websocket:websocket_data/4. This commit fixes the problem and introduces a test of the correct behaviour. --- src/cowboy_http_websocket.erl | 16 +++++++---- test/http_SUITE.erl | 66 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/cowboy_http_websocket.erl b/src/cowboy_http_websocket.erl index 1164684..e481d4b 100644 --- a/src/cowboy_http_websocket.erl +++ b/src/cowboy_http_websocket.erl @@ -232,20 +232,26 @@ websocket_data(State=#state{version=0, eop=EOP}, Req, HandlerState, %% @todo We probably should allow limiting frame length. handler_before_loop(State, Req, HandlerState, Data) end; +%% incomplete hybi data frame. +websocket_data(State=#state{version=Version}, Req, HandlerState, Data) + when Version =/= 0, byte_size(Data) =:= 1 -> + handler_before_loop(State, Req, HandlerState, Data); %% hybi data frame. %% @todo Handle Fin. websocket_data(State=#state{version=Version}, Req, HandlerState, Data) when Version =/= 0 -> << 1:1, 0:3, Opcode:4, Mask:1, PayloadLen:7, Rest/bits >> = Data, - {PayloadLen2, Rest2} = case PayloadLen of - 126 -> << L:16, R/bits >> = Rest, {L, R}; - 127 -> << 0:1, L:63, R/bits >> = Rest, {L, R}; - PayloadLen -> {PayloadLen, Rest} + {PayloadLen2, Rest2} = case {PayloadLen, Rest} of + {126, << L:16, R/bits >>} -> {L, R}; + {126, Rest} -> {undefined, Rest}; + {127, << 0:1, L:63, R/bits >>} -> {L, R}; + {127, Rest} -> {undefined, Rest}; + {PayloadLen, Rest} -> {PayloadLen, Rest} end, case {Mask, PayloadLen2} of {0, 0} -> websocket_dispatch(State, Req, HandlerState, Rest2, Opcode, <<>>); - {1, N} when N + 4 < byte_size(Rest2) -> + {1, N} when N + 4 > byte_size(Rest2); N =:= undefined -> %% @todo We probably should allow limiting frame length. handler_before_loop(State, Req, HandlerState, Data); {1, _N} -> diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl index 813aa15..2208256 100644 --- a/test/http_SUITE.erl +++ b/test/http_SUITE.erl @@ -20,7 +20,8 @@ init_per_group/2, end_per_group/2]). %% ct. -export([chunked_response/1, headers_dupe/1, headers_huge/1, keepalive_nl/1, nc_rand/1, pipeline/1, raw/1, - ws0/1, ws8/1, ws_timeout_hibernate/1]). %% http. + ws0/1, ws8/1, ws8_single_bytes/1, + ws_timeout_hibernate/1]). %% http. -export([http_200/1, http_404/1]). %% http and https. -export([http_10_hostless/1]). %% misc. @@ -33,7 +34,8 @@ groups() -> BaseTests = [http_200, http_404], [{http, [], [chunked_response, headers_dupe, headers_huge, keepalive_nl, nc_rand, pipeline, raw, - ws0, ws8, ws_timeout_hibernate] ++ BaseTests}, + ws0, ws8, ws8_single_bytes, + ws_timeout_hibernate] ++ BaseTests}, {https, [], BaseTests}, {misc, [], [http_10_hostless]}]. init_per_suite(Config) -> @@ -300,6 +302,66 @@ ws8(Config) -> {error, closed} = gen_tcp:recv(Socket, 0, 6000), ok. +ws8_single_bytes(Config) -> + {port, Port} = lists:keyfind(port, 1, Config), + {ok, Socket} = gen_tcp:connect("localhost", Port, + [binary, {active, false}, {packet, raw}]), + ok = gen_tcp:send(Socket, [ + "GET /websocket HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: Upgrade\r\n" + "Upgrade: websocket\r\n" + "Sec-WebSocket-Origin: http://localhost\r\n" + "Sec-WebSocket-Version: 8\r\n" + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n" + "\r\n"]), + {ok, Handshake} = gen_tcp:recv(Socket, 0, 6000), + {ok, {http_response, {1, 1}, 101, "Switching Protocols"}, Rest} + = erlang:decode_packet(http, Handshake, []), + [Headers, <<>>] = websocket_headers( + erlang:decode_packet(httph, Rest, []), []), + {'Connection', "Upgrade"} = lists:keyfind('Connection', 1, Headers), + {'Upgrade', "websocket"} = lists:keyfind('Upgrade', 1, Headers), + {"sec-websocket-accept", "s3pPLMBiTxaQ9kYGzzhZRbK+xOo="} + = lists:keyfind("sec-websocket-accept", 1, Headers), + ok = gen_tcp:send(Socket, << 16#81 >>), %% send one byte + ok = timer:sleep(100), %% sleep for a period + ok = gen_tcp:send(Socket, << 16#85 >>), %% send another and so on + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#37 >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#fa >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#21 >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#3d >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#7f >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#9f >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#4d >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#51 >>), + ok = timer:sleep(100), + ok = gen_tcp:send(Socket, << 16#58 >>), + {ok, << 1:1, 0:3, 1:4, 0:1, 14:7, "websocket_init" >>} + = gen_tcp:recv(Socket, 0, 6000), + {ok, << 1:1, 0:3, 1:4, 0:1, 5:7, "Hello" >>} + = gen_tcp:recv(Socket, 0, 6000), + {ok, << 1:1, 0:3, 1:4, 0:1, 16:7, "websocket_handle" >>} + = gen_tcp:recv(Socket, 0, 6000), + {ok, << 1:1, 0:3, 1:4, 0:1, 16:7, "websocket_handle" >>} + = gen_tcp:recv(Socket, 0, 6000), + {ok, << 1:1, 0:3, 1:4, 0:1, 16:7, "websocket_handle" >>} + = gen_tcp:recv(Socket, 0, 6000), + ok = gen_tcp:send(Socket, << 1:1, 0:3, 9:4, 0:8 >>), %% ping + {ok, << 1:1, 0:3, 10:4, 0:8 >>} = gen_tcp:recv(Socket, 0, 6000), %% pong + ok = gen_tcp:send(Socket, << 1:1, 0:3, 8:4, 0:8 >>), %% close + {ok, << 1:1, 0:3, 8:4, 0:8 >>} = gen_tcp:recv(Socket, 0, 6000), + {error, closed} = gen_tcp:recv(Socket, 0, 6000), + ok. + ws_timeout_hibernate(Config) -> {port, Port} = lists:keyfind(port, 1, Config), {ok, Socket} = gen_tcp:connect("localhost", Port, -- cgit v1.2.3