From 8041c13f78daee777381b61ee984749cfab95d07 Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Wed, 14 Oct 2026 01:29:06 +0000 Subject: [PATCH 1/2] In a terminated tunnel, what is queued for the client survives the TLS socket Under Bun the TLS socket runs on a stream over the tunnel's socket, and has finished once it has written there, long before the client has it all. Destroyed then, it took the stream and the socket with it, and what was queued was dropped. Bun 1.4.3's HTTP server does that after every HTTP/1.0 response (destroySoon): the client lost the tail of a long answer, with no error where there is no Content-Length. A destroy with no error, once the TLS socket has finished, now counts as the stream's end, and the client is given all it holds. And once the stream has handed the socket to closeAfterFlush, its own destruction leaves the socket alone: that cut the close short, and a client with bytes nobody had read lost what the kernel still held for it. --- src/sandbox/tls-terminate-proxy.ts | 31 ++++- test/sandbox/duplex-over-socket.test.ts | 148 ++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 test/sandbox/duplex-over-socket.test.ts diff --git a/src/sandbox/tls-terminate-proxy.ts b/src/sandbox/tls-terminate-proxy.ts index a0ce8acd5..571bb1733 100644 --- a/src/sandbox/tls-terminate-proxy.ts +++ b/src/sandbox/tls-terminate-proxy.ts @@ -150,14 +150,17 @@ export function peekForClientHello( * Bun's TLS socket keeps reading the duplex under it while it is itself * paused, so once `consumer` (the TLS socket) is set, nothing more is read * from the client while that is paused: its uploads wait in the kernel. + * + * @internal Exported for its test. */ -function duplexOverSocket( +export function duplexOverSocket( socket: Duplex, head: Buffer, onClosed: (how: string) => void, -): { duplex: Duplex; setConsumer: (consumer: Readable) => void } { +): { duplex: Duplex; setConsumer: (consumer: Duplex) => void } { let waiting = false - let consumer: Readable | undefined + let closing = false + let consumer: Duplex | undefined let waitingForConsumer = false const onConsumerResume = (): void => { waitingForConsumer = false @@ -194,14 +197,30 @@ function duplexOverSocket( }, final(cb) { socket.off('readable', onReadable) + closing = true closeAfterFlush(socket, onClosed) cb() }, destroy(err, cb) { - socket.destroy() + // From `final` on the socket is closeAfterFlush's to close. + if (err || !closing) socket.destroy() cb(err) }, }) + // A TLS socket on this stream has finished once it has written here, long + // before the client has it all, and may then be destroyed: by a server's + // destroySoon(), or because its client has ended too. That would drop what + // is queued here. So it counts as an end (which, depending on the runtime, + // has or has not come first), and the client is given all of it. + const destroy = d.destroy.bind(d) + d.destroy = (err?: Error) => { + const done = d.writableEnded || consumer?.writableFinished + if (err || !done || d.writableFinished || socket.destroyed) { + return destroy(err) + } + if (!d.writableEnded) d.end() + return d + } if (head.length) d.push(head) socket.once('end', () => d.push(null)) // The socket's own errors are handled where the tunnel starts; here they @@ -224,8 +243,10 @@ function duplexOverSocket( * reads in that socket's own buffer too, which grows by the whole upload. * `outbound` is the stream whose write queue holds what is on its way to * the client. + * + * @internal Exported for its test. */ -function serverTlsOn( +export function serverTlsOn( socket: Duplex, head: Buffer, options: TLSSocketOptions, diff --git a/test/sandbox/duplex-over-socket.test.ts b/test/sandbox/duplex-over-socket.test.ts new file mode 100644 index 000000000..8fc88c289 --- /dev/null +++ b/test/sandbox/duplex-over-socket.test.ts @@ -0,0 +1,148 @@ +import { afterEach, describe, expect, test } from 'bun:test' +import { connect, createServer, type AddressInfo, type Socket } from 'node:net' +import { join } from 'node:path' +import type { Duplex } from 'node:stream' +import { connect as tlsConnect } from 'node:tls' +import { createMitmCA } from '../../src/sandbox/mitm-ca.js' +import { mintLeafCert } from '../../src/sandbox/mitm-leaf.js' +import { + duplexOverSocket, + serverTlsOn, +} from '../../src/sandbox/tls-terminate-proxy.js' + +// More than the kernel takes for a client that is not reading. +const SIZE = 32 << 20 + +const FIXTURES = join(import.meta.dir, '..', 'fixtures', 'tls-terminate') +const LEAF = (): { cert: string; key: string } => { + const leaf = mintLeafCert( + createMitmCA({ + caCertPath: join(FIXTURES, 'ca.crt'), + caKeyPath: join(FIXTURES, 'ca.key'), + }), + 'localhost', + ) + return { cert: leaf.certPem, key: leaf.keyPem } +} + +const cleanup: Array<() => void> = [] +afterEach(() => { + while (cleanup.length) cleanup.pop()!() +}) + +/** + * What a client reads, and whether it saw the end, from a server that hands + * `serve` the stream over its socket. The client reads nothing for a while, + * so `serve` is done long before its bytes are taken, and then slowly, so the + * kernel still holds some whenever the stream finishes. `unread` bytes it + * sends first, which nobody reads. + */ +async function lateClientOf( + serve: (d: Duplex, socket: Socket) => void, + unread = 0, + overTls = false, +): Promise<{ read: number; ended: boolean }> { + // As an http server's sockets are, a CONNECT's among them. + const server = createServer({ allowHalfOpen: true }, socket => { + socket.on('error', () => {}) + if (!overTls) { + const { duplex } = duplexOverSocket(socket, Buffer.alloc(0), () => {}) + return serve(duplex, socket) + } + const { tls } = serverTlsOn(socket, Buffer.alloc(0), LEAF(), () => {}) + tls.on('error', () => {}) + tls.once('secure', () => serve(tls, socket)) + }) + cleanup.push(() => server.close()) + await new Promise(r => server.listen(0, '127.0.0.1', r)) + return new Promise(resolve => { + const to = { + port: (server.address() as AddressInfo).port, + host: '127.0.0.1', + allowHalfOpen: true, + } + const c = overTls + ? tlsConnect({ ...to, rejectUnauthorized: false }) + : connect(to) + let read = 0 + let ended = false + c.pause() + if (unread) c.write(Buffer.alloc(unread, 0x62)) + setTimeout(() => c.resume(), 300) + c.on('data', d => { + read += d.length + c.pause() + setTimeout(() => c.resume(), 5) + }) + c.on('end', () => { + ended = true + c.end() + }) + c.on('error', () => {}) + c.on('close', () => resolve({ read, ended })) + }) +} + +/** All at once, as Bun's TLS socket writes to the stream under it. */ +function writeAll(d: Duplex): void { + const chunk = Buffer.alloc(64 << 10, 0x61) + for (let sent = 0; sent < SIZE; sent += chunk.length) d.write(chunk) +} + +describe('serverTlsOn', () => { + // What a runtime's http server does, depending on the runtime, once it has + // answered an HTTP/1.0 request. + test('ended, then destroySoon(): the client gets every byte, then the end', async () => { + const got = await lateClientOf( + tls => { + writeAll(tls) + tls.end() + ;(tls as Socket).destroySoon() + }, + 0, + true, + ) + expect(got).toEqual({ read: SIZE, ended: true }) + }, 60_000) +}) + +describe('duplexOverSocket', () => { + test('ended, and destroyed with bytes still queued: the client gets them all, then the end', async () => { + let queued = 0 + const got = await lateClientOf(d => { + writeAll(d) + d.end() + queued = d.writableLength + d.destroy() + }) + expect(queued).toBeGreaterThan(0) + expect(got).toEqual({ read: SIZE, ended: true }) + }, 60_000) + + test('destroyed once it has finished: the close goes on, and bytes of the client that nobody read cost it nothing', async () => { + const got = await lateClientOf(d => { + writeAll(d) + d.end() + d.once('finish', () => d.destroy()) + }, 4 << 20) + expect(got).toEqual({ read: SIZE, ended: true }) + }, 60_000) + + test.each([ + ['with an error', (d: Duplex) => (d.end(), d.destroy(new Error('x')))], + ['without having been ended', (d: Duplex) => d.destroy()], + ])( + 'destroyed %s: cut at once', + async (_how, cut) => { + let destroyed: boolean | undefined + const got = await lateClientOf((d, socket) => { + writeAll(d) + cut(d) + destroyed = socket.destroyed + }) + expect(destroyed).toBe(true) + expect(got.read).toBeLessThan(SIZE) + }, + 60_000, + ) +}) From 86fb8052be75419686e0fb4ee10c0b0ebe0282bc Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Wed, 14 Oct 2026 02:25:42 +0000 Subject: [PATCH 2/2] destroySoon() on a tunnel's TLS socket waits for the flush; destroy() cuts again The commit before this one counted every destroy() with no error, once the TLS socket had finished, as the end of the stream under it. That cured the lost tail of an HTTP/1.0 answer, but it softened the proxy's own back-stops, which destroy a tunnel a second after ending it: with no mux in front (srt proxy) a refused client that never closes was held for 5 to 6 s, not 1 s. The two calls mean different things. destroy() is to cut now. destroySoon() is to destroy once all that was written is out, and under Bun it believes that too early, because the TLS socket has finished as soon as it has written to the stream under it. So destroySoon() now ends that socket, which closes the tunnel after the flush, and the stream is as it was. --- src/sandbox/tls-terminate-proxy.ts | 33 ++-- test/sandbox/duplex-over-socket.test.ts | 148 ------------------ .../tls-terminate-destroy-soon.test.ts | 108 +++++++++++++ 3 files changed, 118 insertions(+), 171 deletions(-) delete mode 100644 test/sandbox/duplex-over-socket.test.ts create mode 100644 test/sandbox/tls-terminate-destroy-soon.test.ts diff --git a/src/sandbox/tls-terminate-proxy.ts b/src/sandbox/tls-terminate-proxy.ts index 571bb1733..15af7f61c 100644 --- a/src/sandbox/tls-terminate-proxy.ts +++ b/src/sandbox/tls-terminate-proxy.ts @@ -150,17 +150,14 @@ export function peekForClientHello( * Bun's TLS socket keeps reading the duplex under it while it is itself * paused, so once `consumer` (the TLS socket) is set, nothing more is read * from the client while that is paused: its uploads wait in the kernel. - * - * @internal Exported for its test. */ -export function duplexOverSocket( +function duplexOverSocket( socket: Duplex, head: Buffer, onClosed: (how: string) => void, -): { duplex: Duplex; setConsumer: (consumer: Duplex) => void } { +): { duplex: Duplex; setConsumer: (consumer: Readable) => void } { let waiting = false - let closing = false - let consumer: Duplex | undefined + let consumer: Readable | undefined let waitingForConsumer = false const onConsumerResume = (): void => { waitingForConsumer = false @@ -197,30 +194,14 @@ export function duplexOverSocket( }, final(cb) { socket.off('readable', onReadable) - closing = true closeAfterFlush(socket, onClosed) cb() }, destroy(err, cb) { - // From `final` on the socket is closeAfterFlush's to close. - if (err || !closing) socket.destroy() + socket.destroy() cb(err) }, }) - // A TLS socket on this stream has finished once it has written here, long - // before the client has it all, and may then be destroyed: by a server's - // destroySoon(), or because its client has ended too. That would drop what - // is queued here. So it counts as an end (which, depending on the runtime, - // has or has not come first), and the client is given all of it. - const destroy = d.destroy.bind(d) - d.destroy = (err?: Error) => { - const done = d.writableEnded || consumer?.writableFinished - if (err || !done || d.writableFinished || socket.destroyed) { - return destroy(err) - } - if (!d.writableEnded) d.end() - return d - } if (head.length) d.push(head) socket.once('end', () => d.push(null)) // The socket's own errors are handled where the tunnel starts; here they @@ -263,6 +244,12 @@ export function serverTlsOn( isServer: true, }) setConsumer(tls) + // destroySoon() is to destroy a socket once all it was given is out. This + // one has finished as soon as it has written to the stream under it, long + // before the client has the bytes, and destroyed then it drops them. (A + // runtime's http server may call it after an HTTP/1.0 response.) Ended, it + // closes the tunnel too, after the flush. destroy() still cuts at once. + tls.destroySoon = () => void tls.end() return { tls, outbound: duplex } } diff --git a/test/sandbox/duplex-over-socket.test.ts b/test/sandbox/duplex-over-socket.test.ts deleted file mode 100644 index 8fc88c289..000000000 --- a/test/sandbox/duplex-over-socket.test.ts +++ /dev/null @@ -1,148 +0,0 @@ -import { afterEach, describe, expect, test } from 'bun:test' -import { connect, createServer, type AddressInfo, type Socket } from 'node:net' -import { join } from 'node:path' -import type { Duplex } from 'node:stream' -import { connect as tlsConnect } from 'node:tls' -import { createMitmCA } from '../../src/sandbox/mitm-ca.js' -import { mintLeafCert } from '../../src/sandbox/mitm-leaf.js' -import { - duplexOverSocket, - serverTlsOn, -} from '../../src/sandbox/tls-terminate-proxy.js' - -// More than the kernel takes for a client that is not reading. -const SIZE = 32 << 20 - -const FIXTURES = join(import.meta.dir, '..', 'fixtures', 'tls-terminate') -const LEAF = (): { cert: string; key: string } => { - const leaf = mintLeafCert( - createMitmCA({ - caCertPath: join(FIXTURES, 'ca.crt'), - caKeyPath: join(FIXTURES, 'ca.key'), - }), - 'localhost', - ) - return { cert: leaf.certPem, key: leaf.keyPem } -} - -const cleanup: Array<() => void> = [] -afterEach(() => { - while (cleanup.length) cleanup.pop()!() -}) - -/** - * What a client reads, and whether it saw the end, from a server that hands - * `serve` the stream over its socket. The client reads nothing for a while, - * so `serve` is done long before its bytes are taken, and then slowly, so the - * kernel still holds some whenever the stream finishes. `unread` bytes it - * sends first, which nobody reads. - */ -async function lateClientOf( - serve: (d: Duplex, socket: Socket) => void, - unread = 0, - overTls = false, -): Promise<{ read: number; ended: boolean }> { - // As an http server's sockets are, a CONNECT's among them. - const server = createServer({ allowHalfOpen: true }, socket => { - socket.on('error', () => {}) - if (!overTls) { - const { duplex } = duplexOverSocket(socket, Buffer.alloc(0), () => {}) - return serve(duplex, socket) - } - const { tls } = serverTlsOn(socket, Buffer.alloc(0), LEAF(), () => {}) - tls.on('error', () => {}) - tls.once('secure', () => serve(tls, socket)) - }) - cleanup.push(() => server.close()) - await new Promise(r => server.listen(0, '127.0.0.1', r)) - return new Promise(resolve => { - const to = { - port: (server.address() as AddressInfo).port, - host: '127.0.0.1', - allowHalfOpen: true, - } - const c = overTls - ? tlsConnect({ ...to, rejectUnauthorized: false }) - : connect(to) - let read = 0 - let ended = false - c.pause() - if (unread) c.write(Buffer.alloc(unread, 0x62)) - setTimeout(() => c.resume(), 300) - c.on('data', d => { - read += d.length - c.pause() - setTimeout(() => c.resume(), 5) - }) - c.on('end', () => { - ended = true - c.end() - }) - c.on('error', () => {}) - c.on('close', () => resolve({ read, ended })) - }) -} - -/** All at once, as Bun's TLS socket writes to the stream under it. */ -function writeAll(d: Duplex): void { - const chunk = Buffer.alloc(64 << 10, 0x61) - for (let sent = 0; sent < SIZE; sent += chunk.length) d.write(chunk) -} - -describe('serverTlsOn', () => { - // What a runtime's http server does, depending on the runtime, once it has - // answered an HTTP/1.0 request. - test('ended, then destroySoon(): the client gets every byte, then the end', async () => { - const got = await lateClientOf( - tls => { - writeAll(tls) - tls.end() - ;(tls as Socket).destroySoon() - }, - 0, - true, - ) - expect(got).toEqual({ read: SIZE, ended: true }) - }, 60_000) -}) - -describe('duplexOverSocket', () => { - test('ended, and destroyed with bytes still queued: the client gets them all, then the end', async () => { - let queued = 0 - const got = await lateClientOf(d => { - writeAll(d) - d.end() - queued = d.writableLength - d.destroy() - }) - expect(queued).toBeGreaterThan(0) - expect(got).toEqual({ read: SIZE, ended: true }) - }, 60_000) - - test('destroyed once it has finished: the close goes on, and bytes of the client that nobody read cost it nothing', async () => { - const got = await lateClientOf(d => { - writeAll(d) - d.end() - d.once('finish', () => d.destroy()) - }, 4 << 20) - expect(got).toEqual({ read: SIZE, ended: true }) - }, 60_000) - - test.each([ - ['with an error', (d: Duplex) => (d.end(), d.destroy(new Error('x')))], - ['without having been ended', (d: Duplex) => d.destroy()], - ])( - 'destroyed %s: cut at once', - async (_how, cut) => { - let destroyed: boolean | undefined - const got = await lateClientOf((d, socket) => { - writeAll(d) - cut(d) - destroyed = socket.destroyed - }) - expect(destroyed).toBe(true) - expect(got.read).toBeLessThan(SIZE) - }, - 60_000, - ) -}) diff --git a/test/sandbox/tls-terminate-destroy-soon.test.ts b/test/sandbox/tls-terminate-destroy-soon.test.ts new file mode 100644 index 000000000..632a888a6 --- /dev/null +++ b/test/sandbox/tls-terminate-destroy-soon.test.ts @@ -0,0 +1,108 @@ +import { afterEach, describe, expect, test } from 'bun:test' +import { readFileSync } from 'node:fs' +import { createServer, type AddressInfo } from 'node:net' +import { join } from 'node:path' +import { connect, type TLSSocket } from 'node:tls' +import { createMitmCA } from '../../src/sandbox/mitm-ca.js' +import { mintLeafCert } from '../../src/sandbox/mitm-leaf.js' +import { serverTlsOn } from '../../src/sandbox/tls-terminate-proxy.js' + +// More than the kernel takes for a client that is not reading. +const SIZE = 32 << 20 + +const FIXTURES = join(import.meta.dir, '..', 'fixtures', 'tls-terminate') +const LEAF = (): { cert: string; key: string } => { + const leaf = mintLeafCert( + createMitmCA({ + caCertPath: join(FIXTURES, 'ca.crt'), + caKeyPath: join(FIXTURES, 'ca.key'), + }), + 'localhost', + ) + return { cert: leaf.certPem, key: leaf.keyPem } +} + +const cleanup: Array<() => void> = [] +afterEach(() => { + while (cleanup.length) cleanup.pop()!() +}) + +/** + * What a TLS client reads, and whether it saw the end, from a server that + * writes SIZE bytes to its TLS socket, all at once, and hands it to `then`. + * The client reads nothing for a while, so `then` is done long before the + * bytes are taken, and then slowly. + */ +async function lateClientOf( + then: (tls: TLSSocket) => void, +): Promise<{ read: number; ended: boolean }> { + // As an http server's sockets are, a CONNECT's among them. + const server = createServer({ allowHalfOpen: true }, socket => { + socket.on('error', () => {}) + const { tls } = serverTlsOn(socket, Buffer.alloc(0), LEAF(), () => {}) + tls.on('error', () => {}) + tls.once('secure', () => { + const chunk = Buffer.alloc(64 << 10, 0x61) + for (let sent = 0; sent < SIZE; sent += chunk.length) tls.write(chunk) + then(tls) + }) + }) + cleanup.push(() => server.close()) + await new Promise(r => server.listen(0, '127.0.0.1', r)) + return new Promise(resolve => { + const to = { + port: (server.address() as AddressInfo).port, + host: '127.0.0.1', + allowHalfOpen: true, + servername: 'localhost', + ca: readFileSync(join(FIXTURES, 'ca.crt')), + } + const c = connect(to) + let read = 0 + let ended = false + c.pause() + setTimeout(() => c.resume(), 300) + c.on('data', d => { + read += d.length + c.pause() + setTimeout(() => c.resume(), 5) + }) + c.on('end', () => { + ended = true + c.end() + }) + c.on('error', () => {}) + c.on('close', () => resolve({ read, ended })) + }) +} + +describe('serverTlsOn', () => { + // What a runtime's http server does, depending on the runtime, once it has + // answered an HTTP/1.0 request. + test.each([ + ['end(), then destroySoon()', (t: TLSSocket) => (t.end(), t.destroySoon())], + ['destroySoon()', (t: TLSSocket) => t.destroySoon()], + ])( + '%s: the client gets every byte, then the end', + async (_how, close) => { + expect(await lateClientOf(close)).toEqual({ read: SIZE, ended: true }) + }, + 60_000, + ) + + // As the back-stops do, a while after an end that a client does not answer. + test.each([ + [ + 'end(), and destroy() once it has finished', + (t: TLSSocket) => t.end(() => setImmediate(() => t.destroy())), + ], + ['destroy()', (t: TLSSocket) => t.destroy()], + ])( + '%s: cut at once', + async (_how, cut) => { + // Long before the client reads its first byte. + expect((await lateClientOf(cut)).read).toBeLessThan(SIZE) + }, + 60_000, + ) +})