fix(compiler): escape string literals in SSR branches and returns

.changeset/ssr-escape-literal-branches.md

@@ -0,0 +1,6 @@
+---
+"@solidjs/compiler": patch
+"@solidjs/babel-plugin": patch
+---
+
+The server build escapes string literals in a ternary branch, after `&&`, and in every `return` of a function child, including a `return` inside `if`, `switch`, `try` or a loop. The server now renders them as text, as the client does, in children and in attribute values (#4710). A function value in an attribute keeps its literals as written at every depth, because the server stringifies its source.

packages/babel-plugin/src/ssr/element.ts

@@ -317,6 +317,16 @@ function setAttr(
   }
 }
 
+/**
+ * How `escapeExpression` treats a string literal it reaches.
+ * - `undefined`: the hole itself. The caller escapes a literal when it
+ *   inlines it into the template, so it stays as written here.
+ * - `"escape"`: a position whose value the runtime writes as is.
+ * - `"keep"`: inside a function value in an attribute. `ssrAttribute`
+ *   stringifies the function's source, so literals stay as written.
+ */
+type Literals = "escape" | "keep" | undefined;
+
 function escapeExpression(
   path: BabelPath,
   expression:
@@ -326,37 +336,35 @@ function escapeExpression(
     | null
     | undefined,
   attr?: boolean,
-  escapeLiterals?: boolean
+  literals?: Literals
 ): babelTypes.Expression | babelTypes.JSXElement | babelTypes.JSXFragment | null | undefined {
   if (!expression) return expression;
+  // A literal in a branch is the hole's value, which `ssr` and
+  // `ssrAttribute` write as is, so branches escape literals.
+  const branch: Literals = literals === "keep" ? "keep" : "escape";
+  // A function's returns escape like branches, except for a function value in
+  // an attribute: `ssrAttribute` stringifies its source, so its literals stay
+  // as written at every depth.
+  const returns: Literals = literals === undefined && attr ? "keep" : branch;
   if (
     t.isStringLiteral(expression) ||
     t.isNumericLiteral(expression) ||
     (t.isTemplateLiteral(expression) && expression.expressions.length === 0)
   ) {
-    if (escapeLiterals) {
+    if (literals === "escape") {
       if (t.isStringLiteral(expression))
         return t.stringLiteral(escapeHTML(expression.value, attr) as string);
       else if (t.isTemplateLiteral(expression))
-        return t.stringLiteral(escapeHTML(expression.quasis[0].value.raw, attr) as string);
+        return t.stringLiteral(
+          escapeHTML(
+            expression.quasis[0].value.cooked ?? expression.quasis[0].value.raw,
+            attr
+          ) as string
+        );
     }
     return expression;
   } else if (t.isFunction(expression)) {
-    if (t.isBlockStatement(expression.body)) {
-      expression.body.body = expression.body.body.map(e => {
-        if (t.isReturnStatement(e))
-          e.argument = escapeExpression(
-            path,
-            e.argument,
-            attr,
-            escapeLiterals
-          ) as babelTypes.Expression | null;
-        return e;
-      });
-    } else
-      expression.body = escapeExpression(path, expression.body, attr, escapeLiterals) as
-        | babelTypes.Expression
-        | babelTypes.BlockStatement;
+    escapeFunctionResult(path, expression, attr, returns);
     return expression;
   } else if (t.isTemplateLiteral(expression)) {
     // Interpolations are escaped recursively. The static quasis are not —
@@ -366,12 +374,7 @@ function escapeExpression(
     if (attr) escapeTemplateQuasis(expression, true);
     expression.expressions = expression.expressions.map(
       e =>
-        escapeExpression(
-          path,
-          e as babelTypes.Expression,
-          attr,
-          escapeLiterals
-        ) as babelTypes.Expression
+        escapeExpression(path, e as babelTypes.Expression, attr, literals) as babelTypes.Expression
     );
     return expression;
   } else if (t.isUnaryExpression(expression)) {
@@ -381,27 +384,27 @@ function escapeExpression(
       path,
       expression.left as babelTypes.Expression,
       attr,
-      escapeLiterals
+      literals
     ) as babelTypes.Expression | babelTypes.PrivateName;
     expression.right = escapeExpression(
       path,
       expression.right,
       attr,
-      escapeLiterals
+      literals
     ) as babelTypes.Expression;
     return expression;
   } else if (t.isConditionalExpression(expression)) {
     expression.consequent = escapeExpression(
       path,
       expression.consequent,
       attr,
-      escapeLiterals
+      branch
     ) as babelTypes.Expression;
     expression.alternate = escapeExpression(
       path,
       expression.alternate,
       attr,
-      escapeLiterals
+      branch
     ) as babelTypes.Expression;
     return expression;
   } else if (t.isLogicalExpression(expression)) {
@@ -412,29 +415,14 @@ function escapeExpression(
         path,
         expression.right,
         attr,
-        escapeLiterals
+        branch
       ) as babelTypes.Expression;
       return expression;
     }
   } else if (t.isCallExpression(expression) && t.isFunction(expression.callee)) {
-    if (t.isBlockStatement(expression.callee.body)) {
-      expression.callee.body.body = expression.callee.body.body.map(e => {
-        if (t.isReturnStatement(e))
-          e.argument = escapeExpression(
-            path,
-            e.argument,
-            attr,
-            escapeLiterals
-          ) as babelTypes.Expression | null;
-        return e;
-      });
-    } else
-      expression.callee.body = escapeExpression(
-        path,
-        expression.callee.body,
-        attr,
-        escapeLiterals
-      ) as babelTypes.Expression | babelTypes.BlockStatement;
+    // The call's result is the hole's value, so its returns escape like
+    // branches.
+    escapeFunctionResult(path, expression.callee, attr, branch);
     return expression;
   } else if (t.isJSXElement(expression) && !isComponent(getTagName(expression))) {
     (expression as babelTypes.JSXElement & { wontEscape?: boolean }).wontEscape = true;
@@ -454,6 +442,59 @@ function escapeExpression(
   );
 }
 
+/** Escapes a function's expression body or its `return` arguments. */
+function escapeFunctionResult(
+  path: BabelPath,
+  fn: babelTypes.Function,
+  attr: boolean | undefined,
+  literals: Literals
+) {
+  if (t.isBlockStatement(fn.body)) escapeReturns(path, fn.body, attr, literals);
+  else
+    fn.body = escapeExpression(path, fn.body, attr, literals) as
+      | babelTypes.Expression
+      | babelTypes.BlockStatement;
+}
+
+/**
+ * Escapes the `return` arguments in a statement at any depth, without
+ * entering nested functions or classes.
+ */
+function escapeReturns(
+  path: BabelPath,
+  statement: babelTypes.Statement | null | undefined,
+  attr: boolean | undefined,
+  literals: Literals
+) {
+  if (!statement) return;
+  if (t.isReturnStatement(statement)) {
+    statement.argument = escapeExpression(
+      path,
+      statement.argument,
+      attr,
+      literals
+    ) as babelTypes.Expression | null;
+  } else if (t.isBlockStatement(statement)) {
+    for (const child of statement.body) escapeReturns(path, child, attr, literals);
+  } else if (t.isIfStatement(statement)) {
+    escapeReturns(path, statement.consequent, attr, literals);
+    escapeReturns(path, statement.alternate, attr, literals);
+  } else if (t.isSwitchStatement(statement)) {
+    for (const switchCase of statement.cases)
+      for (const child of switchCase.consequent) escapeReturns(path, child, attr, literals);
+  } else if (t.isTryStatement(statement)) {
+    escapeReturns(path, statement.block, attr, literals);
+    escapeReturns(path, statement.handler?.body, attr, literals);
+    escapeReturns(path, statement.finalizer, attr, literals);
+  } else if (
+    t.isLoop(statement) ||
+    t.isLabeledStatement(statement) ||
+    t.isWithStatement(statement)
+  ) {
+    escapeReturns(path, statement.body as babelTypes.Statement, attr, literals);
+  }
+}
+
 function escapeTemplateQuasis(expression: babelTypes.TemplateLiteral, attr: boolean) {
   for (const quasi of expression.quasis) {
     const src = quasi.value.cooked != null ? quasi.value.cooked : quasi.value.raw;
@@ -812,7 +853,7 @@ function transformAttributes(
                   path,
                   p.value as babelTypes.Expression,
                   true,
-                  true
+                  "escape"
                 ) as babelTypes.Expression
               ];
             });

packages/babel-plugin/test/__ssr_fixtures__/conditionalExpressions/code.js

@@ -155,3 +155,47 @@ const template49 = <div>{cond && <>hello {state.text}</>}</div>;
 // statically boolean left: memo value IS the expression value, logical form kept
 const template77 = <>{state.count > 5 && good()}</>;
 const template77a = <>{!state.hidden && good.good}</>;
+
+// string literals in a branch are the hole's value, so they escape at compile
+// time like a literal child does (#4710)
+const template78 = <div>{state.open ? "Close" : "<b>Open</b>"}</div>;
+const template79 = <div>{state.open && "<i>"}</div>;
+const template80 = <div>{state.open ? `<i>` : 0}</div>;
+const template81 = <div>{state.a ? 0 : state.b ? 1 : "<i>"}</div>;
+const template82 = <div>{() => "<i>"}</div>;
+const template83 = <div title={state.open ? '"<i>' : undefined} />;
+// `ssrAttribute` stringifies a function value's source, so the literals in its
+// branches stay as written
+const template84 = <div title={() => (state.open ? '"<' : "b")} />;
+const template85 = <div title={() => state.open && '"<'} />;
+// every `return` of a function child is the hole's value, at any depth
+const template86 = (
+  <div>
+    {(() => {
+      if (state.open) return "<i>";
+      return "<b>";
+    })()}
+  </div>
+);
+const template87 = (
+  <div>
+    {(() => {
+      if (state.open) return state.text;
+      return "<b>";
+    })()}
+  </div>
+);
+const template88 = (
+  <div>
+    {() => {
+      switch (state.kind) {
+        case 1:
+          return "<i>";
+        default: {
+          const inner = () => "<u>";
+          return inner;
+        }
+      }
+    }}
+  </div>
+);

packages/babel-plugin/test/__ssr_fixtures__/conditionalExpressions/output.js

@@ -1,11 +1,13 @@
+import { ssrAttribute as _$ssrAttribute } from "r-server";
 import { memo as _$memo } from "r-server";
 import { ssr as _$ssr } from "r-server";
 import { escape as _$escape } from "r-server";
 var _tmpl$ = ["<div>", "</div>"],
   _tmpl$2 = "<div>Output</div>",
-  _tmpl$3 = "<span>s</span>",
-  _tmpl$4 = "<span>a</span>",
-  _tmpl$5 = "<span>b</span>";
+  _tmpl$3 = ["<div", "></div>"],
+  _tmpl$4 = "<span>s</span>",
+  _tmpl$5 = "<span>a</span>",
+  _tmpl$6 = "<span>b</span>";
 var _v$ = _$escape(simple);
 const template1 = _$ssr(_tmpl$, _v$);
 var _v$2 = () => _$escape(state.dynamic);
@@ -274,9 +276,9 @@ var _v$29 = cond && _$escape(_$memo(() => state.text));
 const template44 = _$ssr(_tmpl$, _v$29);
 var _v$30 = cond ? _$escape(_$memo(() => state.a)) : _$escape(_$memo(() => state.b));
 const template45 = _$ssr(_tmpl$, _v$30);
-var _v$31 = cond && _$ssr(_tmpl$3);
+var _v$31 = cond && _$ssr(_tmpl$4);
 const template46 = _$ssr(_tmpl$, _v$31);
-var _v$32 = cond ? _$ssr(_tmpl$4) : _$ssr(_tmpl$5);
+var _v$32 = cond ? _$ssr(_tmpl$5) : _$ssr(_tmpl$6);
 const template47 = _$ssr(_tmpl$, _v$32);
 
 // component inside fragment must keep the outer _$escape wrap because a
@@ -292,3 +294,55 @@ const template49 = _$ssr(_tmpl$, _v$34);
 // statically boolean left: memo value IS the expression value, logical form kept
 const template77 = _$memo(() => _$memo(() => state.count > 5)() && good());
 const template77a = _$memo(() => _$memo(() => !state.hidden)() && good.good);
+
+// string literals in a branch are the hole's value, so they escape at compile
+// time like a literal child does (#4710)
+var _v$35 = () => (state.open ? "Close" : "&lt;b>Open&lt;/b>");
+const template78 = _$ssr(_tmpl$, _v$35);
+var _v$36 = () => state.open && "&lt;i>";
+const template79 = _$ssr(_tmpl$, _v$36);
+var _v$37 = () => (state.open ? "&lt;i>" : 0);
+const template80 = _$ssr(_tmpl$, _v$37);
+var _v$38 = (() => {
+  var _c$16 = _$memo(() => !!state.a);
+  return () => (_c$16() ? 0 : state.b ? 1 : "&lt;i>");
+})();
+const template81 = _$ssr(_tmpl$, _v$38);
+var _v$39 = () => "&lt;i>";
+const template82 = _$ssr(_tmpl$, _v$39);
+var _v$40 = () => _$ssrAttribute("title", state.open ? "&quot;&lt;i>" : _$escape(undefined, true));
+const template83 = _$ssr(_tmpl$3, _v$40);
+// `ssrAttribute` stringifies a function value's source, so the literals in its
+// branches stay as written
+const template84 = _$ssr(
+  _tmpl$3,
+  _$ssrAttribute("title", () => (state.open ? '"<' : "b"))
+);
+const template85 = _$ssr(
+  _tmpl$3,
+  _$ssrAttribute("title", () => state.open && '"<')
+);
+// every `return` of a function child is the hole's value, at any depth
+var _v$41 = () =>
+  (() => {
+    if (state.open) return "&lt;i>";
+    return "&lt;b>";
+  })();
+const template86 = _$ssr(_tmpl$, _v$41);
+var _v$42 = () =>
+  (() => {
+    if (state.open) return _$escape(state.text);
+    return "&lt;b>";
+  })();
+const template87 = _$ssr(_tmpl$, _v$42);
+var _v$43 = () => {
+  switch (state.kind) {
+    case 1:
+      return "&lt;i>";
+    default: {
+      const inner = () => "<u>";
+      return _$escape(inner);
+    }
+  }
+};
+const template88 = _$ssr(_tmpl$, _v$43);