diff --git a/src/Util/HTML.php b/src/Util/HTML.php index f0711db9c5..c56bd1cd6b 100644 --- a/src/Util/HTML.php +++ b/src/Util/HTML.php @@ -34,17 +34,25 @@ use InvalidArgumentException; abstract class HTML { - public const ALLOWED_TAGS = ['p', 'br', 'a', 'span']; + /** + * Tags whose content is sensitive to indentation, so we shouldn't indent them + */ + public const NO_INDENT_TAGS = ['a', 'b', 'em', 'i', 'q', 's', 'p', 'sub', 'sup', 'u']; + + public const ALLOWED_TAGS = ['p', 'br', 'a', 'span', 'div']; + public const FORBIDDEN_ATTRIBUTES = [ 'onerror', 'form', 'onforminput', 'onbeforescriptexecute', 'formaction', 'onfocus', 'onload', 'data', 'event', 'autofocus', 'onactivate', 'onanimationstart', 'onwebkittransitionend', 'onblur', 'poster', 'onratechange', 'ontoggle', 'onscroll', 'actiontype', 'dirname', 'srcdoc', ]; + public const SELF_CLOSING_TAG = ['area', 'base', 'br', 'col', 'embed', 'hr', 'img', 'link', 'meta', 'param', 'source', 'track', 'wbr']; + /** * Creates an HTML tag without attributes */ - public static function tag(string $tag, mixed $attrs = null, mixed $content = null, array $options = []): array + public static function tag(string $tag, mixed $attrs = null, mixed $content = null, array $options = []): string { return self::attr_tag($tag, $attrs ?? '', $content ?? '', $options); } @@ -52,20 +60,20 @@ abstract class HTML /** * Create tag, possibly with attributes and indentation */ - private static function attr_tag(string $tag, mixed $attrs, mixed $content = '', array $options = []): array + private static function attr_tag(string $tag, mixed $attrs, mixed $content = '', array $options = []): string { $html = '<' . $tag . (\is_string($attrs) ? ($attrs ? ' ' : '') . $attrs : self::attr($attrs, $options)); - if ($options['empty'] ?? false) { - $html .= '/>'; + if (\in_array($tag, self::SELF_CLOSING_TAG)) { + $html .= '>'; } else { - if ($options['indent'] ?? true) { + if (!\in_array($tag, self::NO_INDENT_TAGS) && ($options['indent'] ?? true)) { $inner = Formatting::indent($content); $html .= ">\n" . ($inner == '' ? '' : $inner . "\n") . ""; } else { $html .= ">{$content}"; } } - return explode("\n", $html); + return $html; } /** @@ -73,32 +81,29 @@ abstract class HTML */ private static function attr(array $attrs, array $options = []): string { - return ' ' . implode( - ' ', - F\map( - $attrs, - /** - * Convert an attr ($key), $val pair to an HTML attribute, but validate to exclude some vectors of injection - */ - function (string $val, string $key, array $_): string { - if (\in_array($key, array_merge($options['forbidden_attributes'] ?? [], self::FORBIDDEN_ATTRIBUTES)) - || str_starts_with($val, 'javascript:')) { - throw new InvalidArgumentException("HTML::html: Attribute {$key} is not allowed"); - } - if (!($options['raw'] ?? false)) { - $val = htmlspecialchars($val, flags: \ENT_QUOTES | \ENT_SUBSTITUTE, double_encode: false); - } - return "{$key}=\"{$val}\""; - }, - ), - ); + return ' ' . implode(' ', F\map($attrs, [self::class, '_process_attribute'])); + } + + /** + * Convert an attr ($key), $val pair to an HTML attribute, but validate to exclude some vectors of injection + */ + public static function _process_attribute(string $val, string $key): string + { + if (\in_array($key, array_merge($options['forbidden_attributes'] ?? [], self::FORBIDDEN_ATTRIBUTES)) + || str_starts_with($val, 'javascript:')) { + throw new InvalidArgumentException("HTML::html: Attribute {$key} is not allowed"); + } + if (!($options['raw'] ?? false)) { + $val = htmlspecialchars($val, flags: \ENT_QUOTES | \ENT_SUBSTITUTE, double_encode: false); + } + return "{$key}=\"{$val}\""; } /** * @param array|string $html The input to convert to HTML * @param array $options = [] ['allowed_tags' => string[], 'forbidden_attributes' => string[], 'raw' => bool, 'indent' => bool] */ - public static function html(string|array $html, array $options = [], int $indent = 1): string + public static function html(string|array $html, array $options = []): string { if (\is_string($html)) { if ($options['raw'] ?? false) { @@ -109,18 +114,18 @@ abstract class HTML } else { $out = ''; foreach ($html as $tag => $contents) { - if ($contents['empty'] ?? false) { - $out .= "<{$tag}/>"; + if (\in_array($tag, self::SELF_CLOSING_TAG)) { + $out .= "<{$tag}>"; } else { $attrs = isset($contents['attrs']) ? self::attr(array_shift($contents), $options) : ''; - $is_tag = is_string($tag) && preg_match('/[A-Za-z][A-Za-z0-9]*/', $tag); - $inner = self::html($contents, $options, $indent + 1); + $is_tag = \is_string($tag) && preg_match('/[A-Za-z][A-Za-z0-9]*/', $tag); + $inner = self::html($contents, $options); if ($is_tag) { if (!\in_array($tag, array_merge($options['allowed_tags'] ?? [], self::ALLOWED_TAGS))) { throw new InvalidArgumentException("HTML::html: Tag {$tag} is not allowed"); } - if (!empty($inner)) { - $inner = ($options['indent'] ?? true) ? ("\n" . Formatting::indent($inner, $indent) . "\n") : $inner; + if (!empty($inner) && !\in_array($tag, self::NO_INDENT_TAGS) && ($options['indent'] ?? true)) { + $inner = "\n" . Formatting::indent($inner) . "\n"; } $out .= "<{$tag}{$attrs}>{$inner}"; } else { diff --git a/tests/Util/HTMLTest.php b/tests/Util/HTMLTest.php index c9863955c7..ec4c084a2a 100644 --- a/tests/Util/HTMLTest.php +++ b/tests/Util/HTMLTest.php @@ -22,9 +22,9 @@ declare(strict_types = 1); namespace App\Tests\Util; use App\Util\HTML; -use InvalidArgumentException; use Jchook\AssertThrows\AssertThrows; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use TypeError; class HTMLTest extends WebTestCase { @@ -33,12 +33,16 @@ class HTMLTest extends WebTestCase public function testHTML() { static::assertSame('', HTML::html('')); - static::assertSame("\n\n\n", HTML::html(['a' => ''])); - static::assertSame("\n

\n

\n
\n", HTML::html(['a' => ['p' => '']])); - static::assertSame("\n

\n

\n
\n", HTML::html(['a' => ['attrs' => ['href' => 'test'], 'p' => '']])); - static::assertSame("\n

\n foo\n

\n
\n
\n", HTML::html(['a' => ['p' => 'foo', 'br' => 'empty']])); - static::assertThrows(InvalidArgumentException::class, fn () => HTML::html(1)); - static::assertSame("\n foo\n", implode("\n", HTML::tag('a', ['href' => 'test'], content: 'foo', options: ['empty' => false]))); - static::assertSame('
', implode("\n", HTML::tag('br', attrs: null, content: null, options: ['empty' => true]))); + static::assertSame('', HTML::html(['a' => ''])); + static::assertSame("
\n

\n
", HTML::html(['div' => ['p' => '']])); + static::assertSame("
\n
\n

\n
\n
", HTML::html(['div' => ['div' => ['p' => '']]])); + static::assertSame("
\n
\n
\n

\n
\n
\n
", HTML::html(['div' => ['div' => ['div' => ['p' => '']]]])); + static::assertSame('

', HTML::html(['a' => ['attrs' => ['href' => 'test'], 'p' => '']])); + static::assertSame('

foo


', HTML::html(['a' => ['p' => 'foo', 'br' => 'empty']])); + static::assertSame("
\n

foo


\n
", HTML::html(['div' => ['a' => ['p' => 'foo', 'br' => 'empty']]])); + static::assertSame('

foo


', HTML::html(['div' => ['a' => ['p' => 'foo', 'br' => 'empty']]], options: ['indent' => false])); + static::assertThrows(TypeError::class, fn () => HTML::html(1)); + static::assertSame('foo', HTML::tag('a', ['href' => 'test'], content: 'foo', options: ['empty' => false])); + static::assertSame('
', HTML::tag('br', attrs: null, content: null, options: ['empty' => true])); } }