From 2c0513782078be37de9aefe885268547fc3647b5 Mon Sep 17 00:00:00 2001 From: Robin Owens Date: Wed, 29 Jul 2026 15:31:50 +0100 Subject: [PATCH 1/2] Preserve the bytes on one-shot TCP reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Tcp.send` decodes its response with `String::from_utf8_lossy`, so every non-UTF-8 sequence becomes U+FFFD. The substitution is many-to-one — 128 of the 256 single-byte values collapse to the same replacement character — so it cannot be undone, and it is silent: the call still returns `Result.Ok`, and `String.len` reports what an uncorrupted read would. That puts every binary protocol out of reach. Add `Tcp.sendBytes : (String, Int, List) -> Result, String>` alongside it. Same socket behaviour as `Tcp.send` — open, write, shutdown(Write), read to EOF, same cap, timeouts and port validation — but payload and response stay bytes end to end. Additive rather than a change in place: reshaping `Tcp.send` would break every existing caller, and erroring on invalid UTF-8 would regress the ones already living with lossy text. Byte values outside 0..=255 return a catchable `Result.Err` naming the value and its index, matching how the port range is handled rather than trapping. A non-`List` payload stays a static type error. Wired through the VM path, the type checker, and the effect classification table, so Oracle `verify trace` can stub it. The codegen backends are not wired yet. Co-Authored-By: Claude Opus 5 --- aver-rt/src/tcp.rs | 29 ++++++ docs/services.md | 10 ++ src/services/tcp.rs | 107 ++++++++++++++++++++- src/types/checker/builtins.rs | 10 ++ src/types/checker/effect_classification.rs | 15 +++ src/vm/builtin.rs | 3 + tests/eval_spec.rs | 69 +++++++++++++ tools/website/llms.txt | 2 +- 8 files changed, 240 insertions(+), 5 deletions(-) diff --git a/aver-rt/src/tcp.rs b/aver-rt/src/tcp.rs index 08f1ba52f..8062abe84 100644 --- a/aver-rt/src/tcp.rs +++ b/aver-rt/src/tcp.rs @@ -130,6 +130,35 @@ pub fn send(host: &str, port: i64, message: &str) -> Result { Ok(String::from_utf8_lossy(&buf).into_owned()) } +/// Byte-clean sibling of [`send`]. +/// +/// Identical socket behaviour — open, write, `shutdown(Write)`, read to EOF — +/// but the payload and the response stay `Vec` end to end. `send` converts +/// the response with `String::from_utf8_lossy`, which replaces every non-UTF-8 +/// sequence with U+FFFD and cannot be undone; that makes it unusable for binary +/// protocols whose framing bytes are not valid UTF-8. This function performs no +/// encoding or decoding, so the caller sees exactly what the peer sent. +pub fn send_bytes(host: &str, port: i64, payload: &[u8]) -> Result, String> { + validate_port(port)?; + let socket_addr = resolve(&format!("{}:{}", host, port))?; + let mut stream = + TcpStream::connect_timeout(&socket_addr, CONNECT_TIMEOUT).map_err(|e| e.to_string())?; + stream.set_read_timeout(Some(IO_TIMEOUT)).ok(); + stream.set_write_timeout(Some(IO_TIMEOUT)).ok(); + stream.write_all(payload).map_err(|e| e.to_string())?; + stream.shutdown(std::net::Shutdown::Write).ok(); + + let mut buf = Vec::new(); + Read::by_ref(&mut stream) + .take(BODY_LIMIT as u64 + 1) + .read_to_end(&mut buf) + .map_err(|e| e.to_string())?; + if buf.len() > BODY_LIMIT { + return Err("Tcp.sendBytes: response exceeds 10 MB limit".to_string()); + } + Ok(buf) +} + pub fn ping(host: &str, port: i64) -> Result<(), String> { validate_port(port)?; let socket_addr = resolve(&format!("{}:{}", host, port))?; diff --git a/docs/services.md b/docs/services.md index 267a7cd81..8ec18374c 100644 --- a/docs/services.md +++ b/docs/services.md @@ -257,6 +257,7 @@ Source: `src/services/tcp.rs` | Function | Signature | |---|---| | `Tcp.send` | `(String, Int, String) -> Result` | +| `Tcp.sendBytes` | `(String, Int, List) -> Result, String>` | | `Tcp.ping` | `(String, Int) -> Result` | **Persistent connections:** @@ -272,6 +273,15 @@ Source: `src/services/tcp.rs` `Tcp.send` is stateless and ephemeral — it opens a fresh socket, writes the request bytes raw (no `\r\n` append), `shutdown(Write)` to signal end-of-request, then reads the peer's response until EOF, capped at 10 MiB. It does **not** touch the persistent-connection pool, so a program holding 256 live `Tcp.connect` handles can still issue `Tcp.send` to another peer. Stream errors (`stream-error.last-operation-failed`) surface as `Result.Err("tcp: stream error")`; a clean half-close (`stream-error.closed`) returns whatever the peer flushed. +`Tcp.sendBytes` is the byte-clean form of `Tcp.send`: same socket behaviour, but +the payload and response stay `List` and no UTF-8 encoding or decoding +happens in either direction. Prefer it for any binary protocol. `Tcp.send` +decodes the response with `String::from_utf8_lossy`, which replaces every +non-UTF-8 sequence with U+FFFD — silently, irreversibly, and starting at the +first offending byte — so it is only safe for protocols whose responses are +valid UTF-8 text. Payload values outside `0..=255` return +`Result.Err` naming the offending value and its index. + ### `Random` namespace — use granular effects (`! [Random.int]`, `! [Random.float]`) Source: `src/services/random.rs` (backed by `aver_rt::random`) diff --git a/src/services/tcp.rs b/src/services/tcp.rs index 9b53520cc..184b0a3e6 100644 --- a/src/services/tcp.rs +++ b/src/services/tcp.rs @@ -2,6 +2,10 @@ /// /// One-shot methods: /// `Tcp.send(host, port, message)` — connect, write message, read response, close. +/// `Tcp.sendBytes(host, port, payload)` — same, but byte-clean: `List` in, +/// `List` out, no UTF-8 encoding or decoding on either side. `send` +/// decodes the response with `String::from_utf8_lossy`, which destroys any +/// non-UTF-8 byte irrecoverably; binary protocols need this variant. /// `Tcp.ping(host, port)` — check whether the port accepts connections. /// /// Persistent-connection methods: @@ -14,14 +18,22 @@ use std::collections::HashMap; use std::sync::Arc as Rc; -use aver_rt::TcpConnection; +use aver_rt::{AverList, TcpConnection}; use crate::nan_value::{Arena, NanValue, NanValueConvert}; use crate::value::{RuntimeError, Value}; pub fn register(global: &mut HashMap) { let mut members = HashMap::new(); - for method in &["send", "ping", "connect", "writeLine", "readLine", "close"] { + for method in &[ + "send", + "sendBytes", + "ping", + "connect", + "writeLine", + "readLine", + "close", + ] { members.insert( method.to_string(), Value::Builtin(format!("Tcp.{}", method)), @@ -38,6 +50,7 @@ pub fn register(global: &mut HashMap) { pub const DECLARED_EFFECTS: &[&str] = &[ "Tcp.send", + "Tcp.sendBytes", "Tcp.ping", "Tcp.connect", "Tcp.writeLine", @@ -48,6 +61,7 @@ pub const DECLARED_EFFECTS: &[&str] = &[ pub fn effects(name: &str) -> &'static [&'static str] { match name { "Tcp.send" => &["Tcp.send"], + "Tcp.sendBytes" => &["Tcp.sendBytes"], "Tcp.ping" => &["Tcp.ping"], "Tcp.connect" => &["Tcp.connect"], "Tcp.writeLine" => &["Tcp.writeLine"], @@ -61,6 +75,7 @@ pub fn effects(name: &str) -> &'static [&'static str] { pub fn call(name: &str, args: &[Value]) -> Option> { match name { "Tcp.send" => Some(tcp_send(args)), + "Tcp.sendBytes" => Some(tcp_send_bytes(args)), "Tcp.ping" => Some(tcp_ping(args)), "Tcp.connect" => Some(tcp_connect(args)), "Tcp.writeLine" => Some(tcp_write_line(args)), @@ -87,6 +102,29 @@ fn tcp_send(args: &[Value]) -> Result { } } +fn tcp_send_bytes(args: &[Value]) -> Result { + if args.len() != 3 { + return Err(RuntimeError::Error(format!( + "Tcp.sendBytes() takes 3 arguments (host, port, payload), got {}", + args.len() + ))); + } + let host = str_arg(&args[0], "Tcp.sendBytes: host must be a String")?; + let port = int_arg(&args[1], "Tcp.sendBytes: port must be an Int")?; + let payload = match bytes_arg(&args[2], "Tcp.sendBytes")? { + Ok(bytes) => bytes, + // Out-of-range byte values are a value error, not a type error, so they + // surface as a catchable `Result.Err` — same treatment the port range + // gets in `aver-rt::tcp` rather than a VM-only trap. + Err(msg) => return Ok(Value::Err(Box::new(Value::Str(msg)))), + }; + + match aver_rt::tcp::send_bytes(&host, port, &payload) { + Ok(response) => Ok(Value::Ok(Box::new(bytes_to_value(&response)))), + Err(e) => Ok(Value::Err(Box::new(Value::Str(e)))), + } +} + fn tcp_ping(args: &[Value]) -> Result { if args.len() != 2 { return Err(RuntimeError::Error(format!( @@ -225,6 +263,53 @@ fn str_arg(val: &Value, msg: &str) -> Result { } } +/// Convert a `List` argument into raw bytes. +/// +/// The outer `Result` is the type check (wrong shape is a `RuntimeError`); the +/// inner one is the value check (an Int outside `0..=255` is a catchable Aver +/// `Result.Err`, reported with its index so a long payload is debuggable). +#[allow(clippy::type_complexity)] +fn bytes_arg(val: &Value, method: &str) -> Result, String>, RuntimeError> { + let items = match val { + Value::List(items) => items, + _ => { + return Err(RuntimeError::Error(format!( + "{}: payload must be a List", + method + ))); + } + }; + let mut out = Vec::with_capacity(items.len()); + for (idx, item) in items.iter().enumerate() { + let n = match item { + Value::Int(n) => n.to_i64().ok_or_else(|| { + RuntimeError::Error(format!("{}: payload must be a List", method)) + })?, + _ => { + return Err(RuntimeError::Error(format!( + "{}: payload must be a List", + method + ))); + } + }; + match u8::try_from(n) { + Ok(b) => out.push(b), + Err(_) => { + return Ok(Err(format!( + "{}: byte {} at index {} is out of range (0\u{2013}255)", + method, n, idx + ))); + } + } + } + Ok(Ok(out)) +} + +fn bytes_to_value(bytes: &[u8]) -> Value { + let items: Vec = bytes.iter().map(|b| Value::int(*b as i64)).collect(); + Value::List(AverList::from_vec(items)) +} + fn int_arg(val: &Value, msg: &str) -> Result { // Phase 4.7+ fix #13 — type check only; the port-range check // moved into `aver-rt::tcp::{connect, send, ping}` so every @@ -242,7 +327,15 @@ fn int_arg(val: &Value, msg: &str) -> Result { // ─── NanValue-native API ───────────────────────────────────────────────────── pub fn register_nv(global: &mut HashMap, arena: &mut Arena) { - let methods = &["send", "ping", "connect", "writeLine", "readLine", "close"]; + let methods = &[ + "send", + "sendBytes", + "ping", + "connect", + "writeLine", + "readLine", + "close", + ]; let mut members: Vec<(Rc, NanValue)> = Vec::with_capacity(methods.len()); for method in methods { let idx = arena.push_builtin(&format!("Tcp.{}", method)); @@ -263,7 +356,13 @@ pub fn call_nv( ) -> Option> { if !matches!( name, - "Tcp.send" | "Tcp.ping" | "Tcp.connect" | "Tcp.writeLine" | "Tcp.readLine" | "Tcp.close" + "Tcp.send" + | "Tcp.sendBytes" + | "Tcp.ping" + | "Tcp.connect" + | "Tcp.writeLine" + | "Tcp.readLine" + | "Tcp.close" ) { return None; } diff --git a/src/types/checker/builtins.rs b/src/types/checker/builtins.rs index aff04219b..e972145e5 100644 --- a/src/types/checker/builtins.rs +++ b/src/types/checker/builtins.rs @@ -128,6 +128,7 @@ impl TypeChecker { "Env.set".to_string(), "Tcp.connect".to_string(), "Tcp.send".to_string(), + "Tcp.sendBytes".to_string(), "Tcp.ping".to_string(), "Tcp.writeLine".to_string(), "Tcp.readLine".to_string(), @@ -287,6 +288,15 @@ impl TypeChecker { Type::Result(Box::new(Type::Str), Box::new(Type::Str)), &["Tcp.send"], ), + ( + "Tcp.sendBytes", + &[Type::Str, Type::Int, Type::List(Box::new(Type::Int))], + Type::Result( + Box::new(Type::List(Box::new(Type::Int))), + Box::new(Type::Str), + ), + &["Tcp.sendBytes"], + ), ( "Tcp.ping", &[Type::Str, Type::Int], diff --git a/src/types/checker/effect_classification.rs b/src/types/checker/effect_classification.rs index bde9a254c..d3be5a824 100644 --- a/src/types/checker/effect_classification.rs +++ b/src/types/checker/effect_classification.rs @@ -80,6 +80,10 @@ pub enum RuntimeType { TcpConnection, /// `Result` — return of `Tcp.connect`. ResultTcpConnectionStr, + /// `List` — the byte payload argument on `Tcp.sendBytes`. + ListInt, + /// `Result, Str>` — return of `Tcp.sendBytes`. + ResultListIntStr, } impl RuntimeType { @@ -111,6 +115,11 @@ impl RuntimeType { RuntimeType::ResultTcpConnectionStr => { Type::Result(Box::new(Type::named("Tcp.Connection")), Box::new(Type::Str)) } + RuntimeType::ListInt => Type::List(Box::new(Type::Int)), + RuntimeType::ResultListIntStr => Type::Result( + Box::new(Type::List(Box::new(Type::Int))), + Box::new(Type::Str), + ), } } } @@ -284,6 +293,12 @@ const CLASSIFICATIONS: &[EffectClassification] = &[ runtime_params: &[RuntimeType::Str, RuntimeType::Int, RuntimeType::Str], runtime_return: RuntimeType::ResultStrStr, }, + EffectClassification { + method: "Tcp.sendBytes", + dimension: EffectDimension::GenerativeOutput, + runtime_params: &[RuntimeType::Str, RuntimeType::Int, RuntimeType::ListInt], + runtime_return: RuntimeType::ResultListIntStr, + }, EffectClassification { method: "Tcp.ping", dimension: EffectDimension::GenerativeOutput, diff --git a/src/vm/builtin.rs b/src/vm/builtin.rs index 0a6a59e9b..1c497da98 100644 --- a/src/vm/builtin.rs +++ b/src/vm/builtin.rs @@ -67,6 +67,7 @@ vm_builtins! { RandomFloat => "Random.float", TcpSend => "Tcp.send", + TcpSendBytes => "Tcp.sendBytes", TcpPing => "Tcp.ping", TcpConnect => "Tcp.connect", TcpWriteLine => "Tcp.writeLine", @@ -274,6 +275,7 @@ impl VmBuiltin { Self::RandomInt | Self::RandomFloat => random::effects(self.name()), Self::TcpSend + | Self::TcpSendBytes | Self::TcpPing | Self::TcpConnect | Self::TcpWriteLine @@ -358,6 +360,7 @@ impl VmBuiltin { Self::RandomInt | Self::RandomFloat => random::call_nv(self.name(), args, arena), Self::TcpSend + | Self::TcpSendBytes | Self::TcpPing | Self::TcpConnect | Self::TcpWriteLine diff --git a/tests/eval_spec.rs b/tests/eval_spec.rs index 0e9960f8c..74a054776 100644 --- a/tests/eval_spec.rs +++ b/tests/eval_spec.rs @@ -2298,6 +2298,75 @@ mod tcp_tests { other => panic!("expected Ok(\"echo me\"), got {:?}", other), } } + + /// Regression: `Tcp.send` decodes the response with + /// `String::from_utf8_lossy`, so every non-UTF-8 byte comes back as U+FFFD + /// and the original is unrecoverable. `Tcp.sendBytes` must round-trip the + /// bytes untouched. The payload here is the Bitcoin mainnet magic + /// (`F9 BE B4 D9`) — four bytes that are each invalid UTF-8 for a different + /// reason, so a regression on any decode path shows up here. + #[test] + #[ignore = "integration: starts a local TCP server; run with --include-ignored --test-threads=1"] + fn tcp_send_bytes_round_trips_non_utf8() { + use std::io::{Read, Write}; + use std::net::TcpListener; + use std::thread; + + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut buf = Vec::new(); + stream.read_to_end(&mut buf).ok(); + stream.write_all(&buf).ok(); + } + }); + + let src = format!( + "fn talk() -> Result, String>\n ! [Tcp.sendBytes]\n Tcp.sendBytes(\"127.0.0.1\", {}, [249, 190, 180, 217])\n", + port + ); + match run_tcp_fn(&src, "talk") { + Value::Ok(inner) => match *inner { + Value::List(items) => { + let got: Vec = items + .iter() + .map(|v| match v { + Value::Int(n) => n.to_i64().unwrap(), + other => panic!("expected Int element, got {:?}", other), + }) + .collect(); + assert_eq!(got, vec![249, 190, 180, 217]); + } + other => panic!("expected List, got {:?}", other), + }, + other => panic!("expected Ok(List), got {:?}", other), + } + } + + /// Byte values outside `0..=255` are a value error, not a type error, so + /// they surface as a catchable `Result.Err` rather than a VM trap — the + /// same treatment the port range gets. + #[test] + fn tcp_send_bytes_rejects_out_of_range_byte() { + let src = concat!( + "fn talk() -> Result, String>\n", + " ! [Tcp.sendBytes]\n", + " Tcp.sendBytes(\"127.0.0.1\", 1, [65, 256])\n", + ); + match run_tcp_fn(src, "talk") { + Value::Err(inner) => match *inner { + Value::Str(msg) => { + assert!( + msg.contains("256") && msg.contains("index 1"), + "error should name the offending byte and its index, got: {msg}" + ); + } + other => panic!("expected Str error, got {:?}", other), + }, + other => panic!("expected Err, got {:?}", other), + } + } } // --------------------------------------------------------------------------- diff --git a/tools/website/llms.txt b/tools/website/llms.txt index 31377dafc..c0e2241b6 100644 --- a/tools/website/llms.txt +++ b/tools/website/llms.txt @@ -333,7 +333,7 @@ Effectful namespaces: - `Console`: print, error, warn, readLine — **`print`/`error`/`warn` take `String`**, not arbitrary values. Stringify at the call site: interpolation `"{x}"` for primitives, a per-type render fn (`fn show(r: Result) -> String`) for compound shapes. - `Http`: get, post, put, patch, delete, head - `Disk`: readText, writeText, appendText, exists, delete, deleteDir, listDir, makeDir -- `Tcp`: connect, writeLine, readLine, close, send, ping +- `Tcp`: connect, writeLine, readLine, close, send, sendBytes, ping — `send`/`readLine` are text-only (UTF-8); use `sendBytes` (`List` in and out) for binary protocols - `Terminal`: enableRawMode, readKey, setCursor, print, clear, size — `Terminal.print` and `Terminal.setColor` also take `String`. - `Time`: now, unixMs, sleep - `Env`: get, set From 346f3f7f60818b07dfe3590a93097c92f2ea5454 Mon Sep 17 00:00:00 2001 From: jasisz Date: Sat, 1 Aug 2026 20:53:17 +0200 Subject: [PATCH 2/2] Treat Ints outside i64 as out-of-range bytes in Tcp.sendBytes --- src/services/tcp.rs | 15 ++++++++++++--- tests/eval_spec.rs | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/services/tcp.rs b/src/services/tcp.rs index 184b0a3e6..8b39f1aff 100644 --- a/src/services/tcp.rs +++ b/src/services/tcp.rs @@ -282,9 +282,18 @@ fn bytes_arg(val: &Value, method: &str) -> Result, String>, Runti let mut out = Vec::with_capacity(items.len()); for (idx, item) in items.iter().enumerate() { let n = match item { - Value::Int(n) => n.to_i64().ok_or_else(|| { - RuntimeError::Error(format!("{}: payload must be a List", method)) - })?, + Value::Int(n) => match n.to_i64() { + Some(n) => n, + // An `Int` outside `i64` is still an `Int` — a fortiori out + // of byte range, so it takes the catchable value-error path, + // not the type-error one. + None => { + return Ok(Err(format!( + "{}: byte {} at index {} is out of range (0\u{2013}255)", + method, n, idx + ))); + } + }, _ => { return Err(RuntimeError::Error(format!( "{}: payload must be a List", diff --git a/tests/eval_spec.rs b/tests/eval_spec.rs index 74a054776..5f8f9a0af 100644 --- a/tests/eval_spec.rs +++ b/tests/eval_spec.rs @@ -2367,6 +2367,30 @@ mod tcp_tests { other => panic!("expected Err, got {:?}", other), } } + + /// An `Int` outside `i64` is still an `Int` — a fortiori out of byte + /// range, so it must take the same catchable value-error path as `256`, + /// not trap as a bogus type error. + #[test] + fn tcp_send_bytes_rejects_bignum_byte() { + let src = concat!( + "fn talk() -> Result, String>\n", + " ! [Tcp.sendBytes]\n", + " Tcp.sendBytes(\"127.0.0.1\", 1, [65, 1208925819614629174706176])\n", + ); + match run_tcp_fn(src, "talk") { + Value::Err(inner) => match *inner { + Value::Str(msg) => { + assert!( + msg.contains("1208925819614629174706176") && msg.contains("index 1"), + "error should name the offending byte and its index, got: {msg}" + ); + } + other => panic!("expected Str error, got {:?}", other), + }, + other => panic!("expected Err, got {:?}", other), + } + } } // ---------------------------------------------------------------------------