FIX: Prevent producing "undefined" strings (#10042)

FIX: Prevent producing “undefined” strings (#10042)

Fixes a bug in search-menu-results (type: “group”), where:

const fullName = escapeExpression(group.fullName);
const name = escapeExpression(group.name);
const groupNames = [h("span.name", fullName || name)];

groupNames could end up having value “undefined” if a group doesn’t have a fullName.

diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js
index 92fa344..cee9ce5 100644
--- a/app/assets/javascripts/discourse/app/lib/utilities.js
+++ b/app/assets/javascripts/discourse/app/lib/utilities.js
@@ -25,6 +25,10 @@ export function translateSize(size) {
 }
 
 export function escapeExpression(string) {
+  if (!string) {
+    return "";
+  }
+
   // don't escape SafeStrings, since they're already safe
   if (string instanceof Handlebars.SafeString) {
     return string.toString();
diff --git a/test/javascripts/lib/utilities-test.js b/test/javascripts/lib/utilities-test.js
index f3d14a2..1ac73ec 100644
--- a/test/javascripts/lib/utilities-test.js
+++ b/test/javascripts/lib/utilities-test.js
@@ -1,5 +1,6 @@
 /* global Int8Array:true */
 import {
+  escapeExpression,
   emailValid,
   extractDomainFromUrl,
   avatarUrl,
@@ -14,9 +15,30 @@ import {
   fillMissingDates,
   inCodeBlock
 } from "discourse/lib/utilities";
+import Handlebars from "handlebars";
 
 QUnit.module("lib:utilities");
 
+QUnit.test("escapeExpression", assert => {
+  assert.equal(
+    escapeExpression(">"),
+    ">",
+    "escapes unsafe characters"
+  );
+
+  assert.equal(
+    escapeExpression(new Handlebars.SafeString(">")),
+    ">",
+    "does not double-escape safe strings"
+  );
+
+  assert.equal(
+    escapeExpression(undefined),
+    "",
+    "returns a falsy string when given a falsy value"
+  );
+});
+
 QUnit.test("emailValid", assert => {
   assert.ok(
     emailValid("Bob@example.com"),

GitHub sha: a859d507

1 Like

This commit appears in #10042 which was approved by eviltrout. It was merged by CvX.