From 113e8dca88b5897b091d33c87dd958a377552021 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 2 Oct 2026 18:10:06 +0200 Subject: [PATCH 1/2] Code, tests and build changes --- src/chunked.toit | 17 +- src/client.toit | 59 +++- src/connection.toit | 101 ++++--- src/headers.toit | 2 +- src/request.toit | 2 +- src/server.toit | 12 +- src/web-socket.toit | 163 ++++++++--- tests/cleanup-test.toit | 304 ++++++++++++++++++++ tests/client-redirect-credentials-test.toit | 44 +++ tests/headers-test.toit | 7 + tests/http-stream-boundaries-test.toit | 96 +++++++ tests/memory-socket.toit | 63 ++++ tests/parse-url-test.toit | 36 +++ tests/websocket-framing-test.toit | 47 +++ tests/websocket-masking-test.toit | 125 ++++++++ 15 files changed, 980 insertions(+), 98 deletions(-) create mode 100644 tests/cleanup-test.toit create mode 100644 tests/client-redirect-credentials-test.toit create mode 100644 tests/http-stream-boundaries-test.toit create mode 100644 tests/memory-socket.toit create mode 100644 tests/websocket-framing-test.toit create mode 100644 tests/websocket-masking-test.toit diff --git a/src/chunked.toit b/src/chunked.toit index 5532a9c..000fd0c 100644 --- a/src/chunked.toit +++ b/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 diff --git a/src/client.toit b/src/client.toit index ff6c5fb..ba95517 100644 --- a/src/client.toit +++ b/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 diff --git a/src/connection.toit b/src/connection.toit index d50110b..310c0cd 100644 --- a/src/connection.toit +++ b/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. @@ -196,8 +206,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 @@ -210,33 +219,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 @@ -250,10 +282,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 @@ -333,7 +364,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 @@ -389,6 +420,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 diff --git a/src/headers.toit b/src/headers.toit index 385a88e..89cae63 100644 --- a/src/headers.toit +++ b/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. diff --git a/src/request.toit b/src/request.toit index 4c4a67a..bbcc04d 100644 --- a/src/request.toit +++ b/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 diff --git a/src/server.toit b/src/server.toit index fc86d30..3bd7b00 100644 --- a/src/server.toit +++ b/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 diff --git a/src/web-socket.toit b/src/web-socket.toit index 1eabd3e..76f03f0 100644 --- a/src/web-socket.toit +++ b/src/web-socket.toit @@ -3,6 +3,7 @@ // found in the LICENSE file. import bitmap show blit XOR +import crypto import crypto.sha1 show sha1 import encoding.base64 import io @@ -42,6 +43,8 @@ class WebSocket: writer-semaphore_ := Semaphore --count=1 current-reader_ /WebSocketReader? := null is-client_ /bool + write-closed_/bool := false + closed_/bool := false constructor .socket_ --client/bool: is-client_ = client @@ -120,7 +123,7 @@ class WebSocket: // Reads the header of the next fragment. next-fragment_ -> FragmentReader_?: - if socket_ == null: return null // Closed. + if closed_: return null reader := socket_.in if not reader.try-ensure-buffered 1: return null @@ -191,10 +194,19 @@ class WebSocket: block until the previous writer is completed. */ start-sending --size/int?=null --opcode/int?=null -> io.CloseableWriter: + if write-closed_: throw "ALREADY_CLOSED" writer-semaphore_.down - assert: current-writer_ == null - current-writer_ = WebSocketWriter.private_ this size --masking=is-client_ --opcode=opcode - return current-writer_ + success := false + try: + if write-closed_: throw "ALREADY_CLOSED" + assert: current-writer_ == null + current-writer_ = WebSocketWriter.private_ this size --masking=is-client_ --opcode=opcode + success = true + return current-writer_ + finally: + if not success: + critical-do --no-respect-deadline: + writer-semaphore_.up /** Send a ping with the given $payload, which is a string or a ByteArray. @@ -207,11 +219,13 @@ class WebSocket: schedule-ping_ payload OPCODE-PING_ schedule-ping_ payload opcode/int -> none: + if write-closed_: throw "ALREADY_CLOSED" if current-writer_: current-writer_.pending-pings_.add [opcode, payload] else: writer-semaphore_.down try: + if write-closed_: throw "ALREADY_CLOSED" // Send immediately. writer := WebSocketWriter.private_ this payload.size --masking=is-client_ --opcode=opcode writer.write payload @@ -221,11 +235,37 @@ class WebSocket: writer-semaphore_.up writer-close_ writer/WebSocketWriter -> none: + // Temporary control-frame writers don't own the message semaphore. + if current-writer_ != writer: return current-writer_ = null writer-semaphore_.up write_ data/io.Data from=0 to=data.byte-size -> none: - socket_.out.write data from to + success := false + try: + socket_.out.write data from to + success = true + finally: + // A partial frame cannot be resumed safely. Preserve the write error. + if not success: + catch: abort_ + + // Releases the message writer without sending any more frame bytes. + abort-writer_ -> none: + critical-do --no-respect-deadline: + write-closed_ = true + if writer := current-writer_: + writer.owner_ = null + writer.mark-closed_ + current-writer_ = null + writer-semaphore_.up + + abort_ -> none: + if closed_: return + closed_ = true + abort-writer_ + current-reader_ = null + socket_.close reader-close_ -> none: current-reader_ = null @@ -238,9 +278,11 @@ class WebSocket: If we are in a suitable place in the protocol, sends a close packet first. */ close --status-code/int=STATUS-WEBSOCKET-NORMAL-CLOSURE: - close-write --status-code=status-code - socket_.close - current-reader_ = null + if closed_: return + try: + close-write --status-code=status-code + finally: + abort_ /** Closes the websocket for writing. @@ -251,23 +293,32 @@ class WebSocket: Most peers will respond by closing the other direction. */ close-write --status-code/int=STATUS-WEBSOCKET-NORMAL-CLOSURE: - if current-writer_ == null: - writer-semaphore_.down - try: - // If we are not in the middle of a message, we can send a close packet. - catch: // Catch because the write end may already be closed. - writer := WebSocketWriter.private_ this 2 --masking=is-client_ --opcode=OPCODE-CLOSE_ - payload := ByteArray 2 - BIG-ENDIAN.put-uint16 payload 0 status-code - writer.write payload - writer.close - finally: - critical-do --no-respect-deadline: - writer-semaphore_.up - socket_.out.close - if current-writer_: - current-writer_ = null - writer-semaphore_.up + if write-closed_: return + write-closed_ = true + success := false + try: + if current-writer_ == null: + writer-semaphore_.down + try: + // If we are not in the middle of a message, we can send a close packet. + catch: // Catch because the write end may already be closed. + writer := WebSocketWriter.private_ this 2 --masking=is-client_ --opcode=OPCODE-CLOSE_ + payload := ByteArray 2 + BIG-ENDIAN.put-uint16 payload 0 status-code + writer.write payload + writer.close + finally: + critical-do --no-respect-deadline: + writer-semaphore_.up + // Another task may have fully closed while the close frame was sent. + if not closed_: + catch --unwind=(: not closed_): socket_.out.close + success = true + finally: + abort-writer_ + // A failed shutdown cannot leave the transport owned by nobody. + if not success: + catch: abort_ static add-client-upgrade-headers_ headers/Headers -> string: // The WebSocket nonce is not very important and does not need to be @@ -341,6 +392,8 @@ class WebSocketWriter extends io.CloseableWriter: remaining-in-fragment_ /int := 0 fragment-sent_ /bool := false masking_ /bool + mask_/ByteArray? := null + mask-offset_/int := 0 // Pings and pongs can be interleaved with the fragments in a message. pending-pings_ /List := [] @@ -370,7 +423,7 @@ class WebSocketWriter extends io.CloseableWriter: size := min (to - from) remaining-in-fragment_ // We don't use slices because data might be a string with UTF-8 // sequences in it. - owner_.write_ data from (from + size) + write-payload_ data from (from + size) from += size remaining-in-fragment_ -= size @@ -386,12 +439,28 @@ class WebSocketWriter extends io.CloseableWriter: opcode := item[0] payload := item[1] write-fragment-header_ payload.size opcode payload.size - owner_.write_ payload + write-payload_ payload + + write-payload_ data/io.Data from/int=0 to/int=data.byte-size -> none: + if not masking_: + owner_.write_ data from to + return + while from < to: + // Bound the temporary allocation and leave the caller's data unchanged. + size := min (to - from) 1024 + masked := ByteArray size: (data.byte-at (from + it)) ^ mask_[(mask-offset_ + it) & 3] + owner_.write_ masked + from += size + mask-offset_ += size + + set-mask_ header/ByteArray -> none: + if not masking_: return + mask_ = crypto.random --size=4 + mask-offset_ = 0 + header.replace (header.size - 4) mask_ write-fragment-header_ max-size/int opcode/int size/int?: header /ByteArray := ? - // If the protocol requires it, we supply a 4 byte mask, but it's always - // zero so we don't need to apply it on send. masking-flag := masking_ ? MASKING-FLAG_ : 0 remaining-in-fragment := ? if size: @@ -419,23 +488,33 @@ class WebSocketWriter extends io.CloseableWriter: header[0] = opcode header[1] = remaining-in-fragment | masking-flag + set-mask_ header owner_.write_ header return remaining-in-fragment close_: - if remaining-in-fragment_ != 0: throw "TOO_LITTLE_WRITTEN" - if owner_: - if size_ == null: - // If size is null, we didn't know the size of the complete message ahead - // of time, which means we didn't set the fin flag on the last packet. Send - // a zero length packet with a fin flag. - header := ByteArray 2 - header[0] = FIN-FLAG_ | (fragment-sent_ ? OPCODE-CONTINUATION_ : OPCODE-BINARY_) - owner_.write_ header - write-any-ping_ - owner_.writer-close_ this // Notify the websocket that we are done. - owner_ = null + owner := owner_ + success := false + try: + if remaining-in-fragment_ != 0: throw "TOO_LITTLE_WRITTEN" + if owner_: + if size_ == null: + // If size is null, we didn't know the size of the complete message ahead + // of time, which means we didn't set the fin flag on the last packet. Send + // a zero length packet with a fin flag. + header := ByteArray (masking_ ? 6 : 2) + header[0] = FIN-FLAG_ | (fragment-sent_ ? OPCODE-CONTINUATION_ : OPCODE-BINARY_) + header[1] = masking_ ? MASKING-FLAG_ : 0 + set-mask_ header + owner_.write_ header + write-any-ping_ + owner_.writer-close_ this // Notify the websocket that we are done. + owner_ = null + success = true + finally: + if not success and owner: + catch: owner.abort_ /** A reader for an individual message sent to us. @@ -460,7 +539,7 @@ class WebSocketReader extends io.Reader: */ read_ -> ByteArray?: result := fragment-reader_.read - if result == null: + while result == null: if fragment-reader_.is-fin: if owner_: owner_.reader-close_ diff --git a/tests/cleanup-test.toit b/tests/cleanup-test.toit new file mode 100644 index 0000000..fe4b248 --- /dev/null +++ b/tests/cleanup-test.toit @@ -0,0 +1,304 @@ +// Copyright (C) 2026 Toit contributors. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/TESTS_LICENSE file. + +import expect show * +import http +import http.connection show Connection +import io +import log +import net.tcp + +import .memory-socket show MemoryReader + +WRITE-ERROR ::= "synthetic write failure" +SHUTDOWN-ERROR ::= "synthetic shutdown failure" +CLOSE-ERROR ::= "synthetic close failure" + +class FaultWriter extends io.CloseableWriter: + buffer/io.Buffer ::= io.Buffer + on-write/Lambda? := null + on-close/Lambda? := null + transport-closed/bool := false + block-write/bool := false + fail-write/bool := false + writes/int := 0 + fail-at/int := -1 + fail-close/bool := false + + try-write_ data/io.Data from/int to/int -> int: + if action := on-write: + on-write = null + action.call + if block-write: sleep --ms=1000 + if fail-write or writes++ == fail-at: throw WRITE-ERROR + return buffer.write data from to + + close_: + if action := on-close: + on-close = null + action.call + if transport-closed: throw CLOSE-ERROR + if fail-close: throw SHUTDOWN-ERROR + +class FaultSocket implements tcp.Socket: + in/MemoryReader + out/FaultWriter ::= FaultWriter + no-delay/bool := true + closed/bool := false + fail-close/bool := false + closes/int := 0 + + constructor chunks/List=[]: + in = MemoryReader chunks + + local-address: unreachable + peer-address: unreachable + mtu: return 1500 + read: return in.read + write data from=0 to=data.byte-size: return out.write data from to + close-write: out.close + close: + closes++ + closed = true + out.transport-closed = true + if fail-close: throw CLOSE-ERROR + +main args: + failures := [] + tests := { + "websocket shutdown": :: test-websocket-shutdown, + "websocket active shutdown": :: test-websocket-shutdown --active, + "websocket half shutdown": :: test-websocket-shutdown --active --half, + "websocket normal half close": :: test-websocket-half-close, + "websocket concurrent close": :: test-websocket-concurrent-close, + "websocket concurrent shutdown": :: test-websocket-concurrent-close --during-shutdown, + "websocket write timeout": :: test-websocket-timeout, + "websocket queued ping": :: test-websocket-queued-ping, + "websocket transport close": :: test-websocket-transport-close, + "websocket header": :: test-websocket-header, + "websocket payload": :: test-websocket-payload, + "websocket streaming payload": :: test-websocket-payload --streaming, + "websocket final fragment": :: test-websocket-final, + "websocket incomplete writer": :: test-websocket-incomplete, + "websocket ping": :: test-websocket-ping, + "response detach": :: test-response-detach, + "response detached close": :: test-response-detach --close-first, + "response error close": :: test-response-error-close, + "response close after error": :: test-response-error-close --close-first, + "response failed detach": :: test-response-failed-detach, + "detached handler failure": :: test-detached-handler, + "connection shutdown": :: test-connection-shutdown, + "connection transport close": :: test-connection-close, + } + tests.do: | name/string test/Lambda | + if not args.is-empty and args[0].starts-with "--case=": + if args[0] != "--case=$name": continue.do + error := catch: test.call + print "$name: $(error or "passed")" + if error: failures.add name + expect failures.is-empty --message="$failures" + +expect-released websocket/http.WebSocket: + expect-null websocket.current-writer_ + with-timeout --ms=100: websocket.writer-semaphore_.down + expect-throw DEADLINE-EXCEEDED-ERROR: + with-timeout --ms=10: websocket.writer-semaphore_.down + websocket.writer-semaphore_.up + +expect-aborted socket/FaultSocket websocket/http.WebSocket: + expect socket.closed + expect-released websocket + expect-throw "ALREADY_CLOSED": + with-timeout --ms=100: websocket.send "later" + expect-released websocket + websocket.close + expect-null websocket.receive + +// A failed half-close must still release the transport and any active writer. +test-websocket-shutdown --active/bool=false --half/bool=false: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + if active: websocket.start-sending + socket.out.fail-close = true + expect-throw SHUTDOWN-ERROR: + if half: websocket.close-write + else: websocket.close + expect-aborted socket websocket + +test-websocket-transport-close: + socket := FaultSocket [#[0x01, 1, 'a']] + websocket := http.WebSocket socket --no-client + websocket.start-receiving + socket.fail-close = true + expect-throw CLOSE-ERROR: websocket.close + expect-null websocket.current-reader_ + expect-released websocket + +test-websocket-header: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + socket.out.fail-write = true + expect-throw WRITE-ERROR: websocket.start-sending --size=1 --opcode=1 + expect-aborted socket websocket + +test-websocket-payload --streaming/bool=false: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + if streaming: + writer := websocket.start-sending --size=1 --opcode=1 + socket.out.fail-write = true + expect-throw WRITE-ERROR: writer.write "a" + else: + socket.out.fail-at = 1 // Fail the payload after send's header succeeds. + expect-throw WRITE-ERROR: websocket.send "b" + expect-aborted socket websocket + +test-websocket-final: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + writer := websocket.start-sending + writer.write "a" + socket.out.fail-write = true + expect-throw WRITE-ERROR: writer.close + expect-aborted socket websocket + +test-websocket-incomplete: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + writer := websocket.start-sending --size=2 --opcode=1 + writer.write "a" + expect-throw "TOO_LITTLE_WRITTEN": writer.close + expect-aborted socket websocket + +test-websocket-ping: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + socket.out.fail-write = true + expect-throw WRITE-ERROR: websocket.ping "a" + expect-aborted socket websocket + +new-request-connection socket/FaultSocket -> Connection: + socket.in.chunks_.add "GET / HTTP/1.1\r\n\r\n".to-byte-array + return Connection socket --location=null + +test-response-detach --close-first/bool=false: + socket := FaultSocket + connection := new-request-connection socket + writer := http.ResponseWriter connection connection.read-request log.default + writer.write-headers http.STATUS-SWITCHING-PROTOCOLS + detached := writer.detach + bytes := socket.out.buffer.size + if close-first: writer.close + expect (writer.close-on-exception_ "synthetic handler error") + writer.close + expect-not socket.closed + expect-equals bytes socket.out.buffer.size + expect-throw "ALREADY_CLOSED": writer.detach + detached.close + +test-response-error-close --close-first/bool=false: + socket := FaultSocket + connection := new-request-connection socket + writer := http.ResponseWriter connection connection.read-request log.default + writer.out.write "a" + expect (writer.close-on-exception_ "synthetic handler error") + if close-first: writer.close + expect (writer.close-on-exception_ "second error") + writer.close + expect socket.closed + +test-response-failed-detach: + socket := FaultSocket + connection := new-request-connection socket + writer := http.ResponseWriter connection connection.read-request log.default + connection.close + expect-throw "ALREADY_CLOSED": writer.detach + expect-not writer.detached_ + writer.close + +test-detached-handler: + socket := FaultSocket + connection := new-request-connection socket + server := http.Server + expect-throw "synthetic handler error": + server.run-connection_ connection (:: | request writer/http.ResponseWriter | + writer.write-headers http.STATUS-SWITCHING-PROTOCOLS + writer.detach + throw "synthetic handler error" + ) log.default + expect-not socket.closed + expect-equals 0 server.handling-count_ + socket.close + +test-connection-shutdown: + socket := FaultSocket ["POST / HTTP/1.1\r\nContent-Length: 1\r\n\r\na".to-byte-array] + connection := Connection socket --location=null + connection.read-request + socket.out.fail-close = true + expect-throw SHUTDOWN-ERROR: connection.close-write_ + expect socket.closed + expect-not connection.is-open_ + connection.close + +test-connection-close: + socket := FaultSocket + connection := Connection socket --location=null + connection.send-headers "HTTP/1.1 200 OK\r\n" (http.Headers) + --is-client-request=false + --content-length=1 + --has-body=true + socket.fail-close = true + expect-throw CLOSE-ERROR: connection.close + expect-not connection.is-open_ + expect-null connection.current-writer_ + expect-null connection.current-reader_ + connection.close + expect-equals 1 socket.closes + +// A successful half-close keeps the read direction usable. +test-websocket-half-close: + socket := FaultSocket [#[0x81, 1, 'a']] + websocket := http.WebSocket socket --no-client + writer := websocket.start-sending + websocket.close-write + expect-not socket.closed + expect-equals "a" websocket.receive + expect-released websocket + expect-throw "WRITER_CLOSED": writer.write "b" + writer.close + websocket.close-write + websocket.close + expect-equals 1 socket.closes + expect-released websocket + +test-websocket-timeout: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + writer := websocket.start-sending + socket.out.block-write = true + expect-throw DEADLINE-EXCEEDED-ERROR: + with-timeout --ms=10: writer.write "a" + expect-aborted socket websocket + +test-websocket-queued-ping: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + writer := websocket.start-sending --size=1 --opcode=1 + websocket.ping "a" + socket.out.fail-at = 2 // Message header and payload succeed, then ping fails. + expect-throw WRITE-ERROR: writer.write "b" + expect-aborted socket websocket + +// Full close can run while another task is sending the half-close frame. +test-websocket-concurrent-close --during-shutdown/bool=false: + socket := FaultSocket + websocket := http.WebSocket socket --no-client + if during-shutdown: + socket.out.on-close = :: websocket.close + else: + socket.out.on-write = :: websocket.close + with-timeout --ms=100: websocket.close-write + expect socket.closed + expect-equals 1 socket.closes + expect-released websocket diff --git a/tests/client-redirect-credentials-test.toit b/tests/client-redirect-credentials-test.toit new file mode 100644 index 0000000..a4cf7c6 --- /dev/null +++ b/tests/client-redirect-credentials-test.toit @@ -0,0 +1,44 @@ +// Copyright (C) 2026 Toit contributors. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/TESTS_LICENSE file. + +import expect show * +import http + +import .memory-socket + +main: + ["GET", "POST"].do: | method/string | + [true, false].do: | same-origin/bool | + [303, 307].do: | status/int | + test method status --same-origin=same-origin + +test method/string status/int --same-origin/bool: + location := same-origin ? "/next" : "http://other.example/next" + redirect := "HTTP/1.1 $status Redirect\r\nLocation: $location\r\nContent-Length: 0\r\n\r\n" + ok := "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n" + first := MemorySocket [(redirect + (same-origin ? ok : "")).to-byte-array] + second := MemorySocket [ok.to-byte-array] + client := http.Client (ScriptedNetwork [first, second]) + headers := http.Headers.from-map { + "Authorization": "Bearer synthetic-test-value", + "Cookie": "session=synthetic-test-value", + "Proxy-Authorization": "Basic synthetic-test-value", + "X-Request": "keep", + } + try: + if method == "GET": + client.get --uri="http://original.example/" --headers=headers + else: + client.post #[] --uri="http://original.example/" --headers=headers + sent := (same-origin ? first : second).out.buffer.to-string + redirected-method := status == 303 ? "GET" : method + start := sent.index-of --last "$redirected-method /next HTTP/1.1" + expect start >= 0 + sent = sent[start..] + expect (sent.contains "X-Request: keep\r\n") + ["Authorization", "Cookie", "Proxy-Authorization"].do: | key/string | + expect-equals same-origin (sent.contains "$key: ") + expect (headers.contains key) + finally: + client.close diff --git a/tests/headers-test.toit b/tests/headers-test.toit index bdee224..a3ffb70 100644 --- a/tests/headers-test.toit +++ b/tests/headers-test.toit @@ -9,6 +9,13 @@ import io main: test-from-map test-keys + test-remove + +test-remove: + ["content-type", "CONTENT-TYPE", "Content-Type"].do: | key/string | + headers := http.Headers.from-map {"Content-Type": "text/plain"} + headers.remove key + expect-not (headers.contains "Content-Type") /** Converts the given $headers to a string. diff --git a/tests/http-stream-boundaries-test.toit b/tests/http-stream-boundaries-test.toit new file mode 100644 index 0000000..648a49f --- /dev/null +++ b/tests/http-stream-boundaries-test.toit @@ -0,0 +1,96 @@ +// Copyright (C) 2026 Toit contributors. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/TESTS_LICENSE file. + +import expect show * +import http.connection show Connection ContentLengthReader_ ContentLengthWriter_ +import http.chunked show ChunkedReader_ + +import .memory-socket + +main: + test-buffering + test-ambiguous-framing + test-invalid-length + test-writer-limit + test-response-framing + test-chunked-metadata + +test-buffering: + socket := MemorySocket ["ab".to-byte-array, "cdNEXT".to-byte-array] + connection := Connection socket --location=null + try: + body := ContentLengthReader_ connection socket.in 4 + body.buffer-all + expect-equals "abcd" body.read-all.to-string + expect-equals "NEXT" socket.in.read-all.to-string + finally: + connection.close + +test-response-framing: + ["HEAD", "304", "103"].do: | kind/string | + next := "HTTP/1.1 200 OK\r\nContent-Length: 1\r\n\r\nx" + first := kind == "103" + ? "HTTP/1.1 103 Early Hints\r\n\r\n" + : "HTTP/1.1 $(kind == "304" ? 304 : 200) Status\r\nContent-Length: 123\r\n\r\n" + socket := MemorySocket [(first + next).to-byte-array] + connection := Connection socket --location=null + try: + method := kind == "HEAD" ? "HEAD" : "GET" + response := (connection.new-request method "/").send + if kind == "103": + expect-equals 200 response.status-code + else: + expect-equals #[] response.body.read-all + response = (connection.new-request "GET" "/").send + expect-equals "x" response.body.read-all.to-string + finally: + connection.close + +test-chunked-metadata: + socket := MemorySocket ["1;name=value\r\nx\r\n0\r\nX-Trailer: ignored\r\n\r\nNEXT".to-byte-array] + connection := Connection socket --location=null + try: + reader := ChunkedReader_ connection socket.in + expect-equals "x" reader.read-all.to-string + expect-equals "NEXT" socket.in.read-all.to-string + finally: + connection.close + +test-ambiguous-framing: + [ + "Content-Length: 0\r\nTransfer-Encoding: chunked\r\n", + "Content-Length: 1\r\nContent-Length: 0\r\n", + "Content-Length: 0\r\nContent-Length: 0\r\n", + ].do: | headers/string | + socket := MemorySocket ["POST / HTTP/1.1\r\n$headers\r\n".to-byte-array] + connection := Connection socket --location=null + try: + expect-throw "INVALID_CONTENT_LENGTH": connection.read-request + finally: + connection.close + response-socket := MemorySocket ["HTTP/1.1 200 OK\r\n$headers\r\n".to-byte-array] + response-connection := Connection response-socket --location=null + expect-throw "INVALID_CONTENT_LENGTH": response-connection.read-response + expect response-socket.closed + +test-invalid-length: + ["-1", "+1", "1_0", "0x10", ""].do: | length/string | + socket := MemorySocket ["POST / HTTP/1.1\r\nContent-Length: $length\r\n\r\n".to-byte-array] + connection := Connection socket --location=null + try: + expect-throw "INVALID_CONTENT_LENGTH": connection.read-request + finally: + connection.close + +test-writer-limit: + socket := MemorySocket + connection := Connection socket --location=null + try: + writer := ContentLengthWriter_ connection socket.out 2 + writer.write "ab" + expect-throw "TOO_MUCH_WRITTEN": writer.write "c" + expect-equals "ab" socket.out.buffer.to-string + writer.close + finally: + connection.close diff --git a/tests/memory-socket.toit b/tests/memory-socket.toit new file mode 100644 index 0000000..c128520 --- /dev/null +++ b/tests/memory-socket.toit @@ -0,0 +1,63 @@ +// Copyright (C) 2026 Toit contributors. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/TESTS_LICENSE file. + +import io +import net +import net.tcp + +class MemorySocket implements tcp.Socket: + in/MemoryReader + out/MemoryWriter ::= MemoryWriter + no-delay/bool := true + closed/bool := false + + constructor chunks/List=[]: + in = MemoryReader chunks + + local-address -> net.SocketAddress: unreachable + peer-address -> net.SocketAddress: unreachable + mtu -> int: return 1500 + read -> ByteArray?: return in.read + write data/io.Data from/int=0 to/int=data.byte-size -> int: + return out.write data from to + close-write: out.close + close: + closed = true + in.close + out.close + +class MemoryReader extends io.CloseableReader: + chunks_/List + index_ := 0 + + constructor .chunks_: + + read_ -> ByteArray?: + if index_ == chunks_.size: return null + return chunks_[index_++] + + close_: + index_ = chunks_.size + +class MemoryWriter extends io.CloseableWriter: + buffer/io.Buffer ::= io.Buffer + max-write-size/int? := null + + try-write_ data/io.Data from/int to/int -> int: + if max-write-size: to = min to (from + max-write-size) + return buffer.write data from to + + close_: + +class ScriptedNetwork implements tcp.Interface: + sockets_/List + index_ := 0 + + constructor .sockets_: + + tcp-connect host/string port/int -> tcp.Socket: + return sockets_[index_++] + + tcp-connect address/net.SocketAddress -> tcp.Socket: unreachable + tcp-listen port/int -> tcp.ServerSocket: unreachable diff --git a/tests/parse-url-test.toit b/tests/parse-url-test.toit index 5f6787f..16612bc 100644 --- a/tests/parse-url-test.toit +++ b/tests/parse-url-test.toit @@ -6,6 +6,7 @@ import http import expect show * main: + test-query-and-fragment parts := http.ParsedUri_.parse_ "https://www.youtube.com/watch?v=2HJxya0CWco#t=0m6s" expect-equals "https" parts.scheme expect-equals "www.youtube.com" parts.host @@ -230,6 +231,7 @@ main: expect parts.use-tls parts = http.ParsedUri_.parse_ "https://[::]:80/foo#fraggy" + expect-equals "[::]:80" parts.host-with-port expect-equals "https" parts.scheme expect-equals "::" parts.host expect-equals 80 parts.port @@ -237,6 +239,9 @@ main: expect-equals "fraggy" parts.fragment expect parts.use-tls + expect-equals "[::1]" + (http.ParsedUri_.parse_ "http://[::1]/").host-with-port + expect-throw "URI_PARSING_ERROR": parts = http.ParsedUri_.parse_ "https://[::] :80/foo#fraggy" expect-throw "URI_PARSING_ERROR": parts = http.ParsedUri_.parse_ "https://[::/foo#fraggy" expect-throw "ILLEGAL_HOSTNAME": parts = http.ParsedUri_.parse_ "https://1234::5678/foo#fraggy" @@ -273,3 +278,34 @@ main: http.ParsedUri_.merge-paths_ "/bar/" "../../foo.txt" expect-throw "ILLEGAL_PATH": http.ParsedUri_.merge-paths_ "/bar/" "./../../foo.txt" + +test-query-and-fragment: + [ + ["http://example.com?x=1", "example.com", "/?x=1", null], + ["http://example.com#frag", "example.com", "/", "frag"], + ["http://example.com?x=/a?b#frag?x", "example.com", "/?x=/a?b", "frag?x"], + ["http://example.com?#", "example.com", "/?", ""], + ["http://[::1]:8080?x=1#frag", "::1", "/?x=1", "frag"], + ].do: | test/List | + parsed := http.ParsedUri_.parse_ test[0] + expect-equals test[1] parsed.host + expect-equals test[2] parsed.path + expect-equals test[3] parsed.fragment + + previous := http.ParsedUri_.parse_ "http://example.com/dir/page?q=old#old" + [ + ["?q=new", "example.com", "/dir/page?q=new", "old"], + ["?", "example.com", "/dir/page?", "old"], + ["#frag", "example.com", "/dir/page?q=old", "frag"], + ["#", "example.com", "/dir/page?q=old", ""], + ["", "example.com", "/dir/page?q=old", "old"], + ["?q=/../?next#frag?x", "example.com", "/dir/page?q=/../?next", "frag?x"], + ["next?q=new#frag", "example.com", "/dir/next?q=new", "frag"], + ["/next#frag", "example.com", "/next", "frag"], + ["//other.example?x=1#frag", "other.example", "/?x=1", "frag"], + ["http://other.example#frag", "other.example", "/", "frag"], + ].do: | test/List | + parsed := http.ParsedUri_.parse_ test[0] --previous=previous + expect-equals test[1] parsed.host + expect-equals test[2] parsed.path + expect-equals test[3] parsed.fragment diff --git a/tests/websocket-framing-test.toit b/tests/websocket-framing-test.toit new file mode 100644 index 0000000..371d462 --- /dev/null +++ b/tests/websocket-framing-test.toit @@ -0,0 +1,47 @@ +// Copyright (C) 2026 Toit contributors. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/TESTS_LICENSE file. + +import expect show * +import http + +import .memory-socket + +main: + test-ping-semaphore + test-close-semaphore + test-empty-continuation + test-masked-terminator + +expect-one-permit socket/http.WebSocket: + socket.writer-semaphore_.down + expect-throw DEADLINE-EXCEEDED-ERROR: + with-timeout --ms=10: socket.writer-semaphore_.down + socket.writer-semaphore_.up + +test-ping-semaphore: + socket := http.WebSocket (MemorySocket) --no-client + socket.ping "hello" + expect-one-permit socket + +test-close-semaphore: + socket := http.WebSocket (MemorySocket) --no-client + socket.close-write + expect-one-permit socket + +test-empty-continuation: + // Two empty non-final fragments followed by the final text payload. + socket := http.WebSocket (MemorySocket [#[0x01, 0, 0x00, 0, 0x80, 1, 'x']]) --client + expect-equals "x" socket.receive + +test-masked-terminator: + transport := MemorySocket + socket := http.WebSocket transport --client + writer := socket.start-sending + writer.write "x" + writer.close + bytes := transport.out.buffer.bytes + // The first frame contains its two header bytes, four mask bytes and x. + expect-equals 13 bytes.size + expect-equals 0x80 bytes[7] + expect-equals 0x80 bytes[8] diff --git a/tests/websocket-masking-test.toit b/tests/websocket-masking-test.toit new file mode 100644 index 0000000..39c485b --- /dev/null +++ b/tests/websocket-masking-test.toit @@ -0,0 +1,125 @@ +// Copyright (C) 2026 Toit contributors. +// Use of this source code is governed by a Zero-Clause BSD license that can +// be found in the tests/TESTS_LICENSE file. + +import expect show * +import http +import io + +import .memory-socket + +main: + test-fresh-masks + test-lengths + test-slices + test-controls + test-fixed-control + test-pong + test-server-frames + +// Decode the wire bytes independently of the package's fragment reader. +frames transport/MemorySocket --masked/bool=true -> List: + reader := io.Reader transport.out.buffer.bytes + result := [] + while reader.try-ensure-buffered 1: + control := reader.read-byte + length-byte := reader.read-byte + expect-equals masked ((length-byte & 0x80) != 0) + size := length-byte & 0x7f + if size == 126: size = reader.big-endian.read-uint16 + else if size == 127: size = reader.big-endian.read-int64 + mask := masked ? (reader.read-bytes 4) : null + payload := reader.read-bytes size + if mask: + payload.size.repeat: payload[it] ^= mask[it & 3] + result.add [control, mask, payload] + return result + +test-fresh-masks: + transport := MemorySocket + socket := http.WebSocket transport --client + 8.repeat: socket.send "test" + decoded := frames transport + expect-equals 8 decoded.size + masks := {} + decoded.do: | frame/List | + masks.add (io.BIG-ENDIAN.uint32 frame[1] 0) + expect-equals "test" frame[2].to-string + // A constant key must fail. Eight independent keys all matching has a + // probability of 2^-224, so this does not reject an occasional zero key. + expect masks.size > 1 + +test-lengths: + [0, 1, 125, 126, 65535, 65536].do: | size/int | + transport := MemorySocket + transport.out.max-write-size = 3 + socket := http.WebSocket transport --client + payload := ByteArray size: it & 0xff + socket.send payload + decoded := frames transport + expect-equals 1 decoded.size + expect-equals 0x82 decoded[0][0] + expect-equals payload decoded[0][2] + // Masking must not modify the caller's buffer. + payload.size.repeat: expect-equals (it & 0xff) payload[it] + +test-slices: + transport := MemorySocket + transport.out.max-write-size = 1 + socket := http.WebSocket transport --client + writer := socket.start-sending --size=4 + text := "A€BC" + // Split a UTF-8 sequence across writes and start at a nonzero byte offset. + writer.write text 1 3 + writer.write text 3 5 + writer.close + decoded := frames transport + expect-equals 1 decoded.size + expect-equals 0x81 decoded[0][0] + expect-equals "€B" decoded[0][2].to-string + +test-controls: + transport := MemorySocket + transport.out.max-write-size = 2 + socket := http.WebSocket transport --client + socket.ping "before" + writer := socket.start-sending + writer.write ("a" * 130) + socket.ping "during" + writer.write ("b" * 130) + writer.close + socket.close-write + decoded := frames transport + expect-equals [0x89, 0x01, 0x00, 0x89, 0x00, 0x00, 0x80, 0x88] + decoded.map: it[0] + expect-equals ["before", "a" * 125, "a" * 5, "during", "b" * 125, "b" * 5, ""] + decoded[..7].map: it[2].to-string + expect-equals #[3, 232] decoded[7][2] + +test-server-frames: + transport := MemorySocket + socket := http.WebSocket transport --no-client + socket.send "server" + decoded := frames transport --no-masked + expect-equals "server" decoded[0][2].to-string + +test-fixed-control: + transport := MemorySocket + socket := http.WebSocket transport --client + writer := socket.start-sending --size=7 + writer.write "abc" + socket.ping "queued" + writer.write "defg" + writer.close + decoded := frames transport + expect-equals [0x81, 0x89] (decoded.map: it[0]) + expect-equals ["abcdefg", "queued"] (decoded.map: it[2].to-string) + +test-pong: + transport := MemorySocket [#[0x89, 1, 'p', 0x81, 1, 'x']] + socket := http.WebSocket transport --client + expect-equals "x" socket.receive + decoded := frames transport + expect-equals 1 decoded.size + expect-equals 0x8a decoded[0][0] + expect-equals "p" decoded[0][2].to-string From 58731db8c158c879974c3b87840ab2e6b3cc7ce1 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Fri, 2 Oct 2026 20:05:38 +0200 Subject: [PATCH 2/2] Use blit to mask outgoing WebSocket payloads Masking built each chunk with a per-byte block call. Copy the chunk with write-to-byte-array and XOR it with the blit-based helper the reader already uses for unmasking. Reuse one scratch buffer per write, since the TCP and TLS writers copy the data before returning. Move the helper to a top-level mask-bytes_, since it now serves both directions. --- src/web-socket.toit | 50 +++++++++++-------- .../websocket-standalone-semaphore-test.toit | 4 +- tests/websocket-standalone-test.toit | 4 +- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/web-socket.toit b/src/web-socket.toit index 76f03f0..8ef84b0 100644 --- a/src/web-socket.toit +++ b/src/web-socket.toit @@ -445,11 +445,15 @@ class WebSocketWriter extends io.CloseableWriter: if not masking_: owner_.write_ data from to return + // Mask a copy, since the caller's data may be a string or reused after + // the write. The socket copies the data, so we can reuse the buffer. + scratch := ByteArray (min (to - from) 4096) while from < to: - // Bound the temporary allocation and leave the caller's data unchanged. - size := min (to - from) 1024 - masked := ByteArray size: (data.byte-at (from + it)) ^ mask_[(mask-offset_ + it) & 3] - owner_.write_ masked + size := min (to - from) scratch.size + chunk := scratch[..size] + data.write-to-byte-array chunk --at=0 from (from + size) + mask-bytes_ chunk mask_ mask-offset_ + owner_.write_ chunk from += size mask-offset_ += size @@ -593,7 +597,7 @@ class FragmentReader_: owner_.unread_ next-byte-array[max..] next-byte-array = next-byte-array[..max] if masking-bytes: - unmask-bytes_ next-byte-array masking-bytes received_ + mask-bytes_ next-byte-array masking-bytes received_ received_ += next-byte-array.size return next-byte-array @@ -601,18 +605,24 @@ class FragmentReader_: if control-bits_ & FIN-FLAG_ == 0: return null return size_ - static unmask-bytes_ byte-array/ByteArray masking-bytes/ByteArray received/int -> none: - for i := 0; i < byte-array.size; i++: - if received & 3 == 0 and i + 4 < byte-array.size: - // When we are at the start of the masking bytes we can accelerate with blit. - blit - masking-bytes // Source. - byte-array[i..] // Destination. - 4 // Line width of 4 bytes. - --source-line-stride=0 // Restart at the beginning of the masking bytes on every line. - --operation=XOR // dest[i] ^= source[j]. - // Skip the bytes we just blitted. - blitted := round-down (byte-array.size - i) 4 - i += blitted - if i < byte-array.size: - byte-array[i] ^= masking-bytes[received++ & 3] +/** +XORs the $byte-array in place with the 4-byte $masking-bytes, starting at + $offset bytes into the mask. + +Masking is its own inverse, so this both masks and unmasks. +*/ +mask-bytes_ byte-array/ByteArray masking-bytes/ByteArray offset/int -> none: + for i := 0; i < byte-array.size; i++: + if offset & 3 == 0 and i + 4 < byte-array.size: + // When we are at the start of the masking bytes we can accelerate with blit. + blit + masking-bytes // Source. + byte-array[i..] // Destination. + 4 // Line width of 4 bytes. + --source-line-stride=0 // Restart at the beginning of the masking bytes on every line. + --operation=XOR // dest[i] ^= source[j]. + // Skip the bytes we just blitted. + blitted := round-down (byte-array.size - i) 4 + i += blitted + if i < byte-array.size: + byte-array[i] ^= masking-bytes[offset++ & 3] diff --git a/tests/websocket-standalone-semaphore-test.toit b/tests/websocket-standalone-semaphore-test.toit index b64456d..74acd11 100644 --- a/tests/websocket-standalone-semaphore-test.toit +++ b/tests/websocket-standalone-semaphore-test.toit @@ -6,7 +6,7 @@ import expect show * import http import monitor show Semaphore import net -import http.web-socket show FragmentReader_ +import http.web-socket show mask-bytes_ // Sets up a web server that can switch to websocket mode on the "/" path. // The server just sends back everything it gets. @@ -116,6 +116,6 @@ unmark-bytes-test -> none: for offset := 0; offset < 4; offset++: for size := 98; size < 102; size++: data := ByteArray size: it - FragmentReader_.unmask-bytes_ data mask offset + mask-bytes_ data mask offset data.size.repeat: expect-equals data[it] (it ^ mask[(it + offset) & 3]) diff --git a/tests/websocket-standalone-test.toit b/tests/websocket-standalone-test.toit index 8d68783..c784bdb 100644 --- a/tests/websocket-standalone-test.toit +++ b/tests/websocket-standalone-test.toit @@ -5,7 +5,7 @@ import expect show * import http import net -import http.web-socket show FragmentReader_ +import http.web-socket show mask-bytes_ // Sets up a web server that can switch to websocket mode on the "/" path. // The server just sends back everything it gets. @@ -116,6 +116,6 @@ unmark-bytes-test -> none: for offset := 0; offset < 4; offset++: for size := 98; size < 102; size++: data := ByteArray size: it - FragmentReader_.unmask-bytes_ data mask offset + mask-bytes_ data mask offset data.size.repeat: expect-equals data[it] (it ^ mask[(it + offset) & 3])