From ca446b2c30b3ecf5347d64e241a766af9989d52c Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 21:56:08 -0500 Subject: [PATCH] Read the tls.createServer SNICallback in source order The runtime call reads the cert and key options before the SNICallback, so a cert or key initializer after the callback that reassigned the callback's binding changed the callback the server used, and the server could serve the wrong certificate. When SNICallback comes before a cert or key entry, the callback is now saved just before the next of them is evaluated. With the callback read in order, a callback expression with side effects lowers instead of being refused. --- .../src/frontend/lowering/lower-server.ts | 27 +++++++++------ tests/corpus/6933-tls-sni-callback.ts | 34 ++++++++++++++++++- tests/diagnostics/tls-sni-callback-fences.ts | 2 -- .../tls-sni-callback-fences.ts.txt | 17 +++------- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/lower-server.ts b/packages/compiler/src/frontend/lowering/lower-server.ts index c27b2513e..058d7874a 100644 --- a/packages/compiler/src/frontend/lowering/lower-server.ts +++ b/packages/compiler/src/frontend/lowering/lower-server.ts @@ -4762,6 +4762,7 @@ function lowerTlsServerOptions( let cert: IrExpr | null = null; let key: IrExpr | null = null; let sni: IrExpr | undefined; + let pendingSni: IrStmt | null = null; for (const prop of node.properties.nodes) { let initializer: ts.Expression | null; if ( @@ -4780,20 +4781,22 @@ function lowerTlsServerOptions( } const k = (prop.name as ts.Identifier | ts.StringLiteral).text; if (k === "SNICallback" && allowSni) { - // The callback is a value read (a function or a binding), so it may - // be read after cert and key without changing what the program sees. - if (initializer !== null && !sideEffectFreeOptionValue(initializer)) { - lowerer.noLowering( - `a ${what} SNICallback expression with side effects`, - prop, - "pass a function or a variable that holds one", - ); - } const value = initializer !== null ? lowerer.lowerExpr(initializer) : lowerer.lowerShorthandValue(prop as ts.ShorthandPropertyAssignment); - sni = checkSniCallbackValue(lowerer, value, prop, what); + const checked = checkSniCallbackValue(lowerer, value, prop, what); + if (cert !== null && key !== null) { + sni = checked; + } else { + // The runtime call reads cert and key first. A cert or key entry + // after the callback can change the binding the callback reads, + // so the callback is saved ahead of the next of them, in source + // order. + const saved = lowerer.declareHiddenLocal("%sniCallback", checked.type); + pendingSni = { kind: "varDecl", localId: saved.id, init: checked, loc: locOf(prop) }; + sni = varRef(saved.id, checked.type, locOf(prop)); + } continue; } if (k !== "cert" && k !== "key") { @@ -4857,6 +4860,10 @@ function lowerTlsServerOptions( "cert/key are PEM strings or Buffers here", ); } + if (pendingSni !== null) { + v = { kind: "seqExpr", stmts: [pendingSni], result: v, type: v.type, loc }; + pendingSni = null; + } if (k === "cert") cert = v; else key = v; } diff --git a/tests/corpus/6933-tls-sni-callback.ts b/tests/corpus/6933-tls-sni-callback.ts index 7958882d5..8c9442806 100644 --- a/tests/corpus/6933-tls-sni-callback.ts +++ b/tests/corpus/6933-tls-sni-callback.ts @@ -33,6 +33,30 @@ const server = tls.createServer( }, ); +// The callback is read where it appears, effects included: a later cert +// initializer that reassigns its binding does not change the callback the +// server uses. +type Sni = (servername: string, cb: (err: Error | null, ctx?: tls.SecureContext) => void) => void; +let chosen: Sni = (servername, cb) => { + console.log("first callback", servername); + cb(null, altContext); +}; +const ordered = tls.createServer( + { + SNICallback: (console.log("read callback"), chosen), + cert: (console.log("read cert"), + (chosen = (servername, cb) => { + console.log("second callback", servername); + cb(null, undefined); + }), + cert), + key, + }, + (socket) => { + socket.end("ordered"); + }, +); + function attempt(port: number, servername: string, next: () => void): void { let done = false; const finish = (line: string): void => { @@ -58,7 +82,15 @@ server.listen(0, "127.0.0.1", () => { attempt(port, "localhost", () => attempt(port, "alt.localhost", () => attempt(port, "other.localhost", () => - attempt(port, "fail.localhost", () => server.close(() => console.log("closed"))), + attempt(port, "fail.localhost", () => + server.close(() => { + console.log("closed"); + ordered.listen(0, "127.0.0.1", () => { + const orderedPort = (ordered.address() as net.AddressInfo).port; + attempt(orderedPort, "alt.localhost", () => ordered.close(() => console.log("ordered closed"))); + }); + }), + ), ), ), ); diff --git a/tests/diagnostics/tls-sni-callback-fences.ts b/tests/diagnostics/tls-sni-callback-fences.ts index 0c745365c..20b6ce271 100644 --- a/tests/diagnostics/tls-sni-callback-fences.ts +++ b/tests/diagnostics/tls-sni-callback-fences.ts @@ -9,7 +9,5 @@ function makeSni(): (servername: string, cb: Answer) => void { return (_servername, cb) => cb(null, undefined); } -// The callback expression has side effects. -tls.createServer({ cert, key, SNICallback: makeSni() }, (socket) => socket.end()); // SNICallback with options the runtime options walk reads. tls.createServer({ cert, key, ALPNProtocols: ["h2"], SNICallback: makeSni }); diff --git a/tests/harness/__snapshots__/tls-sni-callback-fences.ts.txt b/tests/harness/__snapshots__/tls-sni-callback-fences.ts.txt index 920e65532..c45608bb9 100644 --- a/tests/harness/__snapshots__/tls-sni-callback-fences.ts.txt +++ b/tests/harness/__snapshots__/tls-sni-callback-fences.ts.txt @@ -1,17 +1,8 @@ -tls-sni-callback-fences.ts:13:31 - error SC2020: 'a tls.createServer SNICallback expression with side effects' is part of the standard library types but has no scriptc lowering yet +tls-sni-callback-fences.ts:13:54 - error SC2020: 'tls.createServer with SNICallback and options the runtime walk reads' is part of the standard library types but has no scriptc lowering yet - 12 | // The callback expression has side effects. - 13 | tls.createServer({ cert, key, SNICallback: makeSni() }, (socket) => socket.end()); - | ^~~~~~~~~~~~~~~~~~~~~~ - 14 | // SNICallback with options the runtime options walk reads. - - hint: pass a function or a variable that holds one - -tls-sni-callback-fences.ts:15:54 - error SC2020: 'tls.createServer with SNICallback and options the runtime walk reads' is part of the standard library types but has no scriptc lowering yet - - 14 | // SNICallback with options the runtime options walk reads. - 15 | tls.createServer({ cert, key, ALPNProtocols: ["h2"], SNICallback: makeSni }); + 12 | // SNICallback with options the runtime options walk reads. + 13 | tls.createServer({ cert, key, ALPNProtocols: ["h2"], SNICallback: makeSni }); | ^~~~~~~~~~~~~~~~~~~~ - 16 | + 14 | hint: SNICallback lowers with cert and key; ALPNProtocols, minVersion, maxVersion and the validated options are not combined with it yet \ No newline at end of file