diff --git a/README.md b/README.md index 9920f47..5562427 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,16 @@ Available Filters: - `pluralize: 'item', 'items'` - `truncate: 50` - `default: 'fallback value'` +- `raw` (see below) + +**Escaping.** `TemplateMailable` (the transactional send) HTML-escapes every value that comes from the data you pass in, where it lands +in the message's HTML: a name or a note a customer typed cannot add markup or a link to your email. Text you wrote in the template, +including a `default` fallback, is left as written. If a value really is markup you trust (an order table you built), say so in the +template: `{{ order_table | raw }}`. The plain-text part and the subject are text and are not escaped; the subject is always one line, +and `unsubscribe_url` / `listUnsubscribeUrl` must be an http(s) or mailto address to become a `List-Unsubscribe` header. + +`MailBuilder::interpolate($content, $context)` does not escape (callers that escape their own values rely on that); pass +`escape: true` to escape. --- diff --git a/src/Mail/TemplateMailable.php b/src/Mail/TemplateMailable.php index 3fe9904..974d731 100644 --- a/src/Mail/TemplateMailable.php +++ b/src/Mail/TemplateMailable.php @@ -54,10 +54,13 @@ public function __construct( $plain = MailBuilder::plainText($template); } - // Interpolate merge tags - $this->compiledHtml = MailBuilder::interpolate($html, $this->data); + // Interpolate merge tags. Values are HTML-escaped where they land in the HTML ({{ tag | raw }} puts markup the template author + // trusts); plain text and the subject are text, and a subject is one line whatever a value held. + $this->compiledHtml = MailBuilder::interpolate($html, $this->data, escape: true); $this->compiledPlainText = MailBuilder::interpolate($plain, $this->data); - $this->subjectLine = $subject !== null ? MailBuilder::interpolate($subject, $this->data) : 'Important Update'; + $this->subjectLine = $subject !== null + ? trim((string) preg_replace('/[\x00-\x1F\x7F]+/u', ' ', MailBuilder::interpolate($subject, $this->data))) + : 'Important Update'; if ($this->embedCidImages) { $embedResult = CidImageEmbedder::embed($this->compiledHtml); @@ -92,7 +95,9 @@ public function headers(): Headers { $unsub = $this->listUnsubscribeUrl ?? ($this->data['unsubscribe_url'] ?? null); - if (! empty($unsub) && is_string($unsub)) { + // The List-Unsubscribe header carries an address a mailbox provider may call: only an http(s) or mailto address, with nothing + // in it that could end the header value or add another. + if (! empty($unsub) && is_string($unsub) && preg_match('~^(?:https?://|mailto:)[^\s<>"\'\x00-\x1F\x7F]+$~i', $unsub) === 1) { return new Headers( text: [ 'List-Unsubscribe-Post' => 'List-Unsubscribe=One-Click', diff --git a/src/MailBuilder.php b/src/MailBuilder.php index e4f60bb..a834a56 100644 --- a/src/MailBuilder.php +++ b/src/MailBuilder.php @@ -127,9 +127,9 @@ public static function mergeTags(): MergeTagRegistry * * @param array|null $context */ - public static function interpolate(string $content, ?array $context = null): string + public static function interpolate(string $content, ?array $context = null, bool $escape = false): string { - return self::mergeTags()->interpolate($content, $context); + return self::mergeTags()->interpolate($content, $context, $escape); } /** diff --git a/src/MergeTags/MergeTagInterpolator.php b/src/MergeTags/MergeTagInterpolator.php index 3cbe573..801408a 100644 --- a/src/MergeTags/MergeTagInterpolator.php +++ b/src/MergeTags/MergeTagInterpolator.php @@ -13,9 +13,15 @@ class MergeTagInterpolator /** * Interpolate merge tags and conditional blocks within a given string using context. * + * With $escape, a value that comes from the context is HTML-escaped where it is put into the text, so a value that is + * not markup (a name, a note, an order reference a customer typed) cannot add markup or a link to an email. A tag + * written with the `raw` filter ({{ order_table | raw }}) is the template author saying the value is markup they + * trust; fallback text from a `default` filter is the author's own and is not escaped. Without $escape (plain text, + * subjects, and callers that escape their own values) nothing is escaped, as before. + * * @param array $context */ - public function interpolate(string $content, array $context = []): string + public function interpolate(string $content, array $context = [], bool $escape = false): string { // 1. Process conditional logic blocks: {% if condition %}...{% else %}...{% endif %} $content = $this->interpolateConditionals($content, $context); @@ -23,13 +29,17 @@ public function interpolate(string $content, array $context = []): string // 2. Process variable tags with pipe filters: {{ tag | filter1 | filter2:arg }} return (string) preg_replace_callback( '/\{\{\s*([a-zA-Z0-9_\.]+)(?:\s*\|\s*([^}]+))?\s*\}\}/', - function (array $matches) use ($context): string { + function (array $matches) use ($context, $escape): string { $tag = trim($matches[1]); $pipeFilters = isset($matches[2]) ? trim($matches[2]) : null; $raw = Arr::get($context, $tag); $value = ($raw !== null && is_scalar($raw)) ? (string) $raw : null; + // Whether the text being built is the context's (to be escaped) or the template author's own. + $fromContext = $value !== null; + $trusted = false; + if ($pipeFilters !== null && $pipeFilters !== '') { $filters = $this->parseFilters($pipeFilters); @@ -37,10 +47,13 @@ function (array $matches) use ($context): string { $name = $filter['name']; $args = $filter['args']; - if ($name === 'default') { + if ($name === 'raw') { + $trusted = true; + } elseif ($name === 'default') { $defaultFallback = $args[0] ?? ''; if ($value === null || $value === '') { $value = $defaultFallback; + $fromContext = false; } } elseif ($value !== null) { $value = $this->applyFilter($value, $name, $args); @@ -49,7 +62,9 @@ function (array $matches) use ($context): string { } if ($value !== null) { - return $value; + return $escape && $fromContext && ! $trusted + ? htmlspecialchars($value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8') + : $value; } // If no value provided, leave tag intact diff --git a/src/MergeTags/MergeTagRegistry.php b/src/MergeTags/MergeTagRegistry.php index ca96ff4..744b7bb 100644 --- a/src/MergeTags/MergeTagRegistry.php +++ b/src/MergeTags/MergeTagRegistry.php @@ -93,10 +93,10 @@ public function sampleContext(): array * * @param array|null $context */ - public function interpolate(string $content, ?array $context = null): string + public function interpolate(string $content, ?array $context = null, bool $escape = false): string { $context ??= $this->sampleContext(); - return $this->interpolator->interpolate($content, $context); + return $this->interpolator->interpolate($content, $context, $escape); } } diff --git a/tests/Unit/MergeTagEscapingTest.php b/tests/Unit/MergeTagEscapingTest.php new file mode 100644 index 0000000..6328fd3 --- /dev/null +++ b/tests/Unit/MergeTagEscapingTest.php @@ -0,0 +1,73 @@ +interpolate('Hi {{ name }}', ['name' => 'Pat']))->toBe('Hi Pat'); +}); + +it('escapes values from the context where asked, but not the template author\'s own text', function (): void { + $interpolator = new MergeTagInterpolator; + $html = '

Hi {{ name }}, {{ missing | default:"friend" }}

go'; + + $out = $interpolator->interpolate($html, ['name' => '', 'url' => 'https://x.test/?a=1&b="2"'], escape: true); + + expect($out)->toContain('Hi <script>alert(1)</script>') + ->and($out)->toContain('friend') + ->and($out)->toContain('href="https://x.test/?a=1&b="2""') + ->and($out)->not->toContain('