fix(cli/console): always quote and escape inspected strings (denoland…

…#7546)
vitormmatos · Sep 18, 2020 · 38196f7 · 38196f7
1 parent 7845740
commit 38196f7
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 23 deletions.
diff --git a/cli/rt/02_console.js b/cli/rt/02_console.js
@@ -405,7 +405,7 @@
 
     switch (typeof value) {
       case "string":
-        return value;
+        return green(quoteString(value));
       case "number": // Numbers are yellow
         // Special handling of -0
         return yellow(Object.is(value, -0) ? "-0" : `${value}`);
@@ -1265,8 +1265,12 @@
       if (a > 0) {
         string += " ";
       }
-      // Use default maximum depth for null or undefined arguments.
-      string += inspectValue(args[a], new Set(), 0, rInspectOptions);
+      if (typeof args[a] == "string") {
+        string += args[a];
+      } else {
+        // Use default maximum depth for null or undefined arguments.
+        string += inspectValue(args[a], new Set(), 0, rInspectOptions);
+      }
     }
 
     if (rInspectOptions.indentLevel > 0) {
@@ -1543,16 +1547,12 @@
     value,
     inspectOptions = {},
   ) {
-    if (typeof value === "string") {
-      return value;
-    } else {
-      return inspectValue(value, new Set(), 0, {
-        ...DEFAULT_INSPECT_OPTIONS,
-        ...inspectOptions,
-        // TODO(nayeemrmn): Indent level is not supported.
-        indentLevel: 0,
-      });
-    }
+    return inspectValue(value, new Set(), 0, {
+      ...DEFAULT_INSPECT_OPTIONS,
+      ...inspectOptions,
+      // TODO(nayeemrmn): Indent level is not supported.
+      indentLevel: 0,
+    });
   }
 
   // Expose these fields to internalObject for tests.

diff --git a/cli/tests/unit/console_test.ts b/cli/tests/unit/console_test.ts
@@ -831,7 +831,7 @@ unitTest(function consoleTestWithStringFormatSpecifier(): void {
 unitTest(function consoleTestWithObjectFormatSpecifier(): void {
   assertEquals(stringify("%o"), "%o");
   assertEquals(stringify("%o", 42), "42");
-  assertEquals(stringify("%o", "foo"), "foo");
+  assertEquals(stringify("%o", "foo"), `"foo"`);
   assertEquals(stringify("o: %o, a: %O", {}, []), "o: {}, a: []");
   assertEquals(stringify("%o", { a: 42 }), "{ a: 42 }");
   assertEquals(
@@ -1424,6 +1424,17 @@ unitTest(function consoleTrace(): void {
   });
 });
 
+unitTest(function inspectString(): void {
+  assertEquals(
+    stripColor(Deno.inspect("\0")),
+    `"\\x00"`,
+  );
+  assertEquals(
+    stripColor(Deno.inspect("\x1b[2J")),
+    `"\\x1b[2J"`,
+  );
+});
+
 unitTest(function inspectSorted(): void {
   assertEquals(
     stripColor(Deno.inspect({ b: 2, a: 1 }, { sorted: true })),

diff --git a/std/fmt/printf_test.ts b/std/fmt/printf_test.ts
@@ -655,8 +655,14 @@ Deno.test("testErrors", function (): void {
 
   assertEquals(S("%[1]*.2f", "a", "p"), "%!(BAD WIDTH 'a')");
 
-  assertEquals(S("A", "a", "p"), "A%!(EXTRA 'a' 'p')");
-  assertEquals(S("%[2]s %[2]s", "a", "p"), "p p%!(EXTRA 'a')");
+  assertEquals(
+    S("A", "a", "p"),
+    `A%!(EXTRA '\x1b[32m"a"\x1b[39m' '\x1b[32m"p"\x1b[39m')`,
+  );
+  assertEquals(
+    S("%[2]s %[2]s", "a", "p"),
+    `p p%!(EXTRA '\x1b[32m"a"\x1b[39m')`,
+  );
 
   // remains to be determined how to handle bad indices ...
   // (realistically) the entire error handling is still up for grabs.

diff --git a/std/node/assertion_error_test.ts b/std/node/assertion_error_test.ts
@@ -108,7 +108,7 @@ Deno.test({
       stackStartFn,
     });
     assertStrictEquals(err.name, "AssertionError");
-    assertStrictEquals(stripColor(err.message), "deno match /node/");
+    assertStrictEquals(stripColor(err.message), `"deno" match /node/`);
     assertStrictEquals(err.generatedMessage, true);
     assertStrictEquals(err.code, "ERR_ASSERTION");
     assertStrictEquals(err.actual, "deno");

diff --git a/std/testing/asserts.ts b/std/testing/asserts.ts
@@ -20,19 +20,15 @@ export class AssertionError extends Error {
 }
 
 export function _format(v: unknown): string {
-  let string = globalThis.Deno
+  return globalThis.Deno
     ? stripColor(Deno.inspect(v, {
       depth: Infinity,
       sorted: true,
       trailingComma: true,
       compact: false,
       iterableLimit: Infinity,
     }))
-    : String(v);
-  if (typeof v == "string") {
-    string = `"${string.replace(/(?=["\\])/g, "\\")}"`;
-  }
-  return string;
+    : `"${String(v).replace(/(?=["\\])/g, "\\")}"`;
 }
 
 function createColor(diffType: DiffType): (s: string) => string {