From 0ff86f5dc633bb517ac23a95a47626325a680eb8 Mon Sep 17 00:00:00 2001 From: Kacy Fortner Date: Sun, 9 Aug 2026 17:42:57 +0000 Subject: [PATCH] fix: finish a socket write instead of dropping its tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tcp_write and tcp_write_bytes are a single write(2). the kernel takes what fits in the send buffer and returns that count; the rest is not queued anywhere. every std caller discarded the count, so a buffer larger than the send buffer lost its tail silently and the peer waited for bytes that were never written. a 1 MiB redis SET worked; a 4 MiB one hung. std.net.tcp grows write_all and write_all_bytes, which resume from where the last write stopped and only give up when a write accepts nothing at all — that is the reader having gone, and it is the only condition that ends the loop early. redis, postgres, mysql, the h2c client preface and both http/2 plaintext transports now write through it. the loop runs on bytes rather than on text, which turned out to matter more than the missing loop did. std.io already looped correctly, but it resumed by re-slicing the String, and a send buffer fills at whatever byte offset it fills at — including the middle of a multi-byte character, which a String cannot be cut at. TcpStream.write_all on three megabytes of non-ascii stopped the process outright. the text write_all helpers now encode once and resume through their bytes counterpart. the tls write path was already right: Conn.write_bytes caps at one 16 KiB record, but Conn.write_all_bytes loops over it and its socket writes go through std.io's byte loop. separately, the redis reply parser read line[0] to get the tag with nothing checking the line was non-empty. read_line returns what came before the terminator, so a bare CRLF handed it "", and indexing a String is strict — a peer sending two bytes killed the client process, while every other malformed shape in the same parser came back as an ordinary error. --- docs/STYLE.md | 2 +- docs/io_foundation.md | 29 +++++ examples/net_echo.pith | 24 ++-- examples/redis_client.pith | 3 +- examples/tcp_echo.pith | 23 ++-- examples/web_h2.pith | 6 +- std/io.pith | 55 ++------ std/mysql.pith | 3 +- std/net/http2/connection.pith | 3 +- std/net/http2/server.pith | 2 +- std/net/tcp.pith | 51 +++++++- std/postgres.pith | 5 +- std/redis.pith | 14 ++- std/web.pith | 6 +- tests/cases/test_redis_bare_crlf.pith | 80 ++++++++++++ tests/cases/test_tcp_write_all.pith | 160 ++++++++++++++++++++++++ tests/expected/test_redis_bare_crlf.txt | 5 + tests/expected/test_tcp_write_all.txt | 12 ++ 18 files changed, 398 insertions(+), 85 deletions(-) create mode 100644 tests/cases/test_redis_bare_crlf.pith create mode 100644 tests/cases/test_tcp_write_all.pith create mode 100644 tests/expected/test_redis_bare_crlf.txt create mode 100644 tests/expected/test_tcp_write_all.txt diff --git a/docs/STYLE.md b/docs/STYLE.md index 59d15dee..c2e86cf4 100644 --- a/docs/STYLE.md +++ b/docs/STYLE.md @@ -444,7 +444,7 @@ Every file should start with a brief description: # This module provides functions for making HTTP requests with support # for keep-alive connections, retries, and custom headers. -from std.net.tcp import connect, read, write, close +from std.net.tcp import connect, read, write_all, close ``` ### Import Organization diff --git a/docs/io_foundation.md b/docs/io_foundation.md index 05a19dc9..d5e3d978 100644 --- a/docs/io_foundation.md +++ b/docs/io_foundation.md @@ -64,6 +64,35 @@ the io layer now includes: `std.fs` now exposes stream-based `open`, `create`, and `open_append` on the same foundation. +## write is one syscall, write_all is the loop + +this is the one distinction in the layer worth learning before you use it. + +`write` and `write_bytes` are a single `write(2)`. a socket has a bounded send +buffer, so a buffer larger than the space left in it is written *in part*: the +kernel takes what fits and returns that count, and the rest is not queued +anywhere. a short write is normal, not an error — but discarding the returned +count drops the tail silently, and the peer then waits for bytes that were never +written. a 1 MiB redis `SET` used to work and a 4 MiB one used to hang for +exactly this reason. + +`write_all` and `write_all_bytes` are the looping form: they resume from where +the last write stopped and only stop early when a write accepts nothing at all, +which means the reader is gone. use them unless you have a specific reason to +handle the count yourself. the same pair exists at every level: + +- fd level: `std.net.tcp`'s `write` / `write_all` / `write_all_bytes` +- stream level: `TcpStream`, `FileStream`, `ProcessStdin` +- tls: `Conn.write_bytes` is capped at one 16 KiB record, so it too is a partial + write by construction; `Conn.write_all_bytes` is the loop +- the buffered writers flush through `write_all`, so they are already correct + +one subtlety the loop has to get right: the resume runs on **bytes**, never on +text. a send buffer fills at whatever byte offset it fills at, and that offset +can be in the middle of a multi-byte character. a `String` cannot be cut there — +slicing one at a non-boundary offset stops the process — so the text `write_all` +helpers encode once and resume through their bytes counterpart. + ## why the adapters are handle-backed pith structs are value types right now. that means a tiny adapter struct cannot diff --git a/examples/net_echo.pith b/examples/net_echo.pith index 904d52cd..23246ebc 100644 --- a/examples/net_echo.pith +++ b/examples/net_echo.pith @@ -1,12 +1,16 @@ # TCP echo server/client test +import std.net.tcp as tcp + fn run_server(port: Int) -> Int!: - s := tcp_listen("127.0.0.1", port)! - defer tcp_close(s) - c := tcp_accept(s)! - defer tcp_close(c) - data := tcp_read(c, 1024)! - tcp_write(c, "echo: {data}")! + s := tcp.listen("127.0.0.1", port)! + defer tcp.close(s) + c := tcp.accept(s)! + defer tcp.close(c) + data := tcp.read(c, 1024)! + # write_all, not write: one write syscall stops when the send buffer fills, + # and the bytes it did not take are simply not sent. + tcp.write_all(c, "echo: {data}")! return 0 fn run() -> String!: @@ -17,10 +21,10 @@ fn run() -> String!: sleep(200) # connect client - conn := tcp_connect("127.0.0.1", port)! - defer tcp_close(conn) - tcp_write(conn, "hello pith")! - response := tcp_read(conn, 1024)! + conn := tcp.connect("127.0.0.1", port)! + defer tcp.close(conn) + tcp.write_all(conn, "hello pith")! + response := tcp.read(conn, 1024)! print(response) await task diff --git a/examples/redis_client.pith b/examples/redis_client.pith index 242cd113..a715ce80 100644 --- a/examples/redis_client.pith +++ b/examples/redis_client.pith @@ -7,6 +7,7 @@ # skip the mock entirely. import std.redis as redis +import std.net.tcp as tcp # a stand-in redis: accept one connection, then reply to each command in turn # with a pre-baked RESP response. it does not parse the requests — the client @@ -18,7 +19,7 @@ fn fake_redis(port: Int, replies: List[String]) -> Int!: defer tcp_close(conn) for reply in replies: request := tcp_read(conn, 4096)! - tcp_write(conn, reply)! + tcp.write_all(conn, reply)! return 0 fn main() -> Int!: diff --git a/examples/tcp_echo.pith b/examples/tcp_echo.pith index 3a0483c6..286b9bb6 100644 --- a/examples/tcp_echo.pith +++ b/examples/tcp_echo.pith @@ -1,12 +1,15 @@ # tcp echo — start a server, connect a client, echo a message +import std.net.tcp as tcp + fn server(port: Int) -> String!: - server_fd := tcp_listen("127.0.0.1", port)! - defer tcp_close(server_fd) - client_fd := tcp_accept(server_fd)! - defer tcp_close(client_fd) - msg := tcp_read(client_fd, 1024)! - tcp_write(client_fd, msg)! + server_fd := tcp.listen("127.0.0.1", port)! + defer tcp.close(server_fd) + client_fd := tcp.accept(server_fd)! + defer tcp.close(client_fd) + msg := tcp.read(client_fd, 1024)! + # write_all keeps going after a short write; write would drop the remainder. + tcp.write_all(client_fd, msg)! return "" fn run() -> String!: @@ -22,10 +25,10 @@ fn run() -> String!: sleep(100) # connect and exchange data - connection_fd := tcp_connect("127.0.0.1", port)! - defer tcp_close(connection_fd) - tcp_write(connection_fd, "hello from pith")! - response := tcp_read(connection_fd, 1024)! + connection_fd := tcp.connect("127.0.0.1", port)! + defer tcp.close(connection_fd) + tcp.write_all(connection_fd, "hello from pith")! + response := tcp.read(connection_fd, 1024)! print("echo response: {response}") await server_task diff --git a/examples/web_h2.pith b/examples/web_h2.pith index 22477f95..fbd84b20 100644 --- a/examples/web_h2.pith +++ b/examples/web_h2.pith @@ -89,13 +89,13 @@ impl FrameStream: # (END_STREAM + END_HEADERS). :authority is a fixed "localhost" — the app does # not route on it. fn send_get(fd: Int, path: String) -> Int!: - tcp_write_bytes(fd, bytes.from_string_utf8(H2C_PREFACE))! + tcp.write_all_bytes(fd, bytes.from_string_utf8(H2C_PREFACE))! no_settings: List[frames.Setting] := [] - tcp_write_bytes(fd, frames.frame_bytes(frames.settings_frame(no_settings)!)!)! + tcp.write_all_bytes(fd, frames.frame_bytes(frames.settings_frame(no_settings)!)!)! encoder := hpack.new_encoder(hpack.DEFAULT_HEADER_TABLE_SIZE, false) fields := [hpack.header_field(":method", "GET"), hpack.header_field(":path", path), hpack.header_field(":scheme", "http"), hpack.header_field(":authority", "localhost")] block := encoder.encode(fields) - tcp_write_bytes(fd, frames.frame_bytes(frames.headers_frame(1, block, true, true))!)! + tcp.write_all_bytes(fd, frames.frame_bytes(frames.headers_frame(1, block, true, true))!)! return 0 # read response frames until stream 1 ends, returning the DATA payload as text. diff --git a/std/io.pith b/std/io.pith index 9872e7d5..f16cc2d1 100644 --- a/std/io.pith +++ b/std/io.pith @@ -1091,38 +1091,19 @@ fn write_all_string(writer: StringBuffer, data: String) -> Int!: remaining = remaining_after_write(remaining, wrote) return total +# the text write_all loops all resume through their bytes counterpart rather +# than re-slicing the String. a write stops at whatever byte offset the send +# buffer or the pipe ran out at, and that offset can be in the middle of a +# multi-byte character — which a String cannot be cut at, so resuming as text +# would stop the process on the first non-ascii payload big enough to be split. fn write_all_tcp_stream(writer: TcpStream, data: String) -> Int!: - mut total := 0 - mut remaining := data - while remaining.len() > 0: - wrote := writer.write(remaining)! - if wrote <= 0: - fail "write returned 0" - total = total + wrote - remaining = remaining_after_write(remaining, wrote) - return total + return write_all_tcp_stream_bytes(writer, encode_utf8(data)) fn write_all_tcp_stream_ctx(writer: TcpStream, ctx: Context, data: String) -> Int!BlockingError: - mut total := 0 - mut remaining := data - while remaining.len() > 0: - wrote := tcp_stream_write_ctx(writer, ctx, remaining)! - if wrote <= 0: - fail blocking_failed("write returned 0") - total = total + wrote - remaining = remaining_after_write(remaining, wrote) - return total + return write_all_tcp_stream_bytes_ctx(writer, ctx, encode_utf8(data)) fn write_all_file_stream(writer: FileStream, data: String) -> Int!: - mut total := 0 - mut remaining := data - while remaining.len() > 0: - wrote := writer.write(remaining)! - if wrote <= 0: - fail "write returned 0" - total = total + wrote - remaining = remaining_after_write(remaining, wrote) - return total + return write_all_file_stream_bytes(writer, encode_utf8(data)) fn copy_string(reader: StringReader, writer: StringBuffer) -> Int!: return copy_string_chunked(reader, writer, DEFAULT_CHUNK_SIZE) @@ -1228,15 +1209,7 @@ pub fn append_file_text_chunked(path: String, data: String, chunk_size: Int) -> return total fn write_all_process_stdin(writer: ProcessStdin, data: String) -> Int!: - mut total := 0 - mut remaining := data - while remaining.len() > 0: - wrote := writer.write(remaining)! - if wrote <= 0: - fail "write returned 0" - total = total + wrote - remaining = remaining_after_write(remaining, wrote) - return total + return write_all_process_stdin_bytes(writer, encode_utf8(data)) fn read_all_process_stdout(reader: ProcessStdout) -> String!: return read_all_process_stdout_chunked(reader, DEFAULT_CHUNK_SIZE) @@ -1275,15 +1248,7 @@ fn process_stdin_write_ctx(writer: ProcessStdin, ctx: Context, data: String) -> return wrote.ok fn write_all_process_stdin_ctx(writer: ProcessStdin, ctx: Context, data: String) -> Int!BlockingError: - mut total := 0 - mut remaining := data - while remaining.len() > 0: - wrote := process_stdin_write_ctx(writer, ctx, remaining)! - if wrote <= 0: - fail blocking_failed("write returned 0") - total = total + wrote - remaining = remaining_after_write(remaining, wrote) - return total + return write_all_process_stdin_bytes_ctx(writer, ctx, encode_utf8(data)) fn process_stdout_read_ctx(reader: ProcessStdout, ctx: Context, max_bytes: Int) -> String!BlockingError: task := spawn process_stdout_read_worker(reader, max_bytes) diff --git a/std/mysql.pith b/std/mysql.pith index 5134f6e2..8b0bcd9b 100644 --- a/std/mysql.pith +++ b/std/mysql.pith @@ -17,6 +17,7 @@ import std.bytes as bytes import std.bits as bits import std.hash as hash import std.encoding as encoding +import std.net.tcp as tcp import std.net.tls as tls from std.io import TcpStream from std.iter import Iterator @@ -144,7 +145,7 @@ impl Conn: fn send_raw(data: Bytes) -> Int!: if self.secure: return tls.conn_from_handle(self.tls_handle).write_all_bytes(data)! - return tcp_write_bytes(self.fd, data)! + return tcp.write_all_bytes(self.fd, data)! fn send_packet(seq: Int, payload: Bytes) -> Int!: out := bytes.buffer() diff --git a/std/net/http2/connection.pith b/std/net/http2/connection.pith index d76464bb..1f397023 100644 --- a/std/net/http2/connection.pith +++ b/std/net/http2/connection.pith @@ -30,6 +30,7 @@ import std.bytes as bytes import std.binary as binary import std.encoding as encoding import std.net.url as url +import std.net.tcp as tcp import std.net.tls as tls import std.time as time @@ -177,7 +178,7 @@ impl Connection: fn send(data: Bytes) -> Int!: if self.live: if self.plaintext: - return tcp_write_bytes(self.tls_handle, data)! + return tcp.write_all_bytes(self.tls_handle, data)! return tls.conn_from_handle(self.tls_handle).write_all_bytes(data)! self.sent = bytes.concat(self.sent, data) return data.len() diff --git a/std/net/http2/server.pith b/std/net/http2/server.pith index 97fc0c5a..93a79131 100644 --- a/std/net/http2/server.pith +++ b/std/net/http2/server.pith @@ -283,7 +283,7 @@ impl ServerConn: # is sent, exactly as the h2 client writes. fn send(data: Bytes) -> Int!: if self.transport == TRANSPORT_TCP: - return tcp_write_bytes(self.fd, data)! + return tcp.write_all_bytes(self.fd, data)! if self.transport == TRANSPORT_TLS: return tls.conn_from_handle(self.tls_handle).write_all_bytes(data)! self.sent = bytes.concat(self.sent, data) diff --git a/std/net/tcp.pith b/std/net/tcp.pith index 87e0d3e0..28ca77d6 100644 --- a/std/net/tcp.pith +++ b/std/net/tcp.pith @@ -3,13 +3,16 @@ # TCP client and server functionality. All I/O is blocking — use spawn for concurrency. # Connections are represented as plain Int file descriptors. # -# from std.net.tcp import connect, read, write, close -import std.resilience as resilience +# from std.net.tcp import connect, read, write_all, close +# # fd := connect("127.0.0.1", 8080)! -# write(fd, "hello")! +# write_all(fd, "hello")! # data := read(fd, 1024)! # close(fd) +import std.bytes as bytes +import std.resilience as resilience + # =============================================================== # Client Operations # =============================================================== @@ -105,13 +108,47 @@ pub fn read(fd: Int, max_bytes: Int) -> String!: return tcp_read(fd, max_bytes) # Writes data to a connection. -# Returns the number of bytes written or fails on error. +# +# this is one write syscall, and it returns the number of bytes the kernel +# accepted — which may be fewer than were offered. a socket has a bounded send +# buffer, so a buffer larger than the space left in it is written in part and +# the rest is not written at all. the count is the whole result: ignoring it +# drops the tail silently, and the peer waits for bytes that will never come. +# reach for write_all() unless you have a reason to handle the count yourself. # # as with read(), fd must be a socket: a write to a descriptor that is not one # would overwrite whatever file inherited the number, so it stops the process. pub fn write(fd: Int, data: String) -> Int!: return tcp_write(fd, data) +# Writes every byte of data to a connection, and returns how many that was. +# +# a short write is normal rather than an error — the send buffer filled — so +# this resumes from where the last one stopped and keeps going until the buffer +# is out. only a write that accepts nothing at all ends it, and that means the +# reader is gone. +# +# the loop runs on bytes, not on characters: a send buffer fills at whatever +# byte offset it fills at, which may be in the middle of a multi-byte character, +# and a String cannot be cut there. +pub fn write_all(fd: Int, data: String) -> Int!: + return write_all_bytes(fd, bytes.from_string_utf8(data)) + +# Writes every byte of data to a connection, and returns how many that was. +# The Bytes form of write_all(); see it for the contract. +pub fn write_all_bytes(fd: Int, data: Bytes) -> Int!: + total := data.len() + mut sent := 0 + mut remaining := data + while sent < total: + wrote := tcp_write_bytes(fd, remaining)! + if wrote <= 0: + fail "tcp: connection closed after writing {sent} of {total} bytes" + sent = sent + wrote + if sent < total: + remaining = remaining.slice(wrote, remaining.len()) + return sent + # Set read timeout in milliseconds (0 = no timeout). # Connections default to 5 second timeout. pub fn set_timeout(fd: Int, ms: Int): @@ -129,5 +166,11 @@ pub fn close(fd: Int): test "tcp wrappers report invalid descriptor errors": assert(read(-1, 8).is_err) assert(write(-1, "hello").is_err) + assert(write_all(-1, "hello").is_err) + assert(write_all_bytes(-1, bytes.from_string_utf8("hello")).is_err) set_timeout(-1, 1) close(-1) + +test "write_all on an empty buffer writes nothing and does not fail": + assert_eq(write_all(-1, "") catch -1, 0) + assert_eq(write_all_bytes(-1, bytes.empty()) catch -1, 0) diff --git a/std/postgres.pith b/std/postgres.pith index 3913f933..b00eea6b 100644 --- a/std/postgres.pith +++ b/std/postgres.pith @@ -30,6 +30,7 @@ import std.hash as hash import std.crypto.hmac as hmac import std.crypto.kdf as kdf import std.crypto.random as random +import std.net.tcp as tcp import std.net.tls as tls from std.io import TcpStream from std.iter import Iterator @@ -129,7 +130,7 @@ impl Conn: fn send_raw(data: Bytes) -> Int!: if self.secure: return tls.conn_from_handle(self.tls_handle).write_all_bytes(data)! - return tcp_write_bytes(self.fd, data)! + return tcp.write_all_bytes(self.fd, data)! # frame and send a message. a `type_byte` of 0 omits the type prefix, as the # startup message requires. @@ -198,7 +199,7 @@ pub fn connect_tls(host: String, port: Int, user: String, password: String, data # until the socket is wrapped in a Conn (or handed to the tls session), no # owner will close it, so every failure below closes the fd by hand. request := bytes.concat(be32(8), be32(80877103)) - sent := tcp_write_bytes(fd, request) + sent := tcp.write_all_bytes(fd, request) if sent.is_err: tcp_close(fd) fail sent.err diff --git a/std/redis.pith b/std/redis.pith index f517cc30..279ff8e7 100644 --- a/std/redis.pith +++ b/std/redis.pith @@ -9,10 +9,12 @@ # as an error rather than as a string quietly filled with replacement # characters. reach for a redis client that speaks bytes if you store blobs. # -# i/o goes straight to the tcp_* builtins, the same ones std.io wraps; the -# std.net.tcp module is a thin alias over them and isn't needed here. +# reads go straight to the tcp_* builtins, the same ones std.io wraps. writes +# go through std.net.tcp's write_all, because a single write syscall stops once +# the send buffer is full and a large command has to resume from there. import std.bytes as bytes +import std.net.tcp as tcp # replies come off an untrusted socket, so the length and count fields a server # advertises are bounded before we act on them: a hostile or buggy peer must not @@ -200,6 +202,12 @@ impl Client: if depth > MAX_REPLY_DEPTH: fail "redis: reply nesting too deep" line := self.read_line()! + # read_line returns what came before the terminator, so a bare CRLF + # arrives as an empty line. indexing a String is strict — line[0] on an + # empty one stops the process — and a malformed reply must fail the call + # rather than take the program down with it. + if line.len() == 0: + fail "redis: empty reply line" tag := line[0] body := line.substring(1, line.len()) if tag == "+": @@ -236,7 +244,7 @@ impl Client: mut out := "*{args.len()}\r\n" for a in args: out = out + "${a.len()}\r\n{a}\r\n" - tcp_write(self.fd, out)! + tcp.write_all(self.fd, out)! reply := self.read_reply()! if reply_is_error(reply): fail "redis: {reply_text(reply)}" diff --git a/std/web.pith b/std/web.pith index 561bc5fe..dbd28f9b 100644 --- a/std/web.pith +++ b/std/web.pith @@ -847,13 +847,13 @@ impl FrameStream: # (END_STREAM + END_HEADERS). :authority is a fixed "localhost" — the app does # not route on it. fn send_h2c_get(fd: Int, path: String) -> Int!: - tcp_write_bytes(fd, bytes.from_string_utf8(H2C_PREFACE))! + tcp.write_all_bytes(fd, bytes.from_string_utf8(H2C_PREFACE))! no_settings: List[frames.Setting] := [] - tcp_write_bytes(fd, frames.frame_bytes(frames.settings_frame(no_settings)!)!)! + tcp.write_all_bytes(fd, frames.frame_bytes(frames.settings_frame(no_settings)!)!)! encoder := hpack.new_encoder(hpack.DEFAULT_HEADER_TABLE_SIZE, false) fields := [hpack.header_field(":method", "GET"), hpack.header_field(":path", path), hpack.header_field(":scheme", "http"), hpack.header_field(":authority", "localhost")] block := encoder.encode(fields) - tcp_write_bytes(fd, frames.frame_bytes(frames.headers_frame(1, block, true, true))!)! + tcp.write_all_bytes(fd, frames.frame_bytes(frames.headers_frame(1, block, true, true))!)! return 0 # read response frames until stream 1 ends, returning the DATA payload as text. diff --git a/tests/cases/test_redis_bare_crlf.pith b/tests/cases/test_redis_bare_crlf.pith new file mode 100644 index 00000000..d56052d8 --- /dev/null +++ b/tests/cases/test_redis_bare_crlf.pith @@ -0,0 +1,80 @@ +# a malformed redis reply must fail the call, not end the process. +# +# read_line returns what came before the terminator, so a bare "\r\n" hands the +# parser an empty line. it then read line[0] to get the reply tag, and indexing +# a String in pith is strict: an out-of-range index prints a diagnostic and +# exits. a peer sending two bytes could therefore kill a client outright, while +# every other malformed shape in the same parser — an unknown tag, a bad length, +# a truncated payload — came back as an ordinary error. +# +# so what this checks is not only that the empty line is rejected, but that the +# process is still running afterwards and can be told the same thing again. + +import std.redis as redis +from std.redis import Reply +from std.collections import copy_list + +# answer each command with the next canned reply, whatever it is. no parsing: +# the client sends its commands in a fixed order, so canned replies line up. +fn serve(listener: Int, script: List[String]) -> Int!: + conn := tcp_accept(listener)! + for reply in script: + request := tcp_read(conn, 4096)! + if request.len() == 0: + tcp_close(conn) + return 0 + wrote := tcp_write(conn, reply) catch 0 + if wrote == 0: + tcp_close(conn) + return 0 + tcp_close(conn) + return 0 + +fn describe(reply: Reply) -> String: + return match reply: + Reply.Status(s) => "status:" + s + Reply.Error(s) => "error:" + s + Reply.Integer(n) => "int:" + n.to_string() + Reply.Bulk(s) => "bulk:" + s + Reply.Nil => "nil" + Reply.Array(items) => "array:" + items.len().to_string() + +# run one script against a fresh stub and render one line per command. reaching +# the end of this function at all is the point: a reply that stopped the process +# would print nothing after the command that carried it. +fn run(port: Int, script: List[String]) -> List[String]!: + listener := tcp_listen("127.0.0.1", port)! + task := spawn serve(listener, copy_list(script)) + client := redis.connect("127.0.0.1", port)! + mut out: List[String] := [] + mut i := 0 + while i < script.len(): + reply := client.command(["PROBE", i.to_string()]) + if reply.is_err: + out.push("failed:" + reply.err) + else: + out.push(describe(reply.ok)) + i = i + 1 + client.close() + await task + tcp_close(listener) + return out + +fn main() -> Int!: + # a bare terminator, on its own and after a reply that parsed fine. a fresh + # connection each time, since a rejected reply leaves the rest of the stream + # unread and nothing can follow it on the same socket. + for line in run(17540, ["\r\n"])!: + print(line) + for line in run(17541, ["+PONG\r\n"])!: + print(line) + for line in run(17542, ["\n"])!: + print(line) + + # an empty line nested inside an array reply takes the same path one level + # down, and must be reported the same way. + for line in run(17543, ["*2\r\n\r\n:1\r\n"])!: + print(line) + + print("the client is still running") + return 0 diff --git a/tests/cases/test_tcp_write_all.pith b/tests/cases/test_tcp_write_all.pith new file mode 100644 index 00000000..045778dc --- /dev/null +++ b/tests/cases/test_tcp_write_all.pith @@ -0,0 +1,160 @@ +# a write larger than the socket send buffer has to finish, not stop half way. +# +# tcp_write is one write(2). the kernel takes what fits in the send buffer and +# returns that count; the rest is not queued anywhere and is simply not sent. +# a caller that ignores the count drops the tail, and the peer waits forever for +# bytes that were never written — which is why a 1 MiB redis SET worked and a +# 4 MiB one hung. +# +# every case here sends a payload several times the send buffer to a reader that +# deliberately drains slower than the writer fills, which is what forces the +# short write. the delay between chunks is the mechanism under test, not a +# startup wait: the listener is always bound before anything connects to it. +# +# the non-ascii case is the one worth reading twice. the send buffer fills at +# whatever byte offset it fills at, which may be in the middle of a multi-byte +# character, and a String cannot be cut there — resuming as text stopped the +# process outright. the one-byte ascii prefix on that payload guarantees the +# boundary misses a character start. + +import std.redis as redis +import std.net.tcp as tcp +import std.time as time +from std.io import TcpStream + +# how much the stub takes per read, and the pause after each one. together they +# hold the reader well under the writer, so the send buffer is full for most of +# the transfer. +DRAIN_CHUNK := 65536 +DRAIN_PAUSE := 1 + +# grow a seed by doubling until it covers `size` bytes. doubling keeps building +# a multi-megabyte payload cheap; repeated appends would copy the whole thing +# once per append. +fn payload_of(seed: String, size: Int) -> String: + mut out := seed + while out.len() < size: + out = out + out + return out.substring(0, size) + +# accept one connection, then read it dry a chunk at a time, reporting the total. +# reading into Bytes rather than String keeps the count honest: a String read +# that splits a multi-byte character substitutes a replacement character and the +# byte count stops matching what was sent. +fn drain(listener: Int, label: String) -> Int!: + conn := tcp_accept(listener)! + mut got := 0 + while true: + chunk := tcp_read_bytes(conn, DRAIN_CHUNK)! + if chunk.len() == 0: + break + got = got + chunk.len() + time.delay(DRAIN_PAUSE) + print("{label}: read {got}") + tcp_close(conn) + return 0 + +# write `payload` with the fd-level helper and report what it returned. +fn round_trip_fd(port: Int, label: String, payload: String) -> Int!: + listener := tcp_listen("127.0.0.1", port)! + task := spawn drain(listener, label) + fd := tcp_connect("127.0.0.1", port)! + wrote := tcp.write_all(fd, payload)! + print("{label}: write_all returned {wrote}") + tcp_close(fd) + await task + tcp_close(listener) + return 0 + +# the same, through std.io's TcpStream.write_all — the buffered-writer layer +# sits on this one. +fn round_trip_stream(port: Int, label: String, payload: String) -> Int!: + listener := tcp_listen("127.0.0.1", port)! + task := spawn drain(listener, label) + stream := TcpStream(tcp_connect("127.0.0.1", port)!) + wrote := stream.write_all(payload)! + print("{label}: write_all returned {wrote}") + stream.close() + await task + tcp_close(listener) + return 0 + +# a stub redis that reads a command of a known length and answers +OK, draining +# as slowly as the others so the client's request is short-written. +fn stub_redis(listener: Int, want: Int) -> Int!: + conn := tcp_accept(listener)! + mut got := 0 + while got < want: + chunk := tcp_read_bytes(conn, DRAIN_CHUNK)! + if chunk.len() == 0: + print("redis stub: peer stopped after {got} of {want}") + tcp_close(conn) + return 0 + got = got + chunk.len() + time.delay(DRAIN_PAUSE) + print("redis stub: read the whole {got}-byte command") + tcp.write_all(conn, "+OK\r\n")! + tcp_close(conn) + return 0 + +# the exact wire length of `SET big ` as a RESP array of bulk strings. +fn set_command_len(key: String, value: String) -> Int: + header := "*3\r\n$3\r\nSET\r\n" + key_part := "${key.len()}\r\n{key}\r\n" + value_part := "${value.len()}\r\n{value}\r\n" + return header.len() + key_part.len() + value_part.len() + +fn redis_set(port: Int, value: String) -> Int!: + listener := tcp_listen("127.0.0.1", port)! + task := spawn stub_redis(listener, set_command_len("big", value)) + client := redis.connect("127.0.0.1", port)! + stored := client.set("big", value) + if stored.is_err: + print("redis SET: failed: {stored.err}") + else: + print("redis SET: stored={stored.ok}") + client.close() + await task + tcp_close(listener) + return 0 + +# a peer that hangs up without reading: the write must report the close rather +# than claim it sent everything, and must not take the process down with it. +fn hang_up(listener: Int) -> Int!: + conn := tcp_accept(listener)! + tcp_close(conn) + return 0 + +fn write_to_a_gone_peer(port: Int, payload: String) -> Int!: + listener := tcp_listen("127.0.0.1", port)! + task := spawn hang_up(listener) + fd := tcp_connect("127.0.0.1", port)! + await task + result := tcp.write_all(fd, payload) + if result.is_err: + print("closed peer: failed as it should") + else: + print("closed peer: claimed {result.ok} bytes went out") + tcp_close(fd) + tcp_close(listener) + return 0 + +fn main() -> Int!: + # 4 MiB of ascii: the size the redis report named. + ascii := payload_of("abcdefgh", 4 * 1024 * 1024) + print("ascii payload: {ascii.len()} bytes") + round_trip_fd(17530, "fd ascii", ascii)! + redis_set(17531, ascii)! + + # 3 MiB of three-byte characters behind a single ascii byte, so the first + # short write cannot land on a character start. + wide := "x" + payload_of("€€€€€€€€", 3 * 1024 * 1024) + print("wide payload: {wide.len()} bytes") + round_trip_fd(17532, "fd wide", wide)! + round_trip_stream(17533, "stream wide", wide)! + + write_to_a_gone_peer(17534, ascii)! + + # the empty write is a no-op, not a failure, and not a closed-peer report. + print("empty: {tcp.write_all(-1, "") catch -1}") + return 0 diff --git a/tests/expected/test_redis_bare_crlf.txt b/tests/expected/test_redis_bare_crlf.txt new file mode 100644 index 00000000..7cfcbdc6 --- /dev/null +++ b/tests/expected/test_redis_bare_crlf.txt @@ -0,0 +1,5 @@ +failed:redis: empty reply line +status:PONG +failed:redis: connection closed +failed:redis: empty reply line +the client is still running diff --git a/tests/expected/test_tcp_write_all.txt b/tests/expected/test_tcp_write_all.txt new file mode 100644 index 00000000..64b7805f --- /dev/null +++ b/tests/expected/test_tcp_write_all.txt @@ -0,0 +1,12 @@ +ascii payload: 4194304 bytes +fd ascii: write_all returned 4194304 +fd ascii: read 4194304 +redis stub: read the whole 4194338-byte command +redis SET: stored=true +wide payload: 3145729 bytes +fd wide: write_all returned 3145729 +fd wide: read 3145729 +stream wide: write_all returned 3145729 +stream wide: read 3145729 +closed peer: failed as it should +empty: 0