bug symfony#44912 [Console] Allow OutputFormatter::escape() to be used for escaping URLs used in <href> (Seldaek)

This PR was merged into the 4.4 branch.

Discussion
----------

[Console] Allow OutputFormatter::escape() to be used for escaping URLs used in <href>

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

I was trying to use escape() to make user-provided URLs safe in `<href=...>` but I realized it was really only good for avoid starting tags, and not for escaping the content of a tag.

- escape() now escapes `>` as well as `<`
- URLs containing escaped `<`, `>` are now rendered correctly
- user-provided URLs should now be safe to use (as in they cannot break the formatting) as long as they're piped through `escape()`
- possibly also resolves issues if you were trying to use user-provided colors i.e. `'<'.OutputFormatter::escape($color).'>'` where as in current released code it would not help you at all here. I haven't checked that yet

I am happy to spend time adding tests but would like to first get feedback on the changes to know if it's reasonable or not to change `escape()` in this way.

The rest of the changes I think are absolutely safe to merge and make sense regardless.

Commits
-------

cfa8910 Allow OutputFormatter::escape() to be used for escaping URLs used in <href>

Author: fabpot

src/Symfony/Component/Console/Formatter/OutputFormatter.php

@@ -34,15 +34,15 @@ public function __clone()
     }
 
     /**
-     * Escapes "<" special char in given text.
+     * Escapes "<" and ">" special chars in given text.
      *
      * @param string $text Text to escape
      *
      * @return string Escaped text
      */
     public static function escape($text)
     {
-        $text = preg_replace('/([^\\\\]?)</', '$1\\<', $text);
+        $text = preg_replace('/([^\\\\]|^)([<>])/', '$1\\\\$2', $text);
 
         return self::escapeTrailingBackslash($text);
     }
@@ -144,9 +144,10 @@ public function formatAndWrap(string $message, int $width)
     {
         $offset = 0;
         $output = '';
-        $tagRegex = '[a-z][^<>]*+';
+        $openTagRegex = '[a-z](?:[^\\\\<>]*+ | \\\\.)*';
+        $closeTagRegex = '[a-z][^<>]*+';
         $currentLineLength = 0;
-        preg_match_all("#<(($tagRegex) | /($tagRegex)?)>#ix", $message, $matches, \PREG_OFFSET_CAPTURE);
+        preg_match_all("#<(($openTagRegex) | /($closeTagRegex)?)>#ix", $message, $matches, \PREG_OFFSET_CAPTURE);
         foreach ($matches[0] as $i => $match) {
             $pos = $match[1];
             $text = $match[0];
@@ -180,11 +181,7 @@ public function formatAndWrap(string $message, int $width)
 
         $output .= $this->applyCurrentStyle(substr($message, $offset), $output, $width, $currentLineLength);
 
-        if (str_contains($output, "\0")) {
-            return strtr($output, ["\0" => '\\', '\\<' => '<']);
-        }
-
-        return str_replace('\\<', '<', $output);
+        return strtr($output, ["\0" => '\\', '\\<' => '<', '\\>' => '>']);
     }
 
     /**
@@ -218,7 +215,8 @@ private function createStyleFromString(string $string): ?OutputFormatterStyleInt
             } elseif ('bg' == $match[0]) {
                 $style->setBackground(strtolower($match[1]));
             } elseif ('href' === $match[0]) {
-                $style->setHref($match[1]);
+                $url = preg_replace('{\\\\([<>])}', '$1', $match[1]);
+                $style->setHref($url);
             } elseif ('options' === $match[0]) {
                 preg_match_all('([^,;]+)', strtolower($match[1]), $options);
                 $options = array_shift($options);

src/Symfony/Component/Console/Tests/Fixtures/command_2.txt

@@ -2,9 +2,9 @@
   command 2 description
 
 <comment>Usage:</comment>
-  descriptor:command2 [options] [--] \<argument_name>
-  descriptor:command2 -o|--option_name \<argument_name>
-  descriptor:command2 \<argument_name>
+  descriptor:command2 [options] [--] \<argument_name\>
+  descriptor:command2 -o|--option_name \<argument_name\>
+  descriptor:command2 \<argument_name\>
 
 <comment>Arguments:</comment>
   <info>argument_name</info>      

src/Symfony/Component/Console/Tests/Fixtures/command_mbstring.txt

@@ -2,9 +2,9 @@
   command åèä description
 
 <comment>Usage:</comment>
-  descriptor:åèä [options] [--] \<argument_åèä>
-  descriptor:åèä -o|--option_name \<argument_name>
-  descriptor:åèä \<argument_name>
+  descriptor:åèä [options] [--] \<argument_åèä\>
+  descriptor:åèä -o|--option_name \<argument_name\>
+  descriptor:åèä \<argument_name\>
 
 <comment>Arguments:</comment>
   <info>argument_åèä</info>   

src/Symfony/Component/Console/Tests/Fixtures/input_argument_with_style.txt

@@ -1 +1 @@
-  <info>argument_name</info>  argument description<comment> [default: "\<comment>style\</>"]</comment>
+  <info>argument_name</info>  argument description<comment> [default: "\<comment\>style\</\>"]</comment>

src/Symfony/Component/Console/Tests/Fixtures/input_option_with_style.txt

@@ -1 +1 @@
-  <info>-o, --option_name=OPTION_NAME</info>  option description<comment> [default: "\<comment>style\</>"]</comment>
+  <info>-o, --option_name=OPTION_NAME</info>  option description<comment> [default: "\<comment\>style\</\>"]</comment>

src/Symfony/Component/Console/Tests/Fixtures/input_option_with_style_array.txt

@@ -1 +1 @@
-  <info>-o, --option_name=OPTION_NAME</info>  option description<comment> [default: ["\<comment>Hello\</comment>","\<info>world\</info>"]]</comment><comment> (multiple values allowed)</comment>
+  <info>-o, --option_name=OPTION_NAME</info>  option description<comment> [default: ["\<comment\>Hello\</comment\>","\<info\>world\</info\>"]]</comment><comment> (multiple values allowed)</comment>

src/Symfony/Component/Console/Tests/Formatter/OutputFormatterTest.php

@@ -32,7 +32,10 @@ public function testLGCharEscaping()
         $this->assertEquals('foo << bar \\', $formatter->format('foo << bar \\'));
         $this->assertEquals("foo << \033[32mbar \\ baz\033[39m \\", $formatter->format('foo << <info>bar \\ baz</info> \\'));
         $this->assertEquals('<info>some info</info>', $formatter->format('\\<info>some info\\</info>'));
-        $this->assertEquals('\\<info>some info\\</info>', OutputFormatter::escape('<info>some info</info>'));
+        $this->assertEquals('\\<info\\>some info\\</info\\>', OutputFormatter::escape('<info>some info</info>'));
+        // every < and > gets escaped if not already escaped, but already escaped ones do not get escaped again
+        // and escaped backslashes remain as such, same with backslashes escaping non-special characters
+        $this->assertEquals('foo \\< bar \\< baz \\\\< foo \\> bar \\> baz \\\\> \\x', OutputFormatter::escape('foo < bar \\< baz \\\\< foo > bar \\> baz \\\\> \\x'));
 
         $this->assertEquals(
             "\033[33mSymfony\\Component\\Console does work very well!\033[39m",
@@ -259,6 +262,7 @@ public function provideDecoratedAndNonDecoratedOutput()
             ['<question>some question</question>', 'some question', "\033[30;46msome question\033[39;49m"],
             ['<fg=red>some text with inline style</>', 'some text with inline style', "\033[31msome text with inline style\033[39m"],
             ['<href=idea://open/?file=/path/SomeFile.php&line=12>some URL</>', 'some URL', "\033]8;;idea://open/?file=/path/SomeFile.php&line=12\033\\some URL\033]8;;\033\\"],
+            ['<href=https://example.com/\<woohoo\>>some URL with \<woohoo\></>', 'some URL with <woohoo>', "\033]8;;https://example.com/<woohoo>\033\\some URL with <woohoo>\033]8;;\033\\"],
             ['<href=idea://open/?file=/path/SomeFile.php&line=12>some URL</>', 'some URL', 'some URL', 'JetBrains-JediTerm'],
         ];
     }

src/Symfony/Component/Console/Tests/Helper/FormatterHelperTest.php

@@ -83,9 +83,9 @@ public function testFormatBlockLGEscaping()
         $formatter = new FormatterHelper();
 
         $this->assertEquals(
-            '<error>                            </error>'."\n".
-            '<error>  \<info>some info\</info>  </error>'."\n".
-            '<error>                            </error>',
+            '<error>                              </error>'."\n".
+            '<error>  \<info\>some info\</info\>  </error>'."\n".
+            '<error>                              </error>',
             $formatter->formatBlock('<info>some info</info>', 'error', true),
             '::formatBlock() escapes \'<\' chars'
         );