Code, tests and build changes

src/chunked.toit

@@ -59,12 +59,23 @@ class ChunkedReader_ extends io.Reader:
       raw-length := reader_.read-bytes-up-to '\r'
       expect_ '\n'
 
-      left-in-chunk_ = int.parse raw-length --radix=16
+      length := raw-length.to-string
+      extension := length.index-of ";"
+      if extension >= 0: length = length[..extension].trim
+      if length.is-empty: throw "PROTOCOL_ERROR"
+      length.do:
+        if not '0' <= it <= '9' and not 'a' <= it <= 'f' and not 'A' <= it <= 'F':
+          throw "PROTOCOL_ERROR"
+      left-in-chunk_ = int.parse length --radix=16
 
       // End is indicated by a zero hex length.
       if left-in-chunk_ == 0:
-        expect_ '\r'
-        expect_ '\n'
+        // Consume optional trailers through the empty line. Trailers don't
+        // change the framing or the headers already exposed to the caller.
+        while true:
+          trailer := reader_.read-bytes-up-to '\r'
+          expect_ '\n'
+          if trailer.is-empty: break
         connection_.reading-done_ this
         connection_ = null
 

src/client.toit

@@ -430,7 +430,9 @@ class Client:
       if follow-redirects and
           (is-regular-redirect_ response.status-code
             or response.status-code == STATUS-SEE-OTHER):
-        parsed = get-location_ response parsed
+        next := get-location_ response parsed
+        headers = redirect-headers_ headers parsed next
+        parsed = next
         continue.repeat
       else:
         return response
@@ -441,6 +443,15 @@ class Client:
     location := response.headers.single "Location"
     return ParsedUri_.parse_ location --previous=previous
 
+  redirect-headers_ headers/Headers? previous/ParsedUri_ next/ParsedUri_ -> Headers?:
+    if not headers or next.can-reuse-connection previous: return headers
+    // Credentials belong to the origin that the caller supplied them for.
+    result := headers.copy
+    result.remove "Authorization"
+    result.remove "Proxy-Authorization"
+    result.remove "Cookie"
+    return result
+
   /**
   Variant of $(web-socket --host).
 
@@ -492,7 +503,9 @@ class Client:
       if follow-redirects and
           (is-regular-redirect_ response.status-code
             or response.status-code == STATUS-SEE-OTHER):
-        parsed = get-location_ response parsed
+        next := get-location_ response parsed
+        headers = redirect-headers_ headers parsed next
+        parsed = next
         continue.repeat
       else:
         WebSocket.check-client-upgrade-response_ response nonce
@@ -623,10 +636,14 @@ class Client:
         response = request.send
 
       if follow-redirects and is-regular-redirect_ response.status-code:
-        parsed = get-location_ response parsed
+        next := get-location_ response parsed
+        headers = redirect-headers_ headers parsed next
+        parsed = next
         continue.repeat
       else if follow-redirects and response.status-code == STATUS-SEE-OTHER:
-        parsed = get-location_ response parsed
+        next := get-location_ response parsed
+        headers = redirect-headers_ headers parsed next
+        parsed = next
         headers = headers.copy
         clear-payload-headers_ headers
         return get_ parsed headers --follow-redirects=true // Switch from POST to GET.
@@ -919,9 +936,24 @@ class ParsedUri_:
   /// Returns the hostname, with the port appended if it is non-default.
   host-with-port -> string:
     default-port := SCHEMES_[scheme]
-    return default-port == port ? host : "$host:$port"
+    authority-host := (host.contains ":") ? "[$host]" : host
+    return default-port == port ? authority-host : "$authority-host:$port"
 
   constructor.parse_ uri/string --default-scheme/string? --previous/ParsedUri_?=null:
+    original-uri := uri
+    // Separate the fragment and query before looking for an authority or path.
+    // Their contents may include characters such as '/', '?' and ':'.
+    fragment/string? := null
+    hash := uri.index-of "#"
+    if hash >= 0:
+      fragment = uri[hash + 1..]
+      uri = uri[..hash]
+    query/string? := null
+    question := uri.index-of "?"
+    if question >= 0:
+      query = uri[question + 1..]
+      uri = uri[..question]
+
     // We recognize a scheme if it's either one of the four we support or if it's
     // followed by colon-slash.  This lets us recognize localhost:1080.
     colon := uri.index-of ":"
@@ -936,7 +968,7 @@ class ParsedUri_:
           uri = uri[colon + 1..]
 
     scheme = scheme or default-scheme or (previous and previous.scheme)
-    if not scheme: throw "Missing scheme in '$uri'"
+    if not scheme: throw "Missing scheme in '$original-uri'"
     if not SCHEMES_.contains scheme: throw "Unknown scheme: '$scheme'"
     // If this is a URI supplied by the library user (no previous), we allow
     // plain hostnames with no path, but if there is a previous we require a
@@ -966,14 +998,17 @@ class ParsedUri_:
       host = previous.host
       port = previous.port
       path = uri
-    hash := path.index-of "#"
-    fragment := null
-    if hash > 0:
-      fragment = path[hash + 1..]
-      path = path[..hash]
-    if previous and not path.starts-with "/":
+    if previous and not has-host and path.is-empty:
+      // Empty and fragment-only references retain the previous path and query.
+      // A new query replaces only the previous query, not its resource path.
+      path = previous.path
+      if query:
+        previous-question := path.index-of "?"
+        if previous-question >= 0: path = path[..previous-question]
+    else if previous and not path.starts-with "/":
       // Relative path.
       path = merge-paths_ previous.path path
+    if query: path += "?$query"
     return ParsedUri_.private_
         --scheme=scheme
         --host=host

src/connection.toit

@@ -39,15 +39,19 @@ class Connection:
     return socket_ != null
 
   close -> none:
-    if socket_:
-      socket_.close
-      if current-writer_:
-        current-writer_.close
-      socket_ = null
-      remove-finalizer this
-      write-closed_ = true
-      current-reader_ = null
-      current-writer_ = null
+    if not socket_: return
+    socket := socket_
+    writer := current-writer_
+    // Release ownership before closing a transport that may throw.
+    socket_ = null
+    remove-finalizer this
+    write-closed_ = true
+    current-reader_ = null
+    current-writer_ = null
+    try:
+      socket.close
+    finally:
+      if writer: writer.close
 
   finalize_:
     // TODO: We should somehow warn people that they forgot to close the
@@ -97,8 +101,14 @@ class Connection:
     if not current-reader_:
       close
     else if socket_:
-      socket_.out.close
-      write-closed_ = true
+      success := false
+      try:
+        socket_.out.close
+        write-closed_ = true
+        success = true
+      finally:
+        if not success:
+          catch: close
 
   /**
   Sends the given $headers.
@@ -203,8 +213,7 @@ class Connection:
     if reader.read-byte != '\n': throw "FORMAT_ERROR"
 
     headers := read-headers_
-    content-length-str := headers.single "Content-Length"
-    content-length := content-length-str and (int.parse content-length-str)
+    content-length := read-content-length_ headers
     current-reader_ = body-reader_ headers --request=true content-length
 
     body-reader := current-reader_ or ContentLengthReader_ this reader 0
@@ -217,33 +226,56 @@ class Connection:
     remove-finalizer this
     return socket
 
-  read-response -> Response:
+  read-response --request-method/string?=null -> Response:
     reader := socket_.in
     if current-reader_: throw "Previous response not completed"
     headers := null
+    success := false
     try:
-      version := reader.read-string (reader.index-of ' ' --throw-if-absent)
-      reader.skip 1
-      status-code := int.parse (reader.read-string (reader.index-of ' ' --throw-if-absent))
-      reader.skip 1
-      status-message := reader.read-string (reader.index-of '\r' --throw-if-absent)
-      reader.skip 1
-      if reader.read-byte != '\n': throw "FORMAT_ERROR"
-
-      headers = read-headers_
-      content-length-str := headers.single "Content-Length"
-      content-length := content-length-str and (int.parse content-length-str)
-      current-reader_ = body-reader_ headers --request=false --status-code=status-code content-length
-
-      body-reader := current-reader_ or ContentLengthReader_ this reader 0
-      return Response this version status-code status-message headers body-reader
+      while true:
+        version := reader.read-string (reader.index-of ' ' --throw-if-absent)
+        reader.skip 1
+        status-code := int.parse (reader.read-string (reader.index-of ' ' --throw-if-absent))
+        reader.skip 1
+        status-message := reader.read-string (reader.index-of '\r' --throw-if-absent)
+        reader.skip 1
+        if reader.read-byte != '\n': throw "FORMAT_ERROR"
+
+        headers = read-headers_
+        if is-information-status-code status-code and status-code != STATUS-SWITCHING-PROTOCOLS:
+          continue
+        content-length := read-content-length_ headers
+        current-reader_ = request-method == "HEAD"
+            ? null
+            : (body-reader_ headers --request=false --status-code=status-code content-length)
+
+        body-reader := current-reader_ or ContentLengthReader_ this reader 0
+        success = true
+        return Response this version status-code status-message headers body-reader
 
     finally:
-      if not headers:
+      if not success:
         close
 
+  read-content-length_ headers/Headers -> int?:
+    values := headers.get "Content-Length"
+    if not values: return null
+    // Reject ambiguous framing rather than letting a peer or proxy disagree
+    // with us about where the next message starts.
+    if values.size != 1 or headers.contains "Transfer-Encoding":
+      throw "INVALID_CONTENT_LENGTH"
+    value := values[0].trim
+    if value.is-empty: throw "INVALID_CONTENT_LENGTH"
+    value.do:
+      if not '0' <= it <= '9': throw "INVALID_CONTENT_LENGTH"
+    return int.parse value
+
   body-reader_ headers/Headers --request/bool --status-code/int?=null content-length/int? -> io.Reader?:
     reader := socket_.in
+    if not request and (is-information-status-code status-code
+        or status-code == STATUS-NO-CONTENT
+        or status-code == 304):
+      return null
     if content-length:
       if content-length == 0: return null  // No read is needed to drain this response.
       return ContentLengthReader_ this reader content-length
@@ -257,10 +289,9 @@ class Connection:
       else if not headers.matches T-E "identity":
         throw "No support for $T-E: $(headers.single T-E)"
 
-    if request or status-code == STATUS-NO-CONTENT:
+    if request:
       // For requests (we are the server) a missing Content-Length means a zero
-      // length body.  We also do this as client if the server has explicitly
-      // stated that there is no content.  We return a null reader, which means
+      // length body. We return a null reader, which means
       // the user does not need to drain the response.
       return null
 
@@ -340,7 +371,7 @@ class ContentLengthReader_ extends io.Reader:
     if read-from-wrapped_ >= content-size:
       connection_.reading-done_ this
       return null
-    data := reader_.read --max-size=(content-size - processed)
+    data := reader_.read --max-size=(content-size - read-from-wrapped_)
     if not data:
       connection_.close
       throw io.Reader.UNEXPECTED-END-OF-READER
@@ -396,6 +427,8 @@ class ContentLengthWriter_ extends io.CloseableWriter implements BodyWriter:
     return written-to-wrapped_ >= content-length_
 
   try-write_ data/io.Data from/int to/int -> int:
+    if to - from > content-length_ - written-to-wrapped_:
+      throw "TOO_MUCH_WRITTEN"
     result := writer_.try-write data from to
     written-to-wrapped_ += result
     return result

src/headers.toit

@@ -68,7 +68,7 @@ class Headers:
   */
   remove key/string -> none:
     if not headers_: return
-    headers_.remove key
+    headers_.remove (ascii-normalize_ key)
 
   /**
   Returns the stored values for the given $key.

src/request.toit

@@ -57,7 +57,7 @@ class RequestOutgoing extends Request:
       while data := body.read:
         body-writer.write data
     body-writer.close
-    return connection_.read-response
+    return connection_.read-response --request-method=method
 
   drain: body.drain
 

src/server.toit

@@ -303,7 +303,7 @@ class ResponseWriter extends Object with io.OutMixin:
 
   write-headers status-code/int --message/string?=null:
     if body-writer_: throw "headers already written"
-    has-body := status-code != STATUS-NO-CONTENT
+    has-body := status-code != STATUS-NO-CONTENT and request_.method != "HEAD"
     write-headers_
         status-code
         --message=message
@@ -351,7 +351,7 @@ class ResponseWriter extends Object with io.OutMixin:
   // Returns true if the connection was closed due to an error.
   close-on-exception_ message/string -> bool:
     logger_.info message
-    if not connection_.is-open_:
+    if not connection_ or not connection_.is-open_:
       // The connection already died (typically the peer went away while
       // we were writing the headers). Nothing more we can do.
       return true
@@ -379,7 +379,7 @@ class ResponseWriter extends Object with io.OutMixin:
   */
   close -> none:
     mark-writer-closed_
-    if not connection_.is-open_:
+    if not connection_ or not connection_.is-open_:
       // The connection already died (typically the peer went away while
       // we were writing the headers). Nothing more we can do.
       return
@@ -403,7 +403,9 @@ class ResponseWriter extends Object with io.OutMixin:
       logger_.info "Returned from router without any data for the client"
 
   detach -> tcp.Socket:
+    if not connection_: throw "ALREADY_CLOSED"
+    socket := connection_.detach
     detached_ = true
-    connection := connection_
     connection_ = null
-    return connection.detach
+    mark-writer-closed_
+    return socket