[Console] OutputFormatter::escape doesn't :(

[bobthecow]

With OutputFormatter's current naive escaping, there's no way to represent a formatted string which ends in a literal \ character:

bobthecow/psysh#282

I spent quite a while last night heading down a rabbit hole to fix this. Escaping \ itself with a second \ seemed promising at first, but escaping an arbitrary number of slashes (and getting them back unescaped again) started getting absurd. Then I played a bit with CDATA style, since this is XML-ish, but that's yet another rabbit hole that ends in writing (or using) a full XML parser just to colorize sections of output.

Then I realized that we don't need to make it perfect, we just need to make it far less likely that anyone will run into it. The current issue has existed for ages, and it's not unthinkable for a user string to end in \, but as far as I can tell this is the first time it has come up. So instead of trying to solve for 100%, I figured I could solve for 99.9999999% 😄

Thoughts?

diff --git a/src/Symfony/Component/Console/Formatter/OutputFormatter.php b/src/Symfony/Component/Console/Formatter/OutputFormatter.php
index 244e25c..2d8f15b 100644
--- a/src/Symfony/Component/Console/Formatter/OutputFormatter.php
+++ b/src/Symfony/Component/Console/Formatter/OutputFormatter.php
@@ -33,7 +33,10 @@ class OutputFormatter implements OutputFormatterInterface
      */
     public static function escape($text)
     {
-        return preg_replace('/([^\\\\]?)</', '$1\\<', $text);
+        // Use three non-consecutive reserved unicode code points as a placeholder
+        // for `<` characters. The key here isn't to make it impossible to have a
+        // collision, but to make it really, really, really unlikely.
+        return str_replace('<', "\xDF\xFF\xDF\xFD\xDF\xFE", $text);
     }

     /**
@@ -137,10 +140,6 @@ class OutputFormatter implements OutputFormatterInterface
             $pos = $match[1];
             $text = $match[0];

-            if (0 != $pos && '\\' == $message[$pos - 1]) {
-                continue;
-            }
-
             // add the text up to the next tag
             $output .= $this->applyCurrentStyle(substr($message, $offset, $pos - $offset));
             $offset = $pos + strlen($text);
@@ -166,7 +165,12 @@ class OutputFormatter implements OutputFormatterInterface

         $output .= $this->applyCurrentStyle(substr($message, $offset));

-        return str_replace('\\<', '<', $output);
+        /**
+         * Change our placeholder back to `<`.
+         *
+         * @see OutputFormatter::escape()
+         */
+        return str_replace("\xDF\xFF\xDF\xFD\xDF\xFE", '<', $output);
     }

     /**

[fabpot]

ping @nicolas-grekas

[stof]

change the way escaping is done is a BC break, as strings may be escaped manually in some places when writing a message instead of using the escape method.

[nicolas-grekas]

@bobthecow can you please see if #17914 looks good to you?

[bobthecow]

lgtm. thanks!