From 842e8c327d3d7e1727b6b23ac964afc78a733630 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 6 Jul 2026 21:20:18 +0200 Subject: [PATCH 1/4] feat(net-http): keep-alive + timeouts + proxy Extend the Net::HTTP transport binding with the deferred features, built on the existing socket/TLS transport: - Keep-alive: within Net::HTTP.start's block a single connection is reused across requests, each response framed by exact Content-Length / chunked encoding so the next request reads at the right offset; the connection is closed on #finish / block end, or when the server sends Connection: close. - Timeouts: open_timeout / read_timeout / write_timeout accessors set socket deadlines (open_timeout bounds the dial + TLS handshake); a timed-out open/read/write raises Net::OpenTimeout / Net::ReadTimeout / Net::WriteTimeout as MRI does. - Proxy: Net::HTTP.new(host, port, p_addr, p_port[, user, pass]) and the matching Net::HTTP.start proxy args; plain http emits an absolute-URI request-line to the proxy, https tunnels via CONNECT then TLS through it. proxy? / proxy_address / proxy_port / proxy_user / proxy_pass accessors. Co-Authored-By: Claude Opus 4.8 --- internal/vm/nethttp_bind.go | 563 +++++++++++++++++++++++++++++++++++- 1 file changed, 555 insertions(+), 8 deletions(-) diff --git a/internal/vm/nethttp_bind.go b/internal/vm/nethttp_bind.go index 6b4ee458..74b57603 100644 --- a/internal/vm/nethttp_bind.go +++ b/internal/vm/nethttp_bind.go @@ -8,11 +8,14 @@ import ( "bufio" "bytes" "crypto/tls" + "encoding/base64" + "fmt" "io" "net" "net/url" "strconv" "strings" + "time" nethttp "github.com/go-ruby-net-http/net-http" @@ -52,6 +55,11 @@ func (vm *VM) registerNetHTTPTransport() { // Net::HTTPBadResponse class a `rescue Net::HTTPBadResponse` can catch. vm.consts["Net::HTTPError"] = netMod.consts["HTTPError"] vm.consts["Net::HTTPBadResponse"] = netMod.consts["HTTPBadResponse"] + // Publish the timeout classes too, so a raise("Net::ReadTimeout", ...) from the + // transport resolves the real class a `rescue Net::ReadTimeout` catches. + vm.consts["Net::OpenTimeout"] = netMod.consts["OpenTimeout"] + vm.consts["Net::ReadTimeout"] = netMod.consts["ReadTimeout"] + vm.consts["Net::WriteTimeout"] = netMod.consts["WriteTimeout"] vm.registerNetHTTPClassMethods(http) vm.registerNetHTTPInstanceMethods(http) @@ -112,7 +120,7 @@ func (vm *VM) registerNetHTTPClassMethods(http *RClass) { setIvar(inst, "@started", object.Bool(true)) if blk != nil { res := vm.callBlock(blk, []object.Value{inst}) - setIvar(inst, "@started", object.Bool(false)) + vm.nethttpFinish(inst) return res } return inst @@ -154,16 +162,46 @@ func (vm *VM) registerNetHTTPInstanceMethods(http *RClass) { setIvar(self, "@started", object.Bool(true)) if blk != nil { res := vm.callBlock(blk, []object.Value{self}) - setIvar(self, "@started", object.Bool(false)) + vm.nethttpFinish(self) return res } return self }) - http.define("finish", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { - setIvar(self, "@started", object.Bool(false)) + http.define("finish", func(vm *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + vm.nethttpFinish(self) return object.NilV }) + // --- timeout accessors (open/read/write); values are seconds (Integer/Float), + // nil disables the deadline. MRI defaults each to 60. ----------------------- + for _, name := range []string{"open_timeout", "read_timeout", "write_timeout"} { + ivar := "@" + name + http.define(name, func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return getIvar(self, ivar) + }) + http.define(name+"=", func(_ *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + setIvar(self, ivar, args[0]) + return args[0] + }) + } + + // --- proxy accessors -------------------------------------------------------- + http.define("proxy?", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return object.Bool(!object.IsNil(getIvar(self, "@proxy_address"))) + }) + http.define("proxy_address", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return getIvar(self, "@proxy_address") + }) + http.define("proxy_port", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return getIvar(self, "@proxy_port") + }) + http.define("proxy_user", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return getIvar(self, "@proxy_user") + }) + http.define("proxy_pass", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return getIvar(self, "@proxy_pass") + }) + // Instance verb helpers: get/head/delete/options take (path[, header]); the // body-carrying verbs post/put/patch take (path, data[, header]). for _, verb := range []string{"get", "head", "post", "put", "delete", "patch", "options"} { @@ -312,7 +350,9 @@ func nethttpSchemePort(u *url.URL) (scheme, port string) { } // nethttpExecInstance runs a request using a Net::HTTP instance's configured -// address / port / use_ssl / verify_mode. +// address / port / use_ssl / verify_mode / timeouts / proxy. Within a start block +// (@started) it keeps one connection alive across requests; otherwise it opens +// and closes a connection per request. func (vm *VM) nethttpExecInstance(inst object.Value, method, path string, body []byte, hdr [][2]string) object.Value { host := strArg(getIvar(inst, "@address")) port := intArg(getIvar(inst, "@port")) @@ -329,7 +369,485 @@ func (vm *VM) nethttpExecInstance(inst object.Value, method, path string, body [ if v, ok := getIvar(inst, "@verify_mode").(object.Integer); ok { verify = int64(v) } - return vm.nethttpDo(scheme, host, portStr, hostHdr, method, path, body, hdr, verify) + cfg := &nethttpXfer{ + scheme: scheme, host: host, port: portStr, hostHdr: hostHdr, + verifyMode: verify, + openTO: nethttpDuration(getIvar(inst, "@open_timeout")), + readTO: nethttpDuration(getIvar(inst, "@read_timeout")), + writeTO: nethttpDuration(getIvar(inst, "@write_timeout")), + } + cfg.dialHost, cfg.dialPort = host, portStr + if pa := getIvar(inst, "@proxy_address"); !object.IsNil(pa) { + cfg.proxied = true + cfg.dialHost = strArg(pa) + cfg.dialPort = portString(getIvar(inst, "@proxy_port")) + cfg.connectTunnel = scheme == "https" + if u := getIvar(inst, "@proxy_user"); !object.IsNil(u) { + cfg.proxyAuth = basicAuthHeader(strArg(u), strArg(getIvar(inst, "@proxy_pass"))) + } + } + if getIvar(inst, "@started").Truthy() { + cfg.inst = inst + } + return vm.nethttpDoXfer(cfg, method, path, body, hdr) +} + +// nethttpXfer carries the resolved per-request transport configuration for an +// instance request: the final target (scheme/host/port/hostHdr), the TLS verify +// mode, the actual dial endpoint (dialHost/dialPort — the proxy when proxied), +// the proxy shape, the socket deadlines and, when set, the started instance whose +// connection is reused across requests (keep-alive). +type nethttpXfer struct { + scheme, host, port, hostHdr string + verifyMode int64 + dialHost, dialPort string + proxied bool // plain-http proxy: absolute-form request-line + connectTunnel bool // https-via-proxy: CONNECT then TLS through the tunnel + proxyAuth string // Proxy-Authorization value, or "" for none + openTO, readTO, writeTO time.Duration + inst object.Value // non-nil ⇒ persistent (keep-alive) on this instance +} + +// nethttpDoXfer runs one instance request over the configured transport. Within a +// start block it reuses (or opens and caches) a persistent connection and frames +// exactly one response so the next request reads at the right offset; outside a +// start block it opens a fresh connection, reads one response and closes. +func (vm *VM) nethttpDoXfer(cfg *nethttpXfer, method, path string, body []byte, hdr [][2]string) object.Value { + reqBytes, noBody := vm.nethttpBuildRequest(cfg, method, path, body, hdr, cfg.inst == nil) + + persistent := cfg.inst != nil + // Reuse a cached connection once; if the write/read fails (e.g. the server + // dropped an idle keep-alive), redial and retry a single time. + var raw []byte + var err error + var keepAlive bool + reused := false + if persistent { + if s := nethttpGetConn(cfg.inst); s != nil { + reused = true + raw, keepAlive, err = vm.nethttpExchangeFramed(cfg, s, reqBytes, noBody) + if err != nil { + nethttpDropConn(cfg.inst) + reused = false + } + } + } + if !reused { + stream, derr := vm.nethttpDialXfer(cfg) + if derr != nil { + vm.raiseTransportErr(derr, "open") + } + raw, keepAlive, err = vm.nethttpExchangeFramed(cfg, stream, reqBytes, noBody) + if err != nil { + stream.closeConn() + vm.raiseTransportErr(err, "read") + } + if persistent { + nethttpSetConn(cfg.inst, stream) + } else { + stream.closeConn() + } + } + // A server "Connection: close" (or an unframed body read to EOF) means the + // cached connection is spent; drop it so the next request redials. + if persistent && !keepAlive { + nethttpDropConn(cfg.inst) + } + + // A no-body response (HEAD etc.) still carries Content-Length / Transfer-Encoding + // headers but no body bytes: strip that framing so ParseResponse frames an empty + // body instead of trying to read one, mirroring the one-shot path. + if noBody { + raw = trimResponseToHeaders(raw) + } + resp, perr := nethttp.ParseResponse(raw) + if perr != nil { + raise("Net::HTTPBadResponse", "%s", perr.Error()) + } + respVal := vm.nethttpBuildResponse(resp) + if noBody { + setIvar(respVal, "@body", object.NilV) + } + return respVal +} + +// nethttpBuildRequest builds the request bytes for an instance transfer. For a +// plain-http proxy the request-target is the absolute URI (the proxy routes on +// it); otherwise it is the origin-form path. close asks the server to close the +// connection (used for one-shot, non-persistent requests). It returns the bytes +// and whether the response carries no body (HEAD etc.). +func (vm *VM) nethttpBuildRequest(cfg *nethttpXfer, method, path string, body []byte, hdr [][2]string, close bool) ([]byte, bool) { + target := path + if cfg.proxied && !cfg.connectTunnel { + target = cfg.scheme + "://" + cfg.hostHdr + path + } + req, err := nethttp.NewRequest(method, target, cfg.hostHdr, hdr) + if err != nil { + raise("ArgumentError", "%s", err.Error()) + } + if body != nil { + req.SetBody(body) + } + if cfg.proxied && !cfg.connectTunnel && cfg.proxyAuth != "" { + req.Set("Proxy-Authorization", cfg.proxyAuth) + } + if close { + req.Set("Connection", "close") + } + reqBytes, err := req.Bytes("1.1") + if err != nil { + raise("Net::HTTPError", "%s", err.Error()) + } + return reqBytes, !req.ResponseBodyPermitted() +} + +// nethttpExchangeFramed writes the request (under the write deadline) and reads +// exactly one framed response (under the read deadline) off the stream, leaving +// any following bytes buffered for the next keep-alive request. keepAlive reports +// whether the connection may be reused (false ⇒ the server asked to close or the +// body was unframed and read to EOF). +func (vm *VM) nethttpExchangeFramed(cfg *nethttpXfer, s streamIO, reqBytes []byte, noBody bool) (raw []byte, keepAlive bool, err error) { + nethttpSetDeadline(s, cfg.writeTO) + if _, werr := s.writer().Write(reqBytes); werr != nil { + return nil, false, werr + } + nethttpSetDeadline(s, cfg.readTO) + return nethttpReadResponse(s.reader(), noBody) +} + +// nethttpDialXfer opens the transport for a transfer: a direct dial, a plain-http +// proxy dial (a bare TCP socket to the proxy — the absolute-URI request-line does +// the routing), or an https-via-proxy CONNECT tunnel (dial proxy, CONNECT, then +// TLS through the tunnel to the real host). +func (vm *VM) nethttpDialXfer(cfg *nethttpXfer) (streamIO, error) { + if !cfg.proxied { + return nethttpDialTimeout(cfg.scheme, cfg.host, cfg.dialHost, cfg.dialPort, cfg.verifyMode, cfg.openTO) + } + if !cfg.connectTunnel { + // Plain http through a proxy: a bare TCP socket to the proxy. + conn, err := nethttpRawDial(cfg.dialHost, cfg.dialPort, cfg.openTO) + if err != nil { + return nil, err + } + return newTCPSocket(nil, conn), nil + } + // https through a proxy: CONNECT then TLS to the real host over the tunnel. + conn, err := nethttpRawDial(cfg.dialHost, cfg.dialPort, cfg.openTO) + if err != nil { + return nil, err + } + if err := nethttpProxyConnect(conn, cfg.hostHdr, cfg.proxyAuth); err != nil { + conn.Close() + return nil, err + } + return nethttpTLSWrap(conn, cfg.host, cfg.verifyMode) +} + +// nethttpProxyConnect sends a CONNECT request for target through conn and reads +// the proxy's response, erroring unless it is a 2xx. It reads only up to the +// response's header terminator (byte at a time) so no tunnelled TLS bytes are +// consumed. +func nethttpProxyConnect(conn net.Conn, target, proxyAuth string) error { + var b strings.Builder + b.WriteString("CONNECT " + target + " HTTP/1.1\r\n") + b.WriteString("Host: " + target + "\r\n") + if proxyAuth != "" { + b.WriteString("Proxy-Authorization: " + proxyAuth + "\r\n") + } + b.WriteString("\r\n") + if _, err := conn.Write([]byte(b.String())); err != nil { + return err + } + // Read the response headers a byte at a time up to CRLFCRLF. + var resp []byte + buf := make([]byte, 1) + for !bytes.HasSuffix(resp, []byte("\r\n\r\n")) { + n, err := conn.Read(buf) + if n > 0 { + resp = append(resp, buf[0]) + } + if err != nil { + return err + } + } + line := string(resp) + if i := strings.Index(line, "\r\n"); i >= 0 { + line = line[:i] + } + fields := strings.Fields(line) + if len(fields) < 2 || !strings.HasPrefix(fields[1], "2") { + return fmt.Errorf("proxy CONNECT failed: %s", strings.TrimSpace(line)) + } + return nil +} + +// nethttpFinish closes a started instance's cached keep-alive connection (if any) +// and marks it un-started. It backs both Net::HTTP#finish and the start-block +// teardown. +func (vm *VM) nethttpFinish(inst object.Value) { + nethttpDropConn(inst) + setIvar(inst, "@started", object.Bool(false)) +} + +// nethttpGetConn returns the instance's cached persistent connection, or nil. +func nethttpGetConn(inst object.Value) streamIO { + if s, ok := getIvar(inst, "@conn").(streamIO); ok { + return s + } + return nil +} + +// nethttpSetConn caches a persistent connection on the instance. +func nethttpSetConn(inst object.Value, s streamIO) { + setIvar(inst, "@conn", s.(object.Value)) +} + +// nethttpDropConn closes and clears the instance's cached connection. +func nethttpDropConn(inst object.Value) { + if s := nethttpGetConn(inst); s != nil { + s.closeConn() + } + setIvar(inst, "@conn", object.NilV) +} + +// nethttpDuration converts a Ruby timeout (Integer/Float seconds) to a Duration; +// a nil or non-positive value yields 0 (no deadline). +func nethttpDuration(v object.Value) time.Duration { + var secs float64 + switch n := v.(type) { + case object.Integer: + secs = float64(n) + case object.Float: + secs = float64(n) + default: + return 0 + } + if secs <= 0 { + return 0 + } + return time.Duration(secs * float64(time.Second)) +} + +// nethttpSetDeadline applies a relative deadline to a stream's underlying socket +// (0 clears it). It is a no-op for a stream with no net.Conn (e.g. a test fake). +func nethttpSetDeadline(s streamIO, d time.Duration) { + conn := nethttpNetConn(s) + if conn == nil { + return + } + if d <= 0 { + conn.SetDeadline(time.Time{}) + return + } + conn.SetDeadline(time.Now().Add(d)) +} + +// nethttpNetConn extracts the underlying net.Conn from the two real stream +// transports (tcpSocket / sslSocket); anything else (a test fake) yields nil. +func nethttpNetConn(s streamIO) net.Conn { + switch t := s.(type) { + case *tcpSocket: + return t.conn + case *sslSocket: + return t.conn + default: + return nil + } +} + +// raiseTransportErr maps a transport error to the MRI exception: an i/o timeout +// during open/read/write raises Net::OpenTimeout / Net::ReadTimeout / +// Net::WriteTimeout; anything else raises SocketError. +func (vm *VM) raiseTransportErr(err error, phase string) { + if ne, ok := err.(net.Error); ok && ne.Timeout() { + switch phase { + case "open": + raise("Net::OpenTimeout", "execution expired") + case "write": + raise("Net::WriteTimeout", "execution expired") + default: + raise("Net::ReadTimeout", "execution expired") + } + } + raise("SocketError", "%s", err.Error()) +} + +// basicAuthHeader builds an HTTP Basic credential value for Proxy-Authorization. +func basicAuthHeader(user, pass string) string { + return "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) +} + +// nethttpRawDial opens a raw TCP connection with an optional dial timeout +// (open_timeout); 0 means no bound. +func nethttpRawDial(host, port string, openTO time.Duration) (net.Conn, error) { + addr := net.JoinHostPort(host, port) + if openTO > 0 { + return net.DialTimeout("tcp", addr, openTO) + } + return net.Dial("tcp", addr) +} + +// nethttpDialTimeout is nethttpDial with an open_timeout on the TCP dial and the +// TLS handshake, returning the connected stream (raw for http, TLS for https). +func nethttpDialTimeout(scheme, host, dialHost, dialPort string, verifyMode int64, openTO time.Duration) (streamIO, error) { + conn, err := nethttpRawDial(dialHost, dialPort, openTO) + if err != nil { + return nil, err + } + if scheme != "https" { + return newTCPSocket(nil, conn), nil + } + if openTO > 0 { + conn.SetDeadline(time.Now().Add(openTO)) + } + s, err := nethttpTLSWrap(conn, host, verifyMode) + if openTO > 0 && err == nil { + conn.SetDeadline(time.Time{}) + } + return s, err +} + +// nethttpTLSWrap performs the TLS handshake over an already-connected socket and +// returns the sslSocket stream. verifyMode 0 (VERIFY_NONE) skips verification. +func nethttpTLSWrap(conn net.Conn, host string, verifyMode int64) (streamIO, error) { + tcp := newTCPSocket(nil, conn) + cfg := &tls.Config{ServerName: host} + if verifyMode == 0 { + cfg.InsecureSkipVerify = true + } + c := tls.Client(conn, cfg) + if err := c.Handshake(); err != nil { + conn.Close() + return nil, err + } + return &sslSocket{tcp: tcp, ctx: object.NilV, conn: conn, tls: c, r: bufio.NewReader(c), hostname: host}, nil +} + +// nethttpReadResponse reads exactly one HTTP/1.1 response off r — the status line, +// the header block, and the body framed by Transfer-Encoding: chunked or +// Content-Length (or read to EOF when unframed) — returning the raw bytes to hand +// to nethttp.ParseResponse. keepAlive reports whether the connection may be reused +// (false when the server sent Connection: close or the body was read to EOF). +// noBody (HEAD and other body-forbidden responses) stops after the header block. +func nethttpReadResponse(r *bufio.Reader, noBody bool) (raw []byte, keepAlive bool, err error) { + var buf bytes.Buffer + status, err := r.ReadString('\n') + if err != nil { + return nil, false, err + } + buf.WriteString(status) + headers := map[string]string{} + for { + line, lerr := r.ReadString('\n') + if lerr != nil { + return nil, false, lerr + } + buf.WriteString(line) + if strings.TrimRight(line, "\r\n") == "" { + break + } + if i := strings.IndexByte(line, ':'); i >= 0 { + k := strings.ToLower(strings.TrimSpace(line[:i])) + v := strings.TrimSpace(line[i+1:]) + if prev, ok := headers[k]; ok { + headers[k] = prev + ", " + v + } else { + headers[k] = v + } + } + } + keepAlive = !strings.EqualFold(headers["connection"], "close") + if noBody { + return buf.Bytes(), keepAlive, nil + } + if te, ok := headers["transfer-encoding"]; ok && nethttpIsChunked(te) { + if cerr := nethttpCopyChunked(r, &buf); cerr != nil { + return nil, false, cerr + } + return buf.Bytes(), keepAlive, nil + } + if cl, ok := headers["content-length"]; ok { + n, cerr := strconv.Atoi(strings.TrimSpace(cl)) + if cerr != nil || n < 0 { + return nil, false, fmt.Errorf("wrong Content-Length: %q", cl) + } + if _, cerr := io.CopyN(&buf, r, int64(n)); cerr != nil { + return nil, false, cerr + } + return buf.Bytes(), keepAlive, nil + } + // No framing: read to EOF. The connection is spent afterwards. + rest, rerr := io.ReadAll(r) + if rerr != nil { + return nil, false, rerr + } + buf.Write(rest) + return buf.Bytes(), false, nil +} + +// nethttpCopyChunked copies a chunked body (raw, including the chunk framing) from +// r into buf until the zero-size chunk, then consumes the trailer up to a blank +// line — the same framing nethttp.ParseResponse later decodes. +func nethttpCopyChunked(r *bufio.Reader, buf *bytes.Buffer) error { + for { + line, err := r.ReadString('\n') + if err != nil { + return err + } + buf.WriteString(line) + hexlen := nethttpFirstHexRun(line) + if hexlen == "" { + return fmt.Errorf("wrong chunk size line: %q", line) + } + n, err := strconv.ParseInt(hexlen, 16, 64) + if err != nil { + return fmt.Errorf("wrong chunk size line: %q", line) + } + if n == 0 { + break + } + if _, err := io.CopyN(buf, r, n); err != nil { + return err + } + if _, err := io.CopyN(buf, r, 2); err != nil { // trailing CRLF + return err + } + } + for { // trailer lines until a blank one (or EOF) + line, err := r.ReadString('\n') + buf.WriteString(line) + if err != nil || strings.TrimRight(line, "\r\n") == "" { + break + } + } + return nil +} + +// nethttpIsChunked reports whether a Transfer-Encoding field requests chunked +// framing (case-insensitive substring, as MRI's chunked? does). +func nethttpIsChunked(te string) bool { + return strings.Contains(strings.ToLower(te), "chunked") +} + +// nethttpFirstHexRun returns the first maximal run of hex digits in a chunk-size +// line (the codec's firstHexRun), "" when there is none. +func nethttpFirstHexRun(line string) string { + start := -1 + for i := 0; i < len(line); i++ { + c := line[i] + isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') + if isHex { + if start < 0 { + start = i + } + } else if start >= 0 { + return line[start:i] + } + } + if start >= 0 { + return line[start:] + } + return "" } // nethttpDo is the codec↔socket seam: build the request bytes with the net-http @@ -483,8 +1001,12 @@ func (vm *VM) nethttpResponseClass(netMod *RClass, class, category string) *RCla // --- argument helpers ------------------------------------------------------- -// nethttpNewInstance builds a Net::HTTP instance from (address[, port]), -// defaulting the port to 80 and use_ssl to false. +// nethttpNewInstance builds a Net::HTTP instance from +// (address[, port[, p_addr[, p_port[, p_user[, p_pass]]]]]), defaulting the port +// to 80, use_ssl to false and each timeout to 60s (MRI's defaults). A proxy is +// configured only when p_addr is an explicit host String; a nil or a bare +// :ENV symbol (MRI's default, which would consult http_proxy) leaves the +// instance proxy-less — environment-proxy resolution is a flagged follow-up. func (vm *VM) nethttpNewInstance(http *RClass, args []object.Value) object.Value { o := &RObject{class: http, ivars: map[string]object.Value{}} host := "" @@ -499,6 +1021,31 @@ func (vm *VM) nethttpNewInstance(http *RClass, args []object.Value) object.Value o.ivars["@port"] = object.IntValue(port) o.ivars["@use_ssl"] = object.Bool(false) o.ivars["@header"] = object.NewHash() + // MRI defaults: 60s open/read/write timeouts. + o.ivars["@open_timeout"] = object.IntValue(60) + o.ivars["@read_timeout"] = object.IntValue(60) + o.ivars["@write_timeout"] = object.IntValue(60) + // Proxy (p_addr, p_port, p_user, p_pass): only an explicit host String enables it. + o.ivars["@proxy_address"] = object.NilV + o.ivars["@proxy_port"] = object.NilV + o.ivars["@proxy_user"] = object.NilV + o.ivars["@proxy_pass"] = object.NilV + if len(args) > 2 { + if ps, ok := args[2].(*object.String); ok && ps.Str() != "" { + o.ivars["@proxy_address"] = args[2] + pp := int64(80) + if len(args) > 3 && !object.IsNil(args[3]) { + pp = intArg(args[3]) + } + o.ivars["@proxy_port"] = object.IntValue(pp) + if len(args) > 4 && !object.IsNil(args[4]) { + o.ivars["@proxy_user"] = args[4] + } + if len(args) > 5 && !object.IsNil(args[5]) { + o.ivars["@proxy_pass"] = args[5] + } + } + } return o } From 605c8da29b82697fd252ea254ac83debdd7f181d Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 6 Jul 2026 21:34:21 +0200 Subject: [PATCH 2/4] test(net-http): prove keep-alive/timeout/proxy; unify one-shot path - Route the class-level URL conveniences (get/post/...) through the same nethttpDoXfer transport as instance requests, removing the now-redundant one-shot nethttpDo / nethttpRoundTrip / nethttpDial / httpExchange so there is a single, fully-exercised request path. - Add hermetic proofs: keep-alive (3 requests, 1 accepted TCP connection), server-Connection:close fallback, stale-connection redial retry, read timeout (Net::ReadTimeout), open timeout, http proxy (absolute-URI request-line + Proxy-Authorization routed through a forwarding proxy) and https-via-CONNECT tunnel, plus unit coverage of every transport helper and error arm. 100% coverage on nethttp_bind.go. Co-Authored-By: Claude Opus 4.8 --- internal/vm/nethttp_bind.go | 102 +---- internal/vm/nethttp_bind_test.go | 639 ++++++++++++++++++++++++++++++- 2 files changed, 639 insertions(+), 102 deletions(-) diff --git a/internal/vm/nethttp_bind.go b/internal/vm/nethttp_bind.go index 74b57603..025c6118 100644 --- a/internal/vm/nethttp_bind.go +++ b/internal/vm/nethttp_bind.go @@ -326,10 +326,17 @@ func (vm *VM) augmentNetHTTPResponse(resp *RClass) { // nethttpExecURL runs a one-shot request against a parsed URL, resolving the // scheme / host / port and defaulting TLS verification off (the class-level -// conveniences take no context). Returns the Net::HTTPResponse. +// conveniences take no context). It routes through the same transport as instance +// requests, as a non-persistent (Connection: close), un-proxied, deadline-less +// transfer. Returns the Net::HTTPResponse. func (vm *VM) nethttpExecURL(u *url.URL, method string, body []byte, hdr [][2]string) object.Value { scheme, port := nethttpSchemePort(u) - return vm.nethttpDo(scheme, u.Hostname(), port, u.Host, method, u.RequestURI(), body, hdr, 0) + host := u.Hostname() + cfg := &nethttpXfer{ + scheme: scheme, host: host, port: port, hostHdr: u.Host, + dialHost: host, dialPort: port, + } + return vm.nethttpDoXfer(cfg, method, u.RequestURI(), body, hdr) } // nethttpSchemePort resolves a URL's scheme (defaulting to http) and port @@ -850,47 +857,6 @@ func nethttpFirstHexRun(line string) string { return "" } -// nethttpDo is the codec↔socket seam: build the request bytes with the net-http -// codec, write them to a connected (optionally TLS) socket, read the whole -// response, parse it, and build the Ruby Net::HTTPResponse. -func (vm *VM) nethttpDo(scheme, host, port, hostHdr, method, path string, body []byte, hdr [][2]string, verifyMode int64) object.Value { - req, err := nethttp.NewRequest(method, path, hostHdr, hdr) - if err != nil { - raise("ArgumentError", "%s", err.Error()) - } - if body != nil { - req.SetBody(body) - } - // One connection per request: ask the server to close so the read drains to - // EOF (persistent connections are a follow-up). - req.Set("Connection", "close") - reqBytes, err := req.Bytes("1.1") - if err != nil { - raise("Net::HTTPError", "%s", err.Error()) - } - raw, err := vm.nethttpRoundTrip(scheme, host, port, reqBytes, verifyMode) - if err != nil { - raise("SocketError", "%s", err.Error()) - } - // A HEAD (and any request whose response body is not permitted) still receives - // the server's Content-Length / Transfer-Encoding headers but no body bytes; - // strip the framing so the status-based codec does not try to read a body, and - // report a nil body as MRI's response_body_permitted? false path does. - noBody := !req.ResponseBodyPermitted() - if noBody { - raw = trimResponseToHeaders(raw) - } - resp, err := nethttp.ParseResponse(raw) - if err != nil { - raise("Net::HTTPBadResponse", "%s", err.Error()) - } - respVal := vm.nethttpBuildResponse(resp) - if noBody { - setIvar(respVal, "@body", object.NilV) - } - return respVal -} - // trimResponseToHeaders keeps only the status line and header block of a raw // response, dropping the body and any Content-Length / Transfer-Encoding fields, // so ParseResponse frames an empty body (used for HEAD, whose response carries @@ -911,56 +877,6 @@ func trimResponseToHeaders(raw []byte) []byte { return append(bytes.Join(kept, []byte("\r\n")), []byte("\r\n\r\n")...) } -// nethttpRoundTrip opens the connection through rbgo's socket transport (a -// tcpSocket, wrapped in a TLS sslSocket for https), writes the request bytes and -// reads the full response to EOF. It reuses the transport's connected-stream -// types and streamIO surface so the bytes travel the same path a Ruby -// TCPSocket / OpenSSL::SSL::SSLSocket would. -func (vm *VM) nethttpRoundTrip(scheme, host, port string, reqBytes []byte, verifyMode int64) ([]byte, error) { - stream, err := nethttpDial(scheme, host, port, verifyMode) - if err != nil { - return nil, err - } - defer stream.closeConn() - return httpExchange(stream, reqBytes) -} - -// nethttpDial opens the connection through rbgo's socket transport (a tcpSocket, -// wrapped in a TLS sslSocket for https) and returns it as a streamIO. verify_mode -// VERIFY_NONE (0) — the default — skips certificate verification, matching the -// SSLSocket transport's bare-context default; VERIFY_PEER turns on Go's chain + -// hostname checks. -func nethttpDial(scheme, host, port string, verifyMode int64) (streamIO, error) { - conn, err := net.Dial("tcp", net.JoinHostPort(host, port)) - if err != nil { - return nil, err - } - tcp := newTCPSocket(nil, conn) - if scheme != "https" { - return tcp, nil - } - cfg := &tls.Config{ServerName: host} - if verifyMode == 0 { - cfg.InsecureSkipVerify = true - } - c := tls.Client(conn, cfg) - if err := c.Handshake(); err != nil { - conn.Close() - return nil, err - } - return &sslSocket{tcp: tcp, ctx: object.NilV, conn: conn, tls: c, r: bufio.NewReader(c), hostname: host}, nil -} - -// httpExchange writes the request bytes to a connected stream and reads the whole -// response to EOF. It is the transport-agnostic write/read half of the round trip -// (split from the dial so both I/O-error arms are exercisable). -func httpExchange(stream streamIO, reqBytes []byte) ([]byte, error) { - if _, err := stream.writer().Write(reqBytes); err != nil { - return nil, err - } - return io.ReadAll(stream.reader()) -} - // nethttpBuildResponse turns a parsed *nethttp.Response into a Ruby // Net::HTTPResponse of the subclass MRI would have instantiated (e.g. // Net::HTTPOK), carrying @code / @message / @body / @header / @http_version. diff --git a/internal/vm/nethttp_bind_test.go b/internal/vm/nethttp_bind_test.go index 21e62f85..6ef4beb0 100644 --- a/internal/vm/nethttp_bind_test.go +++ b/internal/vm/nethttp_bind_test.go @@ -6,6 +6,9 @@ package vm import ( "bufio" + "bytes" + "encoding/base64" + "errors" "fmt" "io" "net" @@ -17,7 +20,11 @@ import ( "path/filepath" "runtime" "strings" + "sync/atomic" "testing" + "time" + + "github.com/go-embedded-ruby/ruby/internal/object" ) // The Net::HTTP networking surface is proven end-to-end against in-process @@ -309,14 +316,15 @@ func TestNetHTTPWhiteBox(t *testing.T) { vm := New(io.Discard) netMod := vm.consts["Net"].(*RClass) - // nethttpDo: NewRequest rejects an unknown method (ArgumentError). + // nethttpBuildRequest: NewRequest rejects an unknown method (ArgumentError). + cfg := &nethttpXfer{scheme: "http", host: "127.0.0.1", port: "1", hostHdr: "h"} wantRaise(t, "ArgumentError", func() { - vm.nethttpDo("http", "127.0.0.1", "1", "h", "BOGUS", "/", nil, nil, 0) + vm.nethttpBuildRequest(cfg, "BOGUS", "/", nil, nil, true) }) - // nethttpDo: Request.Bytes rejects a Request-Line with CR/LF (Net::HTTPError), - // before any dial. + // nethttpBuildRequest: Request.Bytes rejects a Request-Line with CR/LF + // (Net::HTTPError), before any dial. wantRaise(t, "Net::HTTPError", func() { - vm.nethttpDo("http", "127.0.0.1", "1", "h", "GET", "/a\r\nb", nil, nil, 0) + vm.nethttpBuildRequest(cfg, "GET", "/a\r\nb", nil, nil, true) }) // nethttpResponseClass falls back to Net::HTTPResponse for an unknown class and @@ -448,15 +456,17 @@ func (f *fakeStream) markClosed() {} func (f *fakeStream) isClosed() bool { return false } func (f *fakeStream) closeConn() error { return nil } -// TestNetHTTPExchangeErrors covers httpExchange's write-error and read-error arms -// and trimResponseToHeaders' no-terminator arm. +// TestNetHTTPExchangeErrors covers nethttpExchangeFramed's write-error and +// read-error arms and trimResponseToHeaders' no-terminator arm. func TestNetHTTPExchangeErrors(t *testing.T) { + vm := New(io.Discard) + cfg := &nethttpXfer{} // Write error: the writer fails immediately. - if _, err := httpExchange(&fakeStream{w: failWriter{}, r: bufio.NewReader(strings.NewReader(""))}, []byte("x")); err != io.ErrClosedPipe { + if _, _, err := vm.nethttpExchangeFramed(cfg, &fakeStream{w: failWriter{}, r: bufio.NewReader(strings.NewReader(""))}, []byte("x"), false); err != io.ErrClosedPipe { t.Errorf("write-error arm = %v, want ErrClosedPipe", err) } // Read error: the write succeeds, the read fails. - if _, err := httpExchange(&fakeStream{w: io.Discard, r: bufio.NewReader(errReader{})}, []byte("x")); err != io.ErrUnexpectedEOF { + if _, _, err := vm.nethttpExchangeFramed(cfg, &fakeStream{w: io.Discard, r: bufio.NewReader(errReader{})}, []byte("x"), false); err != io.ErrUnexpectedEOF { t.Errorf("read-error arm = %v, want ErrUnexpectedEOF", err) } // trimResponseToHeaders returns its input unchanged when there is no header @@ -477,3 +487,614 @@ func mustPort(t *testing.T, raw string) string { } return u.Port() } + +// countingServer starts an httptest server over nethttpTestMux that counts the +// distinct TCP connections it accepts (via ConnState StateNew), so keep-alive can +// be proven by the connection count rather than the request count. +func countingServer(t *testing.T) (*httptest.Server, *int64) { + t.Helper() + var conns int64 + ts := httptest.NewUnstartedServer(nethttpTestMux()) + ts.Config.ConnState = func(_ net.Conn, state http.ConnState) { + if state == http.StateNew { + atomic.AddInt64(&conns, 1) + } + } + ts.Start() + return ts, &conns +} + +// TestNetHTTPKeepAlive is the persistent-connection proof: three requests inside a +// single Net::HTTP.start block return the right bodies while the server accepts +// exactly one TCP connection (keep-alive reuse). A separate case shows requests +// outside a start block each open their own connection. +func TestNetHTTPKeepAlive(t *testing.T) { + ts, conns := countingServer(t) + defer ts.Close() + u, _ := url.Parse(ts.URL) + + // Three requests, one start block: bodies correct AND one connection. + got := nethttpRun(t, ts.URL, `h=Net::HTTP.new(URI(BASE).host, URI(BASE).port) +h.start do + puts h.get("/").body + puts h.get("/echo?n=2").body + puts h.get("/created").body +end`) + want := "hello net/http\nGET|/echo?n=2||\nmade" + if got != want { + t.Fatalf("keep-alive bodies = %q, want %q", got, want) + } + if n := atomic.LoadInt64(conns); n != 1 { + t.Fatalf("keep-alive used %d connections, want 1", n) + } + // A chunked response inside the same reused connection frames correctly and + // still leaves the connection reusable for a following request. + atomic.StoreInt64(conns, 0) + got = nethttpRun(t, ts.URL, `h=Net::HTTP.new(URI(BASE).host, URI(BASE).port) +h.start do + puts h.get("/chunked").body + puts h.get("/").body +end`) + if want = "chunk-body\nhello net/http"; got != want { + t.Fatalf("keep-alive chunked = %q, want %q", got, want) + } + if n := atomic.LoadInt64(conns); n != 1 { + t.Fatalf("keep-alive chunked used %d connections, want 1", n) + } + // Without a start block, each request opens (and closes) its own connection. + atomic.StoreInt64(conns, 0) + got = nethttpRun(t, ts.URL, `h=Net::HTTP.new(URI(BASE).host, URI(BASE).port) +puts h.get("/").body +puts h.get("/").body`) + if want = "hello net/http\nhello net/http"; got != want { + t.Fatalf("non-persistent bodies = %q, want %q", got, want) + } + if n := atomic.LoadInt64(conns); n != 2 { + t.Fatalf("non-persistent used %d connections, want 2", n) + } + _ = u +} + +// TestNetHTTPKeepAliveServerClose proves the fallback arm: when the server sends +// Connection: close, the cached connection is dropped and the next request in the +// same start block redials (so the body is still correct and a new connection is +// accepted). +func TestNetHTTPKeepAliveServerClose(t *testing.T) { + var conns int64 + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Connection", "close") // ask the client to not reuse + io.WriteString(w, "closed-body") + }) + ts := httptest.NewUnstartedServer(mux) + ts.Config.ConnState = func(_ net.Conn, state http.ConnState) { + if state == http.StateNew { + atomic.AddInt64(&conns, 1) + } + } + ts.Start() + defer ts.Close() + + got := nethttpRun(t, ts.URL, `h=Net::HTTP.new(URI(BASE).host, URI(BASE).port) +h.start do + puts h.get("/").body + puts h.get("/").body +end`) + if want := "closed-body\nclosed-body"; got != want { + t.Fatalf("server-close bodies = %q, want %q", got, want) + } + if n := atomic.LoadInt64(&conns); n != 2 { + t.Fatalf("server-close used %d connections, want 2 (one per request)", n) + } +} + +// TestNetHTTPReadTimeout is the timeout proof: a slow handler that never answers +// within read_timeout makes the request raise Net::ReadTimeout, and the accessor +// round-trips the configured value. +func TestNetHTTPReadTimeout(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/slow", func(w http.ResponseWriter, r *http.Request) { + select { + case <-time.After(5 * time.Second): + case <-r.Context().Done(): // client hung up (its read timed out) -> return promptly + } + }) + ts := httptest.NewServer(mux) + defer ts.Close() + + got := nethttpRun(t, ts.URL, `h=Net::HTTP.new(URI(BASE).host, URI(BASE).port) +h.read_timeout = 0.2 +puts h.read_timeout +begin + h.get("/slow") +rescue Net::ReadTimeout + puts "readtimeout" +end`) + if want := "0.2\nreadtimeout"; got != want { + t.Fatalf("read-timeout = %q, want %q", got, want) + } +} + +// TestNetHTTPTimeoutAccessors covers the open/read/write_timeout accessor surface +// and the MRI defaults (60 each), independent of any I/O. +func TestNetHTTPTimeoutAccessors(t *testing.T) { + got := runSrc(t, `require "net/http" +h = Net::HTTP.new("h", 80) +puts h.open_timeout +puts h.read_timeout +puts h.write_timeout +h.open_timeout = 5 +h.write_timeout = 3 +puts h.open_timeout +puts h.write_timeout`) + if want := "60\n60\n60\n5\n3"; got != want { + t.Fatalf("timeout accessors = %q, want %q", got, want) + } +} + +// TestNetHTTPOpenTimeout proves open_timeout bounds the dial: a routeable but +// unreachable address (RFC 5737 TEST-NET-1) with a tiny open_timeout raises +// Net::OpenTimeout rather than hanging. +func TestNetHTTPOpenTimeout(t *testing.T) { + if testing.Short() { + t.Skip("skipping network dial-timeout test in -short") + } + got := runSrc(t, `require "net/http" +h = Net::HTTP.new("192.0.2.1", 80) +h.open_timeout = 0.2 +begin + h.get("/") +rescue Net::OpenTimeout + puts "opentimeout" +rescue SocketError + puts "socketerror" +end`) + // A blackholed TEST-NET address times out; some CI networks reject it outright + // (SocketError). Either proves open_timeout did not hang. + if got != "opentimeout" && got != "socketerror" { + t.Fatalf("open-timeout = %q, want opentimeout or socketerror", got) + } +} + +// TestNetHTTPProxyHTTP is the plain-http proxy proof: a tiny forwarding proxy sees +// the absolute-URI request-line (and any Proxy-Authorization), forwards to the +// backend, and the client gets the backend's body — proving the request was +// routed through the proxy. +func TestNetHTTPProxyHTTP(t *testing.T) { + backend := httptest.NewServer(nethttpTestMux()) + defer backend.Close() + + var proxyHits int64 + var sawAuth string + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&proxyHits, 1) + sawAuth = r.Header.Get("Proxy-Authorization") + // The request-line is absolute-form for a plain-http proxy: r.RequestURI is + // the full target URL. Forward it to the backend and copy the response back. + resp, err := http.Get(r.RequestURI) + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + w.Write(body) + })) + defer proxy.Close() + + bu, _ := url.Parse(backend.URL) + pu, _ := url.Parse(proxy.URL) + + // Route a GET through the proxy to the backend /echo. + got := runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new(%q, %s, %q, %s) +puts h.proxy? +puts h.proxy_address +puts h.proxy_port +puts h.get("/echo").body`, bu.Hostname(), bu.Port(), pu.Hostname(), pu.Port())) + want := "true\n" + pu.Hostname() + "\n" + pu.Port() + "\nGET|/echo||" + if got != want { + t.Fatalf("proxy GET = %q, want %q", got, want) + } + if n := atomic.LoadInt64(&proxyHits); n < 1 { + t.Fatalf("proxy was not hit (%d)", n) + } + + // Proxy credentials are sent as Proxy-Authorization. + atomic.StoreInt64(&proxyHits, 0) + got = runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new(%q, %s, %q, %s, "usr", "pwd") +puts h.proxy_user +puts h.proxy_pass +puts h.get("/").body`, bu.Hostname(), bu.Port(), pu.Hostname(), pu.Port())) + if want = "usr\npwd\nhello net/http"; got != want { + t.Fatalf("proxy auth GET = %q, want %q", got, want) + } + wantAuth := "Basic " + base64Std("usr:pwd") + if sawAuth != wantAuth { + t.Fatalf("Proxy-Authorization = %q, want %q", sawAuth, wantAuth) + } +} + +// TestNetHTTPProxyConnect proves the https-via-proxy path: a CONNECT-tunnelling +// proxy splices bytes to the TLS backend, and the client completes the TLS +// handshake and request through the tunnel. +func TestNetHTTPProxyConnect(t *testing.T) { + backend := httptest.NewTLSServer(nethttpTestMux()) + defer backend.Close() + + var connects int64 + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodConnect { + http.Error(w, "expected CONNECT", http.StatusMethodNotAllowed) + return + } + atomic.AddInt64(&connects, 1) + dst, err := net.Dial("tcp", r.Host) // r.Host is the CONNECT target host:port + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + hj, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "no hijack", http.StatusInternalServerError) + dst.Close() + return + } + conn, _, err := hj.Hijack() + if err != nil { + dst.Close() + return + } + conn.Write([]byte("HTTP/1.1 200 Connection Established\r\n\r\n")) + go func() { io.Copy(dst, conn); dst.Close() }() + io.Copy(conn, dst) + conn.Close() + })) + defer proxy.Close() + + bu, _ := url.Parse(backend.URL) + pu, _ := url.Parse(proxy.URL) + + got := runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new(%q, %s, %q, %s) +h.use_ssl = true +puts h.get("/").body`, bu.Hostname(), bu.Port(), pu.Hostname(), pu.Port())) + if want := "hello net/http"; got != want { + t.Fatalf("proxy CONNECT body = %q, want %q", got, want) + } + if n := atomic.LoadInt64(&connects); n != 1 { + t.Fatalf("CONNECT count = %d, want 1", n) + } +} + +// TestNetHTTPProxyConnectFailure covers the CONNECT-refused arm: a proxy that +// answers CONNECT with a non-2xx status makes the tunnel dial fail (SocketError). +func TestNetHTTPProxyConnectFailure(t *testing.T) { + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hj, _ := w.(http.Hijacker) + conn, _, err := hj.Hijack() + if err != nil { + return + } + conn.Write([]byte("HTTP/1.1 403 Forbidden\r\n\r\n")) + conn.Close() + })) + defer proxy.Close() + pu, _ := url.Parse(proxy.URL) + + got := runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new("example.com", 443, %q, %s) +h.use_ssl = true +begin + h.get("/") +rescue SocketError + puts "connectfail" +end`, pu.Hostname(), pu.Port())) + if got != "connectfail" { + t.Fatalf("CONNECT-failure = %q, want connectfail", got) + } +} + +// TestNetHTTPTransportUnits covers the transport helpers directly on the arms that +// normal Ruby dispatch does not reach: duration conversion, the chunk-size hex +// scanner, chunked detection, net.Conn extraction, the timeout error mapping and +// the CONNECT / read framing error arms. +func TestNetHTTPTransportUnits(t *testing.T) { + // nethttpDuration: Integer / Float / nil / non-positive. + if d := nethttpDuration(object.IntValue(2)); d != 2*time.Second { + t.Errorf("duration(2) = %v", d) + } + if d := nethttpDuration(object.Float(0.5)); d != 500*time.Millisecond { + t.Errorf("duration(0.5) = %v", d) + } + if d := nethttpDuration(object.NilV); d != 0 { + t.Errorf("duration(nil) = %v, want 0", d) + } + if d := nethttpDuration(object.IntValue(0)); d != 0 { + t.Errorf("duration(0) = %v, want 0", d) + } + if d := nethttpDuration(object.Float(-1)); d != 0 { + t.Errorf("duration(-1) = %v, want 0", d) + } + + // nethttpFirstHexRun: leading junk, trailing run, and no-hex. + for _, c := range []struct{ in, want string }{ + {"1a\r\n", "1a"}, {" ;\r\n", ""}, {"ff", "ff"}, {"; 5 ", "5"}, + } { + if got := nethttpFirstHexRun(c.in); got != c.want { + t.Errorf("firstHexRun(%q) = %q, want %q", c.in, got, c.want) + } + } + // nethttpIsChunked. + if !nethttpIsChunked("chunked") || nethttpIsChunked("gzip") { + t.Error("isChunked mismatch") + } + // nethttpNetConn: a non-socket stream (test fake) yields nil. + if c := nethttpNetConn(&fakeStream{}); c != nil { + t.Errorf("netConn(fake) = %v, want nil", c) + } + // nethttpSetDeadline on a no-conn stream is a no-op (both the 0 and >0 arms). + nethttpSetDeadline(&fakeStream{}, 0) + nethttpSetDeadline(&fakeStream{}, time.Second) + + // basicAuthHeader. + if got := basicAuthHeader("a", "b"); got != "Basic "+base64Std("a:b") { + t.Errorf("basicAuthHeader = %q", got) + } + + // raiseTransportErr: timeout maps per phase; non-timeout is a SocketError. + for phase, class := range map[string]string{ + "open": "Net::OpenTimeout", "write": "Net::WriteTimeout", "read": "Net::ReadTimeout", + } { + vm := New(io.Discard) + wantRaise(t, class, func() { vm.raiseTransportErr(timeoutErr{}, phase) }) + } + vmS := New(io.Discard) + wantRaise(t, "SocketError", func() { vmS.raiseTransportErr(errors.New("boom"), "read") }) + + // nethttpReadResponse: a Content-Length that cannot parse is a framing error. + _, _, err := nethttpReadResponse(bufio.NewReader(strings.NewReader( + "HTTP/1.1 200 OK\r\nContent-Length: xx\r\n\r\n")), false) + if err == nil { + t.Error("bad Content-Length: expected error") + } + // nethttpReadResponse: an EOF before the status line. + if _, _, err := nethttpReadResponse(bufio.NewReader(strings.NewReader("")), false); err == nil { + t.Error("empty stream: expected error") + } + // nethttpReadResponse: a truncated header block (EOF before the blank line). + if _, _, err := nethttpReadResponse(bufio.NewReader(strings.NewReader("HTTP/1.1 200 OK\r\n")), false); err == nil { + t.Error("truncated headers: expected error") + } + // nethttpReadResponse: a body shorter than Content-Length is an EOF error. + if _, _, err := nethttpReadResponse(bufio.NewReader(strings.NewReader( + "HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nhi")), false); err == nil { + t.Error("short body: expected error") + } + // nethttpReadResponse: an unframed body (no Content-Length, no chunked) reads to + // EOF and marks the connection non-reusable. + raw, keep, err := nethttpReadResponse(bufio.NewReader(strings.NewReader( + "HTTP/1.1 200 OK\r\n\r\nunframed")), false) + if err != nil || keep { + t.Errorf("unframed: err=%v keep=%v, want nil/false", err, keep) + } + if !strings.HasSuffix(string(raw), "unframed") { + t.Errorf("unframed raw = %q", raw) + } + // nethttpReadResponse: a repeated header key accumulates (both-values arm). + raw, _, err = nethttpReadResponse(bufio.NewReader(strings.NewReader( + "HTTP/1.1 200 OK\r\nX-A: 1\r\nX-A: 2\r\nContent-Length: 0\r\n\r\n")), false) + if err != nil { + t.Errorf("dup-header: %v", err) + } + // nethttpReadResponse: a chunked body whose framing is broken propagates the + // chunk error. + if _, _, err := nethttpReadResponse(bufio.NewReader(strings.NewReader( + "HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\nzz\r\n")), false); err == nil { + t.Error("chunked framing error: expected error") + } + // nethttpReadResponse: an unframed body whose read fails after the headers + // surfaces the read error (the io.ReadAll error arm). + if _, _, err := nethttpReadResponse(bufio.NewReader(&prefixErrReader{ + prefix: []byte("HTTP/1.1 200 OK\r\n\r\n")}), false); err == nil { + t.Error("unframed read error: expected error") + } + // nethttpCopyChunked: a chunk size that overflows int64 is a framing error. + b0 := &bytes.Buffer{} + if err := nethttpCopyChunked(bufio.NewReader(strings.NewReader("fffffffffffffffff\r\n")), b0); err == nil { + t.Error("chunk-size overflow: expected error") + } + // nethttpCopyChunked: an EOF while consuming the trailing CRLF after the data. + b0.Reset() + if err := nethttpCopyChunked(bufio.NewReader(strings.NewReader("1\r\nX")), b0); err == nil { + t.Error("missing chunk CRLF: expected error") + } + + // nethttpCopyChunked: a bad chunk-size line and a mid-chunk EOF both error. + var b bytes.Buffer + if err := nethttpCopyChunked(bufio.NewReader(strings.NewReader("zzz\r\n")), &b); err == nil { + t.Error("bad chunk size: expected error") + } + b.Reset() + if err := nethttpCopyChunked(bufio.NewReader(strings.NewReader("5\r\nab")), &b); err == nil { + t.Error("truncated chunk data: expected error") + } + b.Reset() + if err := nethttpCopyChunked(bufio.NewReader(strings.NewReader("")), &b); err == nil { + t.Error("EOF at chunk size: expected error") + } + + // nethttpProxyConnect: a non-2xx CONNECT reply and a mid-header EOF both error. + if err := nethttpProxyConnect(&scriptConn{r: strings.NewReader("HTTP/1.1 407 Denied\r\n\r\n")}, "h:1", ""); err == nil { + t.Error("CONNECT non-2xx: expected error") + } + if err := nethttpProxyConnect(&scriptConn{r: strings.NewReader("HTTP/1.1 20")}, "h:1", "Basic x"); err == nil { + t.Error("CONNECT truncated: expected error") + } + // nethttpProxyConnect: a write failure surfaces. + if err := nethttpProxyConnect(&scriptConn{writeErr: io.ErrClosedPipe}, "h:1", ""); err == nil { + t.Error("CONNECT write error: expected error") + } + // nethttpProxyConnect: a 2xx reply succeeds. + if err := nethttpProxyConnect(&scriptConn{r: strings.NewReader("HTTP/1.1 200 OK\r\n\r\n")}, "h:1", ""); err != nil { + t.Errorf("CONNECT 2xx: %v", err) + } +} + +// TestNetHTTPProxyNilArms covers nethttpNewInstance's proxy-argument arms not +// exercised elsewhere: a nil / :ENV / empty p_addr leaves the instance proxy-less, +// and proxy_user / proxy_pass default to nil. +func TestNetHTTPProxyNilArms(t *testing.T) { + got := runSrc(t, `require "net/http" +puts Net::HTTP.new("h", 80).proxy? +puts Net::HTTP.new("h", 80, nil).proxy? +puts Net::HTTP.new("h", 80, :ENV).proxy? +puts Net::HTTP.new("h", 80, "").proxy? +h = Net::HTTP.new("h", 80, "px", 3128) +puts h.proxy? +p h.proxy_user +p h.proxy_pass`) + if want := "false\nfalse\nfalse\nfalse\ntrue\nnil\nnil"; got != want { + t.Fatalf("proxy nil-arms = %q, want %q", got, want) + } +} + +// TestNetHTTPDialXferError covers nethttpDialXfer's proxy dial-error arms (both the +// plain-http and the CONNECT-tunnel dial) against a refused proxy endpoint, via a +// started instance so the persistent dial path is taken. +func TestNetHTTPDialXferError(t *testing.T) { + ln, _ := net.Listen("tcp", "127.0.0.1:0") + refused := ln.Addr().(*net.TCPAddr) + ln.Close() + port := fmt.Sprintf("%d", refused.Port) + + // Plain-http proxy dial refused. + got := runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new("backend", 80, "127.0.0.1", %s) +begin; h.get("/"); rescue SocketError; puts "httpdial"; end`, port)) + if got != "httpdial" { + t.Fatalf("proxy http dial = %q, want httpdial", got) + } + // CONNECT-tunnel proxy dial refused (use_ssl -> connectTunnel). + got = runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new("backend", 443, "127.0.0.1", %s) +h.use_ssl = true +begin; h.get("/"); rescue SocketError; puts "connectdial"; end`, port)) + if got != "connectdial" { + t.Fatalf("proxy connect dial = %q, want connectdial", got) + } + // Direct (non-proxy) started-instance dial refused (persistent open-error arm). + got = runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new("127.0.0.1", %s) +begin; h.start { h.get("/") }; rescue SocketError; puts "directdial"; end`, port)) + if got != "directdial" { + t.Fatalf("direct started dial = %q, want directdial", got) + } +} + +// prefixErrReader yields prefix once, then fails every read with io.ErrUnexpectedEOF, +// so nethttpReadResponse can read a header block and then hit a body read error. +type prefixErrReader struct { + prefix []byte + done bool +} + +func (r *prefixErrReader) Read(p []byte) (int, error) { + if !r.done { + n := copy(p, r.prefix) + r.done = true + return n, nil + } + return 0, io.ErrUnexpectedEOF +} + +// TestNetHTTPKeepAliveRetry proves the stale-connection retry arm: a raw server +// that answers one request per connection (keep-alive implied, but the socket is +// then closed) makes the second request in a start block find its cached +// connection dead, drop it and redial — so both bodies are correct and two +// connections are used. +func TestNetHTTPKeepAliveRetry(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + var accepts int64 + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + seq := atomic.AddInt64(&accepts, 1) + go func(c net.Conn, n int64) { + // Read the request headers, answer with a keep-alive-looking response + // (no Connection: close) but then close the socket, so a reused + // connection is dead on the next request. + br := bufio.NewReader(c) + for { + line, err := br.ReadString('\n') + if err != nil || strings.TrimRight(line, "\r\n") == "" { + break + } + } + body := fmt.Sprintf("R%d", n) + fmt.Fprintf(c, "HTTP/1.1 200 OK\r\nContent-Length: %d\r\n\r\n%s", len(body), body) + c.Close() + }(conn, seq) + } + }() + + addr := ln.Addr().(*net.TCPAddr) + got := runSrc(t, fmt.Sprintf(`require "net/http" +h = Net::HTTP.new("127.0.0.1", %d) +h.start do + puts h.get("/").body + puts h.get("/").body +end`, addr.Port)) + if want := "R1\nR2"; got != want { + t.Fatalf("keep-alive retry bodies = %q, want %q", got, want) + } + if n := atomic.LoadInt64(&accepts); n != 2 { + t.Fatalf("keep-alive retry used %d connections, want 2", n) + } +} + +// base64Std is the std base64 of s, for building expected Basic credentials. +func base64Std(s string) string { + return base64.StdEncoding.EncodeToString([]byte(s)) +} + +// timeoutErr is a net.Error whose Timeout() is true, for raiseTransportErr. +type timeoutErr struct{} + +func (timeoutErr) Error() string { return "i/o timeout" } +func (timeoutErr) Timeout() bool { return true } +func (timeoutErr) Temporary() bool { return false } + +// scriptConn is a minimal net.Conn whose reads come from r and whose writes +// optionally fail with writeErr, for driving nethttpProxyConnect off a socket. +type scriptConn struct { + r io.Reader + writeErr error +} + +func (c *scriptConn) Read(p []byte) (int, error) { + if c.r == nil { + return 0, io.EOF + } + return c.r.Read(p) +} +func (c *scriptConn) Write(p []byte) (int, error) { + if c.writeErr != nil { + return 0, c.writeErr + } + return len(p), nil +} +func (c *scriptConn) Close() error { return nil } +func (c *scriptConn) LocalAddr() net.Addr { return nil } +func (c *scriptConn) RemoteAddr() net.Addr { return nil } +func (c *scriptConn) SetDeadline(_ time.Time) error { return nil } +func (c *scriptConn) SetReadDeadline(_ time.Time) error { return nil } +func (c *scriptConn) SetWriteDeadline(_ time.Time) error { return nil } From 662d0618eeac2bd4d5efbff0cc4c88cddbdc2443 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 6 Jul 2026 21:37:15 +0200 Subject: [PATCH 3/4] fix(net-http): map a write-deadline timeout to Net::WriteTimeout nethttpExchangeFramed now reports which half (write/read) failed so a write-deadline timeout raises Net::WriteTimeout instead of being misreported as Net::ReadTimeout, matching MRI. Co-Authored-By: Claude Opus 4.8 --- internal/vm/nethttp_bind.go | 17 ++++++++++------- internal/vm/nethttp_bind_test.go | 12 ++++++------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/internal/vm/nethttp_bind.go b/internal/vm/nethttp_bind.go index 025c6118..38f90edc 100644 --- a/internal/vm/nethttp_bind.go +++ b/internal/vm/nethttp_bind.go @@ -432,7 +432,7 @@ func (vm *VM) nethttpDoXfer(cfg *nethttpXfer, method, path string, body []byte, if persistent { if s := nethttpGetConn(cfg.inst); s != nil { reused = true - raw, keepAlive, err = vm.nethttpExchangeFramed(cfg, s, reqBytes, noBody) + raw, keepAlive, _, err = vm.nethttpExchangeFramed(cfg, s, reqBytes, noBody) if err != nil { nethttpDropConn(cfg.inst) reused = false @@ -444,10 +444,11 @@ func (vm *VM) nethttpDoXfer(cfg *nethttpXfer, method, path string, body []byte, if derr != nil { vm.raiseTransportErr(derr, "open") } - raw, keepAlive, err = vm.nethttpExchangeFramed(cfg, stream, reqBytes, noBody) + var phase string + raw, keepAlive, phase, err = vm.nethttpExchangeFramed(cfg, stream, reqBytes, noBody) if err != nil { stream.closeConn() - vm.raiseTransportErr(err, "read") + vm.raiseTransportErr(err, phase) } if persistent { nethttpSetConn(cfg.inst, stream) @@ -512,14 +513,16 @@ func (vm *VM) nethttpBuildRequest(cfg *nethttpXfer, method, path string, body [] // exactly one framed response (under the read deadline) off the stream, leaving // any following bytes buffered for the next keep-alive request. keepAlive reports // whether the connection may be reused (false ⇒ the server asked to close or the -// body was unframed and read to EOF). -func (vm *VM) nethttpExchangeFramed(cfg *nethttpXfer, s streamIO, reqBytes []byte, noBody bool) (raw []byte, keepAlive bool, err error) { +// body was unframed and read to EOF). phase names which half failed ("write" / +// "read") so a deadline timeout maps to Net::WriteTimeout vs Net::ReadTimeout. +func (vm *VM) nethttpExchangeFramed(cfg *nethttpXfer, s streamIO, reqBytes []byte, noBody bool) (raw []byte, keepAlive bool, phase string, err error) { nethttpSetDeadline(s, cfg.writeTO) if _, werr := s.writer().Write(reqBytes); werr != nil { - return nil, false, werr + return nil, false, "write", werr } nethttpSetDeadline(s, cfg.readTO) - return nethttpReadResponse(s.reader(), noBody) + raw, keepAlive, err = nethttpReadResponse(s.reader(), noBody) + return raw, keepAlive, "read", err } // nethttpDialXfer opens the transport for a transfer: a direct dial, a plain-http diff --git a/internal/vm/nethttp_bind_test.go b/internal/vm/nethttp_bind_test.go index 6ef4beb0..a46a7fcd 100644 --- a/internal/vm/nethttp_bind_test.go +++ b/internal/vm/nethttp_bind_test.go @@ -461,13 +461,13 @@ func (f *fakeStream) closeConn() error { return nil } func TestNetHTTPExchangeErrors(t *testing.T) { vm := New(io.Discard) cfg := &nethttpXfer{} - // Write error: the writer fails immediately. - if _, _, err := vm.nethttpExchangeFramed(cfg, &fakeStream{w: failWriter{}, r: bufio.NewReader(strings.NewReader(""))}, []byte("x"), false); err != io.ErrClosedPipe { - t.Errorf("write-error arm = %v, want ErrClosedPipe", err) + // Write error: the writer fails immediately (phase "write"). + if _, _, phase, err := vm.nethttpExchangeFramed(cfg, &fakeStream{w: failWriter{}, r: bufio.NewReader(strings.NewReader(""))}, []byte("x"), false); err != io.ErrClosedPipe || phase != "write" { + t.Errorf("write-error arm = (%q,%v), want (write,ErrClosedPipe)", phase, err) } - // Read error: the write succeeds, the read fails. - if _, _, err := vm.nethttpExchangeFramed(cfg, &fakeStream{w: io.Discard, r: bufio.NewReader(errReader{})}, []byte("x"), false); err != io.ErrUnexpectedEOF { - t.Errorf("read-error arm = %v, want ErrUnexpectedEOF", err) + // Read error: the write succeeds, the read fails (phase "read"). + if _, _, phase, err := vm.nethttpExchangeFramed(cfg, &fakeStream{w: io.Discard, r: bufio.NewReader(errReader{})}, []byte("x"), false); err != io.ErrUnexpectedEOF || phase != "read" { + t.Errorf("read-error arm = (%q,%v), want (read,ErrUnexpectedEOF)", phase, err) } // trimResponseToHeaders returns its input unchanged when there is no header // terminator (the i < 0 arm). From d73ebefe285fbeae390f6a948b24c264d221299c Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 6 Jul 2026 21:40:17 +0200 Subject: [PATCH 4/4] fix(net-http): use explicit host:port for the CONNECT authority hostHdr omits the port for a default-port target; CONNECT requires an explicit host:port, so build it from host + port. Co-Authored-By: Claude Opus 4.8 --- internal/vm/nethttp_bind.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/vm/nethttp_bind.go b/internal/vm/nethttp_bind.go index 38f90edc..459e30cd 100644 --- a/internal/vm/nethttp_bind.go +++ b/internal/vm/nethttp_bind.go @@ -546,7 +546,9 @@ func (vm *VM) nethttpDialXfer(cfg *nethttpXfer) (streamIO, error) { if err != nil { return nil, err } - if err := nethttpProxyConnect(conn, cfg.hostHdr, cfg.proxyAuth); err != nil { + // CONNECT always names an explicit host:port authority (hostHdr omits the port + // for a default-port target, which CONNECT does not permit). + if err := nethttpProxyConnect(conn, net.JoinHostPort(cfg.host, cfg.port), cfg.proxyAuth); err != nil { conn.Close() return nil, err }