From bc17398c18dd84ae08580357cfe3458df5dd8a0a Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Thu, 13 Jul 2023 13:14:04 +0200 Subject: [PATCH 1/3] Use Content-Length header more often. --- src/connection.toit | 14 +++++++++++--- src/request.toit | 5 +++++ src/server.toit | 25 ++++++++++++++++++------- tests/http_standalone_test.toit | 31 +++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/connection.toit b/src/connection.toit index 5aec14a..1e0d22b 100644 --- a/src/connection.toit +++ b/src/connection.toit @@ -92,16 +92,24 @@ class Connection: send_headers -> BodyWriter status/string headers/Headers --is_client_request/bool + --content_length/int? --has_body/bool: if current_writer_: throw "Previous request not completed" body_writer/BodyWriter := ? needs_to_write_chunked_header := false if has_body: - content_length := headers.single "Content-Length" + content_length_header := headers.single "Content-Length" + if content_length_header: + header_length := int.parse content_length_header + if not content_length: content_length = header_length + if content_length != header_length: + throw "Content-Length header does not match content length" + else if content_length: + headers.set "Content-Length" "$content_length" + if content_length: - length := int.parse content_length - body_writer = ContentLengthWriter this writer_ length + body_writer = ContentLengthWriter this writer_ content_length else: needs_to_write_chunked_header = true body_writer = ChunkedWriter this writer_ diff --git a/src/request.toit b/src/request.toit index 0a478d2..c1f432b 100644 --- a/src/request.toit +++ b/src/request.toit @@ -44,11 +44,16 @@ class RequestOutgoing extends Request: constructor.private_ .connection_ .method .path .headers: send -> Response: + has_body := body != null + content_length := has_body and (body is reader.SizedReader) + ? (body as reader.SizedReader).size + : null slash := (path.starts_with "/") ? "" : "/" body_writer := connection_.send_headers "$method $slash$path HTTP/1.1\r\n" headers --is_client_request=true + --content_length=content_length --has_body=(body != null) if body: while data := body.read: diff --git a/src/server.toit b/src/server.toit index abfb67b..dbc8e4f 100644 --- a/src/server.toit +++ b/src/server.toit @@ -191,27 +191,32 @@ class ResponseWriter: write_headers status_code/int --message/string?=null: if body_writer_: throw "headers already written" has_body := status_code != STATUS_NO_CONTENT - write_headers_ status_code --message=message --has_body=has_body + write_headers_ + status_code + --message=message + --content_length=null + --has_body=has_body write data: - write_headers_ STATUS_OK --message=null --has_body=true + write_headers_ STATUS_OK --message=null --content_length=data.size --has_body=true body_writer_.write data - write_headers_ status_code/int --message/string? --has_body/bool: + write_headers_ status_code/int --message/string? --content_length/int? --has_body/bool: if body_writer_: return body_writer_ = connection_.send_headers "$VERSION $status_code $(message or (status_message status_code))\r\n" headers --is_client_request=false + --content_length=content_length --has_body=has_body redirect status_code/int location/string --message/string?=null --body/string?=null -> none: headers.set "Location" location if body and body.size > 0: - write_headers_ status_code --message=message --has_body=true + write_headers_ status_code --message=message --content_length=body.size --has_body=true body_writer_.write body else: - write_headers_ status_code --message=message --has_body=false + write_headers_ status_code --message=message --content_length=null --has_body=false // Returns true if the connection was closed due to an error. close_on_exception_ message/string -> bool: @@ -226,7 +231,10 @@ class ResponseWriter: else: // We don't have a body writer, so perhaps we didn't send a response // yet. Send a 500 to indicate an internal server error. - write_headers_ STATUS_INTERNAL_SERVER_ERROR --message=message --has_body=false + write_headers_ STATUS_INTERNAL_SERVER_ERROR + --message=message + --content_length=null + --has_body=false return false // Close the response. We call this automatically after the block if the @@ -246,7 +254,10 @@ class ResponseWriter: // Nothing was written, yet we are already closing. This indicates // We return a 500 error code and log the issue. We don't need to close // the connection. - write_headers_ STATUS_INTERNAL_SERVER_ERROR --message=null --has_body=false + write_headers_ STATUS_INTERNAL_SERVER_ERROR + --message=null + --content_length=null + --has_body=false logger_.info "Returned from router without any data for the client" detach -> tcp.Socket: diff --git a/tests/http_standalone_test.toit b/tests/http_standalone_test.toit index c93d559..1f6fc18 100644 --- a/tests/http_standalone_test.toit +++ b/tests/http_standalone_test.toit @@ -8,6 +8,7 @@ import expect show * import http import http.connection show is_close_exception_ import net +import reader import .cat @@ -26,6 +27,19 @@ POST_DATA ::= { "slash": "/&%" } +class NonSizedTestReader implements reader.Reader: + call_count_ := 0 + chunks_ := List 5: "$it" * it + + read -> ByteArray?: + if call_count_ == chunks_.size: + return null + call_count_++ + return chunks_[call_count_ - 1] + + full_data -> ByteArray: + return (chunks_.join).to_byte_array + run_client network port/int -> none: client := http.Client network @@ -175,6 +189,16 @@ run_client network port/int -> none: response11 := client.post_form --host="localhost" --port=port --path="/post_form" POST_DATA expect_equals 200 response11.status_code + test_reader := NonSizedTestReader + request = client.new_request "POST" --host="localhost" --port=port --path="/post_chunked" + request.body = test_reader + response12 := request.send + expect_equals 200 response12.status_code + response_data := #[] + while chunk := response12.body.read: + response_data += chunk + expect_equals test_reader.full_data response_data + client.close expect_json response/http.Response [verify_block]: @@ -206,6 +230,9 @@ start_server network -> int: listen server server_socket my_port other_port: server.listen server_socket:: | request/http.RequestIncoming response_writer/http.ResponseWriter | + if request.method == "POST" and request.path != "/post_chunked": + expect_not_null (request.headers.single "Content-Length") + if request.path == "/": response_writer.headers.set "Content-Type" "text/html" response_writer.write INDEX_HTML @@ -268,5 +295,9 @@ listen server server_socket my_port other_port: response_writer.headers.set "Content-Type" "application/json" while data := request.body.read: response_writer.redirect http.STATUS_SEE_OTHER "http://localhost:$my_port/cat.png" + else if request.path == "/post_chunked": + response_writer.headers.set "Content-Type" "text/plain" + while data := request.body.read: + response_writer.write data else: response_writer.write_headers http.STATUS_NOT_FOUND --message="Not Found" From 9902f88f4c93bd159e77bf51de516404dac77bfd Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 28 Jul 2023 17:41:55 +0200 Subject: [PATCH 2/3] Feedback. --- src/request.toit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/request.toit b/src/request.toit index c1f432b..b490fb7 100644 --- a/src/request.toit +++ b/src/request.toit @@ -54,7 +54,7 @@ class RequestOutgoing extends Request: headers --is_client_request=true --content_length=content_length - --has_body=(body != null) + --has_body=has_body if body: while data := body.read: body_writer.write data From 9f1b3700bf0924f99c33438e5c3d89ae57f60c13 Mon Sep 17 00:00:00 2001 From: Erik Corry Date: Fri, 28 Jul 2023 18:05:16 +0200 Subject: [PATCH 3/3] Make test run. We can't get the content-length in the write method because this is a method that can be called more than once. Fix some types in the test. --- src/connection.toit | 2 +- src/server.toit | 2 +- tests/http_standalone_test.toit | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/connection.toit b/src/connection.toit index 2d16b57..a6bc75a 100644 --- a/src/connection.toit +++ b/src/connection.toit @@ -104,7 +104,7 @@ class Connection: header_length := int.parse content_length_header if not content_length: content_length = header_length if content_length != header_length: - throw "Content-Length header does not match content length" + throw "Content-Length header ($header_length) does not match content length ($content_length)" else if content_length: headers.set "Content-Length" "$content_length" diff --git a/src/server.toit b/src/server.toit index dbc8e4f..4323e3d 100644 --- a/src/server.toit +++ b/src/server.toit @@ -198,7 +198,7 @@ class ResponseWriter: --has_body=has_body write data: - write_headers_ STATUS_OK --message=null --content_length=data.size --has_body=true + write_headers_ STATUS_OK --message=null --content_length=null --has_body=true body_writer_.write data write_headers_ status_code/int --message/string? --content_length/int? --has_body/bool: diff --git a/tests/http_standalone_test.toit b/tests/http_standalone_test.toit index 1f6fc18..fbbc32c 100644 --- a/tests/http_standalone_test.toit +++ b/tests/http_standalone_test.toit @@ -31,14 +31,14 @@ class NonSizedTestReader implements reader.Reader: call_count_ := 0 chunks_ := List 5: "$it" * it - read -> ByteArray?: + read -> string?: if call_count_ == chunks_.size: return null call_count_++ return chunks_[call_count_ - 1] full_data -> ByteArray: - return (chunks_.join).to_byte_array + return (chunks_.join "").to_byte_array run_client network port/int -> none: client := http.Client network