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.

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;
   }

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")));
+            });
+          }),
+        ),
       ),
     ),
   );

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 });

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