Merge branch 'write_body_from_file-options'

Author: daurnimator

NEWS

@@ -6,6 +6,7 @@ UNRELEASED
   - Add http.tls.old_cipher_list (#112)
   - Add http.cookie module (#117)
   - Improvements to http.hsts module (#119)
+  - Add `options` argument form to `stream:write_body_from_file()` (#125)
 
 
 0.2 - 2017-05-28

doc/interfaces/stream.md

@@ -89,7 +89,12 @@ Writes the string `chunk` to the stream. If `end_stream` is true, the body will
 Writes the string `str` to the stream and ends the stream. On error, returns `nil`, an error message and an error number.
 
 
-### `stream:write_body_from_file(file, timeout)` <!-- --> {#stream:write_body_from_file}
+### `stream:write_body_from_file(options|file, timeout)` <!-- --> {#stream:write_body_from_file}
+
+  - `options` is a table containing:
+	- `.file` (file)
+	- `.count` (positive integer): number of bytes of `file` to write  
+	  defaults to infinity (the whole file will be written)
 
 Writes the contents of file `file` to the stream and ends the stream. `file` will not be automatically seeked, so ensure it is at the correct offset before calling. On error, returns `nil`, an error message and an error number.
 

doc/modules/http.h1_stream.md

@@ -94,9 +94,9 @@ See [`stream:write_chunk(chunk, end_stream, timeout)`](#stream:write_chunk)
 See [`stream:write_body_from_string(str, timeout)`](#stream:write_body_from_string)
 
 
-### `h1_stream:write_body_from_file(file, timeout)` <!-- --> {#http.h1_stream:write_body_from_file}
+### `h1_stream:write_body_from_file(options|file, timeout)` <!-- --> {#http.h1_stream:write_body_from_file}
 
-See [`stream:write_body_from_file(file, timeout)`](#stream:write_body_from_file)
+See [`stream:write_body_from_file(options|file, timeout)`](#stream:write_body_from_file)
 
 
 ### `h1_stream:shutdown()` <!-- --> {#http.h1_stream:shutdown}

doc/modules/http.h2_stream.md

@@ -87,9 +87,9 @@ See [`stream:write_chunk(chunk, end_stream, timeout)`](#stream:write_chunk)
 See [`stream:write_body_from_string(str, timeout)`](#stream:write_body_from_string)
 
 
-### `h2_stream:write_body_from_file(file, timeout)` <!-- --> {#http.h2_stream:write_body_from_file}
+### `h2_stream:write_body_from_file(options|file, timeout)` <!-- --> {#http.h2_stream:write_body_from_file}
 
-See [`stream:write_body_from_file(file, timeout)`](#stream:write_body_from_file)
+See [`stream:write_body_from_file(options|file, timeout)`](#stream:write_body_from_file)
 
 
 ### `h2_stream:shutdown()` <!-- --> {#http.h2_stream:shutdown}

http/stream_common.lua

@@ -152,21 +152,35 @@ function stream_methods:write_body_from_string(str, timeout)
 	return self:write_chunk(str, true, timeout)
 end
 
-function stream_methods:write_body_from_file(file, timeout)
+function stream_methods:write_body_from_file(options, timeout)
 	local deadline = timeout and (monotime()+timeout)
-	-- Can't use :lines here as in Lua 5.1 it doesn't take a parameter
-	while true do
-		local chunk, err = file:read(CHUNK_SIZE)
+	local file, count
+	if io.type(options) then -- lua-http <= 0.2 took a file handle
+		file = options
+	else
+		file = options.file
+		count = options.count
+	end
+	if count == nil then
+		count = math.huge
+	elseif type(count) ~= "number" or count < 0 or count % 1 ~= 0 then
+		error("invalid .count parameter (expected positive integer)")
+	end
+	while count > 0 do
+		local chunk, err = file:read(math.min(CHUNK_SIZE, count))
 		if chunk == nil then
 			if err then
 				error(err)
+			elseif count ~= math.huge and count > 0 then
+				error("unexpected EOF")
 			end
 			break
 		end
 		local ok, err2, errno2 = self:write_chunk(chunk, false, deadline and (deadline-monotime()))
 		if not ok then
 			return nil, err2, errno2
 		end
+		count = count - #chunk
 	end
 	return self:write_chunk("", true, deadline and (deadline-monotime()))
 end

spec/stream_common_spec.lua

@@ -10,16 +10,19 @@ describe("http.stream_common", function()
 		c = h1_connection.new(c, "client", version)
 		return s, c
 	end
+	local function new_request_headers()
+		local headers = new_headers()
+		headers:append(":authority", "myauthority")
+		headers:append(":method", "GET")
+		headers:append(":path", "/")
+		return headers
+	end
 	it("Can read a number of characters", function()
 		local server, client = new_pair(1.1)
 		local cq = cqueues.new()
 		cq:wrap(function()
 			local stream = client:new_stream()
-			local headers = new_headers()
-			headers:append(":authority", "myauthority")
-			headers:append(":method", "GET")
-			headers:append(":path", "/")
-			assert(stream:write_headers(headers, false))
+			assert(stream:write_headers(new_request_headers(), false))
 			assert(stream:write_chunk("foo", false))
 			assert(stream:write_chunk("\nb", false))
 			assert(stream:write_chunk("ar\n", true))
@@ -47,11 +50,7 @@ describe("http.stream_common", function()
 		local cq = cqueues.new()
 		cq:wrap(function()
 			local stream = client:new_stream()
-			local headers = new_headers()
-			headers:append(":authority", "myauthority")
-			headers:append(":method", "GET")
-			headers:append(":path", "/")
-			assert(stream:write_headers(headers, false))
+			assert(stream:write_headers(new_request_headers(), false))
 			assert(stream:write_chunk("foo", false))
 			assert(stream:write_chunk("\nb", false))
 			assert(stream:write_chunk("ar\n", true))
@@ -72,11 +71,7 @@ describe("http.stream_common", function()
 		local cq = cqueues.new()
 		cq:wrap(function()
 			local stream = client:new_stream()
-			local headers = new_headers()
-			headers:append(":authority", "myauthority")
-			headers:append(":method", "GET")
-			headers:append(":path", "/")
-			assert(stream:write_headers(headers, false))
+			assert(stream:write_headers(new_request_headers(), false))
 			assert(stream:write_chunk("hello world!", true))
 		end)
 		cq:wrap(function()
@@ -89,28 +84,116 @@ describe("http.stream_common", function()
 		client:close()
 		server:close()
 	end)
-	it("can write body from temporary file", function()
-		local server, client = new_pair(1.1)
-		local cq = cqueues.new()
-		cq:wrap(function()
-			local file = io.tmpfile()
-			assert(file:write("hello world!"))
-			assert(file:seek("set"))
-			local stream = client:new_stream()
-			local headers = new_headers()
-			headers:append(":authority", "myauthority")
-			headers:append(":method", "GET")
-			headers:append(":path", "/")
-			assert(stream:write_headers(headers, false))
-			assert(stream:write_body_from_file(file))
+	describe("write_body_from_file", function()
+		it("works with a temporary file", function()
+			local server, client = new_pair(1.1)
+			local cq = cqueues.new()
+			cq:wrap(function()
+				local file = io.tmpfile()
+				assert(file:write("hello world!"))
+				assert(file:seek("set"))
+				local stream = client:new_stream()
+				assert(stream:write_headers(new_request_headers(), false))
+				assert(stream:write_body_from_file(file))
+			end)
+			cq:wrap(function()
+				local stream = assert(server:get_next_incoming_stream())
+				assert.same("hello world!", assert(stream:get_body_as_string()))
+			end)
+			assert_loop(cq, TEST_TIMEOUT)
+			assert.truthy(cq:empty())
+			client:close()
+			server:close()
 		end)
-		cq:wrap(function()
-			local stream = assert(server:get_next_incoming_stream())
-			assert.same("hello world!", assert(stream:get_body_as_string()))
+		it("works using the options form", function()
+			local server, client = new_pair(1.1)
+			local cq = cqueues.new()
+			cq:wrap(function()
+				local file = io.tmpfile()
+				assert(file:write("hello world!"))
+				assert(file:seek("set"))
+				local stream = client:new_stream()
+				assert(stream:write_headers(new_request_headers(), false))
+				assert(stream:write_body_from_file({
+					file = file;
+				}))
+			end)
+			cq:wrap(function()
+				local stream = assert(server:get_next_incoming_stream())
+				assert.same("hello world!", assert(stream:get_body_as_string()))
+			end)
+			assert_loop(cq, TEST_TIMEOUT)
+			assert.truthy(cq:empty())
+			client:close()
+			server:close()
+		end)
+		it("validates .count option", function()
+			local server, client = new_pair(1.1)
+			local cq = cqueues.new()
+			cq:wrap(function()
+				local stream = client:new_stream()
+				assert(stream:write_headers(new_request_headers(), false))
+				assert.has_error(function()
+					stream:write_body_from_file({
+						file = io.tmpfile();
+						count = "invalid count field";
+					})
+				end)
+			end)
+			cq:wrap(function()
+				assert(server:get_next_incoming_stream())
+			end)
+			assert_loop(cq, TEST_TIMEOUT)
+			assert.truthy(cq:empty())
+			client:close()
+			server:close()
+		end)
+		it("limits number of bytes when using .count option", function()
+			local server, client = new_pair(1.1)
+			local cq = cqueues.new()
+			cq:wrap(function()
+				local file = io.tmpfile()
+				assert(file:write("hello world!"))
+				assert(file:seek("set"))
+				local stream = client:new_stream()
+				assert(stream:write_headers(new_request_headers(), false))
+				assert(stream:write_body_from_file({
+					file = file;
+					count = 5;
+				}))
+			end)
+			cq:wrap(function()
+				local stream = assert(server:get_next_incoming_stream())
+				assert.same("hello", assert(stream:get_body_as_string()))
+			end)
+			assert_loop(cq, TEST_TIMEOUT)
+			assert.truthy(cq:empty())
+			client:close()
+			server:close()
+		end)
+		it("reports an error on early EOF", function()
+			local server, client = new_pair(1.1)
+			local cq = cqueues.new()
+			cq:wrap(function()
+				local file = io.tmpfile()
+				assert(file:write("hello world!"))
+				assert(file:seek("set"))
+				local stream = client:new_stream()
+				assert(stream:write_headers(new_request_headers(), false))
+				assert.has_error(function()
+					assert(stream:write_body_from_file({
+						file = file;
+						count = 50; -- longer than the file
+					}))
+				end)
+			end)
+			cq:wrap(function()
+				assert(server:get_next_incoming_stream())
+			end)
+			assert_loop(cq, TEST_TIMEOUT)
+			assert.truthy(cq:empty())
+			client:close()
+			server:close()
 		end)
-		assert_loop(cq, TEST_TIMEOUT)
-		assert.truthy(cq:empty())
-		client:close()
-		server:close()
 	end)
 end)