diff --git a/README.md b/README.md index ad4afc3..629d338 100644 --- a/README.md +++ b/README.md @@ -760,12 +760,12 @@ To see how well the cache works: - Request headers that do not come in 10 s get HTTP 408, and a request whose body does not come in 300 s too. This is also true in a tunnel and with `proxyProtocol`. -- A `CONNECT` tunnel that sends no byte for 10 s after the `200` closes. A - tunnel that sends and gets no byte for 10 minutes while a request waits - for its response closes. Between two requests, a tunnel closes after - about 5 s without a new request (the `keepAliveTimeout` of Node), and - the client opens a new one. A gap: in a tunnel without TLS, Node removes - the 10-minute limit after the first request. +- A `CONNECT` tunnel that sends no byte for 10 s after the `200` closes. +- A connection or tunnel that sends and gets no byte for 10 minutes + closes, also while a request waits for its response. This is true for + each request, with or without TLS. Between two requests, a connection + closes after 6 s without a new request (the `keepAliveTimeout` of Node, + 5 s, and 1 s that Node adds), and the client opens a new one. - The credentials that a run learns and the values that the sessions of a run without the token remember have limits. See [Config](#config) and [Changing values](#changing-values). diff --git a/packages/service/src/server.ts b/packages/service/src/server.ts index 0dd2210..4c489f4 100644 --- a/packages/service/src/server.ts +++ b/packages/service/src/server.ts @@ -70,11 +70,12 @@ const MAX_BODY_BYTES = 64 * 1024 * 1024; /** A tunnel that sends no byte this long after its `200` closes. */ const TUNNEL_FIRST_BYTE_MS = 10_000; /** - * A tunnel that sends and gets no byte this long closes. A live model - * call can take minutes before its first byte, so this is long. Between - * two requests, Node closes the tunnel sooner (`keepAliveTimeout`, 5 s). + * A connection or tunnel that sends and gets no byte this long closes. A + * live model call can take minutes before its first byte, so this is long. + * Between two requests, Node closes the connection sooner + * (`keepAliveTimeout`, 5 s, and 1 s that Node adds). */ -const TUNNEL_IDLE_MS = 10 * 60 * 1000; +const IDLE_MS = 10 * 60 * 1000; /** * The HTTP servers of the proxy. Headers that do not come in 10 s get * HTTP 408 from Node. A whole request, with its body, has Node's default @@ -520,6 +521,11 @@ export async function listenProxy( } serveProxied(tunnel, incoming, outgoing, tunnel.origin); }); + // Between two requests, Node sets the time limit of a socket to its + // `keepAliveTimeout`, and at the next request back to `timeout`. With + // the default `timeout` of 0, a later request in a tunnel without TLS + // had no idle limit. A TLS tunnel also keeps the limit of its raw socket. + tunnelServer.timeout = IDLE_MS; const server = http.createServer(HTTP_OPTIONS, (incoming, outgoing) => { // A request to the proxy itself has a path. A proxied plain HTTP // request has an absolute URL. @@ -550,6 +556,10 @@ export async function listenProxy( const listener = options.proxyProtocol ? net.createServer((socket) => acceptProxied(socket, server)) : server; + // Node's `requestTimeout` stops when a request is read. Without this, a + // request that waits on a hung upstream, or a client that does not read + // its response, holds the connection with no limit. + server.timeout = IDLE_MS; listener.on("connection", (socket: Socket) => { sockets.add(socket); socket.on("close", () => sockets.delete(socket)); @@ -597,7 +607,7 @@ export async function listenProxy( } socket.write("HTTP/1.1 200 Connection Established\r\n\r\n"); // A tunnel that a client leaves open must not stay open forever. - socket.setTimeout(TUNNEL_IDLE_MS, () => socket.destroy()); + socket.setTimeout(IDLE_MS, () => socket.destroy()); const open = async (first: Buffer) => { socket.unshift(first); // A TLS handshake starts with byte 0x16. Anything else is plain HTTP. diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index c86f8dc..a3fde9b 100644 --- a/packages/service/tests/service.test.ts +++ b/packages/service/tests/service.test.ts @@ -1450,6 +1450,59 @@ describe("limits of the service", () => { ]), ).resolves.toBe("closed"); }); + it.each(["a tunnel without TLS", "an absolute-form connection"])( + "keeps the idle limit of %s after its first request", + async (kind) => { + // Between two requests, Node sets the time limit of a socket to its + // keep-alive time, and at the next request to the `timeout` of the + // server. Before, that was 0, so a later request had no idle limit, + // also after Node read all of it (`requestTimeout` stops there). + const run = await startRun(undefined, "replay"); + const host = new URL(origin).host; + let socket: ReturnType; + let post: string; + if (kind === "a tunnel without TLS") { + socket = await openTunnel(service.url, run.url); + post = `POST /v1/messages HTTP/1.1\r\nhost: ${host}\r\ncontent-length: 2\r\n\r\n`; + } else { + const { username, password } = new URL(run.url); + const credentials = Buffer.from( + `${decodeURIComponent(username)}:${decodeURIComponent(password)}`, + ).toString("base64"); + socket = connect(Number(new URL(service.url).port), "127.0.0.1"); + onTestFinished(() => { + socket.destroy(); + }); + await once(socket, "connect"); + post = `POST ${origin}/v1/messages HTTP/1.1\r\nhost: ${host}\r\nproxy-authorization: Basic ${credentials}\r\ncontent-length: 2\r\n\r\n`; + } + // The service has its end of the connection in a socket of the same + // class. + const socketClass = Object.getPrototypeOf(socket) as typeof socket; + const setTimeoutSpy = vi.spyOn(socketClass, "setTimeout"); + onTestFinished(() => setTimeoutSpy.mockRestore()); + const port = Number(new URL(service.url).port); + /** The time limits that the service set on its end of the connection. */ + const limits = () => + setTimeoutSpy.mock.calls.flatMap(([ms], n) => { + const peer = setTimeoutSpy.mock.contexts[n] as typeof socket; + const ours = + peer.localPort === port && peer.remotePort === socket.localPort; + return ours ? [ms] : []; + }); + socket.write(`${post}{}`); + const [first] = (await once(socket, "data")) as [Buffer]; + expect(first.toString()).toMatch(/^HTTP\/1.1 412 /); + // Node waits 5 s and 1 s more for the next request. + await until(() => limits().at(-1) === 6000); + const before = limits().length; + // A second request whose body does not come. + socket.write(`${post}{`); + await until(() => limits().length > before); + expect(limits().at(-1)).toBe(10 * 60 * 1000); + }, + ); + it("times out slow headers, also with proxyProtocol and in a tunnel", async () => { // With `proxyProtocol`, the HTTP servers get their connections from a // TCP server, and Node did not check their time limits.