Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Content-Length header more often. #102

Merged
merged 4 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/connection.toit
Original file line number Diff line number Diff line change
Expand Up @@ -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 ($header_length) does not match content length ($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_
Expand Down
7 changes: 6 additions & 1 deletion src/request.toit
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,17 @@ 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
--has_body=(body != null)
--content_length=content_length
--has_body=has_body
if body:
while data := body.read:
body_writer.write data
Expand Down
25 changes: 18 additions & 7 deletions src/server.toit
Original file line number Diff line number Diff line change
Expand Up @@ -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=null --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:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would save code to have content_length default to null here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping as is, to make callers think about the content-length.

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:
Expand All @@ -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
Expand All @@ -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:
Expand Down
31 changes: 31 additions & 0 deletions tests/http_standalone_test.toit
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import expect show *
import http
import http.connection show is_close_exception_
import net
import reader

import .cat

Expand All @@ -26,6 +27,19 @@ POST_DATA ::= {
"slash": "/&%"
}

class NonSizedTestReader implements reader.Reader:
call_count_ := 0
chunks_ := List 5: "$it" * it

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

run_client network port/int -> none:
client := http.Client network

Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"