bugfix: client: bound the handshake response header read

README.markdown

@@ -364,6 +364,9 @@ An optional options table can be specified. The following options are as follows
 * `max_send_len`
 
     Specifies the maximal length of payload allowed when sending WebSocket frames. Defaults to the value of `max_payload_len`.
+* `max_header_len`
+
+    Specifies the maximal size, in bytes, of the response header block read during the handshake. A server that sends more than this, or that never terminates the header block at all, is refused instead of being buffered. Defaults to `8192`. Set it to `0` to read without a limit.
 * `send_unmasked`
 
     Specifies whether to send out an unmasked WebSocket frames. When it is `true`, unmasked frames are always sent. Default to `false`. RFC 6455 requires, however, that the client MUST send masked frames to the server, so never set this option to `true` unless you know what you are doing.

lib/resty/websocket/client.lua

@@ -48,18 +48,56 @@ _M._VERSION = '0.13'
 local mt = { __index = _M }
 
 
+-- default cap on the size of a handshake response header block, in bytes;
+-- mirrors nginx's own 8k header buffer
+local DEFAULT_MAX_HEADER_LEN = 8192
+
+
+-- reads the response header block terminated by CRLFCRLF, refusing to buffer
+-- more than max_header_len bytes. max_header_len == 0 means no limit.
+local function recv_header(sock, max_header_len)
+    local reader = sock:receiveuntil("\r\n\r\n")
+
+    if max_header_len == 0 then
+        return reader()
+    end
+
+    -- ask for one byte past the limit: a header block that fits comes back
+    -- whole, while an oversized one (or a peer that never terminates the
+    -- block at all) comes back at the limit instead of being buffered forever
+    local header, err, partial = reader(max_header_len + 1)
+    if not header then
+        return nil, err, partial
+    end
+
+    if #header > max_header_len then
+        return nil, "response headers too large (limit: "
+                    .. max_header_len .. " bytes)"
+    end
+
+    return header
+end
+
+
 function _M.new(self, opts)
     local sock, err = tcp()
     if not sock then
         return nil, err
     end
 
     local max_payload_len, send_unmasked, timeout
-    local max_recv_len, max_send_len
+    local max_recv_len, max_send_len, max_header_len
     if opts then
         max_payload_len = opts.max_payload_len
         max_recv_len = opts.max_recv_len
         max_send_len = opts.max_send_len
+        max_header_len = opts.max_header_len
+
+        if max_header_len ~= nil
+           and (type(max_header_len) ~= "number" or max_header_len < 0)
+        then
+            return nil, "max_header_len must be a non-negative number"
+        end
 
         send_unmasked = opts.send_unmasked
         timeout = opts.timeout
@@ -72,11 +110,13 @@ function _M.new(self, opts)
     max_payload_len = max_payload_len or 65535
     max_recv_len = max_recv_len or max_payload_len
     max_send_len = max_send_len or max_payload_len
+    max_header_len = max_header_len or DEFAULT_MAX_HEADER_LEN
 
     return setmetatable({
         sock = sock,
         max_recv_len = max_recv_len,
         max_send_len = max_send_len,
+        max_header_len = max_header_len,
         send_unmasked = send_unmasked,
     }, mt)
 end
@@ -288,9 +328,7 @@ function _M.connect(self, uri, opts)
                 return nil, "failed to send the handshake request: " .. err
             end
 
-            local header_reader = sock:receiveuntil("\r\n\r\n")
-            -- FIXME: check for too big response headers
-            local header, err, _ = header_reader()
+            local header, err = recv_header(sock, self.max_header_len)
             if not header then
                 return nil, "failed to receive response header: " .. err
             end
@@ -360,9 +398,8 @@ function _M.connect(self, uri, opts)
         return nil, "failed to send the handshake request: " .. err
     end
 
-    local header_reader = sock:receiveuntil("\r\n\r\n")
-    -- FIXME: check for too big response headers
-    local header, err, partial = header_reader()
+    local header
+    header, err = recv_header(sock, self.max_header_len)
     if not header then
         return nil, "failed to receive response header: " .. err
     end

t/max_header_len.t

@@ -0,0 +1,224 @@
+# vim:set ft= ts=4 sw=4 et:
+
+use Test::Nginx::Socket::Lua;
+use Cwd qw(cwd);
+
+repeat_each(2);
+
+plan tests => repeat_each() * (3 * blocks());
+
+my $pwd = cwd();
+
+our $HttpConfig = qq{
+    lua_package_path "$pwd/lib/?.lua;;";
+    lua_package_cpath "/usr/local/openresty-debug/lualib/?.so;/usr/local/openresty/lualib/?.so;;";
+};
+
+# a valid handshake response for the fixed key used below, padded past the
+# 8192 byte default limit
+our $BigReply = "HTTP/1.1 101 Switching Protocols\r
+Upgrade: websocket\r
+Connection: Upgrade\r
+Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r
+X-Padding: " . ("a" x 9000) . "\r
+\r
+";
+
+our $SmallReply = "HTTP/1.1 101 Switching Protocols\r
+Upgrade: websocket\r
+Connection: Upgrade\r
+Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r
+\r
+";
+
+# never terminated by CRLFCRLF: the client must stop reading at the limit
+our $UnterminatedReply = "HTTP/1.1 101 Switching Protocols\r
+X-Padding: " . ("a" x 9000);
+
+no_long_string();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: oversized response headers are refused by default
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local client = require "resty.websocket.client"
+            local wb = client:new()
+
+            local uri = "ws://127.0.0.1:7986/"
+            local ok, err = wb:connect(uri,
+                                       { key = "dGhlIHNhbXBsZSBub25jZQ==" })
+            if not ok then
+                ngx.say("failed to connect: ", err)
+                return
+            end
+
+            ngx.say("connected")
+        }
+    }
+--- request
+GET /t
+--- tcp_listen: 7986
+--- tcp_reply eval: $::BigReply
+--- response_body
+failed to connect: failed to receive response header: response headers too large (limit: 8192 bytes)
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: max_header_len = 0 restores the unbounded read
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local client = require "resty.websocket.client"
+            local wb = client:new{ max_header_len = 0 }
+
+            local uri = "ws://127.0.0.1:7986/"
+            local ok, err = wb:connect(uri,
+                                       { key = "dGhlIHNhbXBsZSBub25jZQ==" })
+            if not ok then
+                ngx.say("failed to connect: ", err)
+                return
+            end
+
+            ngx.say("connected")
+        }
+    }
+--- request
+GET /t
+--- tcp_listen: 7986
+--- tcp_reply eval: $::BigReply
+--- response_body
+connected
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: a smaller limit refuses a response that would otherwise fit
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local client = require "resty.websocket.client"
+            local wb = client:new{ max_header_len = 32 }
+
+            local uri = "ws://127.0.0.1:7986/"
+            local ok, err = wb:connect(uri,
+                                       { key = "dGhlIHNhbXBsZSBub25jZQ==" })
+            if not ok then
+                ngx.say("failed to connect: ", err)
+                return
+            end
+
+            ngx.say("connected")
+        }
+    }
+--- request
+GET /t
+--- tcp_listen: 7986
+--- tcp_reply eval: $::SmallReply
+--- response_body
+failed to connect: failed to receive response header: response headers too large (limit: 32 bytes)
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: a header block that is never terminated is refused
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local client = require "resty.websocket.client"
+            local wb = client:new()
+
+            local uri = "ws://127.0.0.1:7986/"
+            local ok, err = wb:connect(uri,
+                                       { key = "dGhlIHNhbXBsZSBub25jZQ==" })
+            if not ok then
+                ngx.say("failed to connect: ", err)
+                return
+            end
+
+            ngx.say("connected")
+        }
+    }
+--- request
+GET /t
+--- tcp_listen: 7986
+--- tcp_reply eval: $::UnterminatedReply
+--- response_body
+failed to connect: failed to receive response header: response headers too large (limit: 8192 bytes)
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: new() rejects a bad max_header_len
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local client = require "resty.websocket.client"
+
+            local wb, err = client:new{ max_header_len = -1 }
+            ngx.say("negative: ", wb, ", ", err)
+
+            wb, err = client:new{ max_header_len = "8192" }
+            ngx.say("string: ", wb, ", ", err)
+        }
+    }
+--- request
+GET /t
+--- response_body
+negative: nil, max_header_len must be a non-negative number
+string: nil, max_header_len must be a non-negative number
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: a real handshake still succeeds under the default limit
+--- http_config eval: $::HttpConfig
+--- config
+    location = /ws {
+        content_by_lua_block {
+            local server = require "resty.websocket.server"
+            local wb, err = server:new()
+            if not wb then
+                ngx.log(ngx.ERR, "failed to new websocket: ", err)
+                return ngx.exit(444)
+            end
+            wb:recv_frame()
+        }
+    }
+
+    location = /t {
+        content_by_lua_block {
+            local client = require "resty.websocket.client"
+            local wb = client:new()
+
+            local uri = "ws://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+            local ok, err = wb:connect(uri)
+            if not ok then
+                ngx.say("failed to connect: ", err)
+                return
+            end
+
+            ngx.say("connected")
+            wb:close()
+        }
+    }
+--- request
+GET /t
+--- response_body
+connected
+--- no_error_log
+[error]