Escape merge-tag values in transactional HTML; one-line subjects; validated List-Unsubscribe

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.
 
 ---
 

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',

src/MailBuilder.php

@@ -127,9 +127,9 @@ public static function mergeTags(): MergeTagRegistry
      *
      * @param  array<string, mixed>|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);
     }
 
     /**

src/MergeTags/MergeTagInterpolator.php

@@ -13,34 +13,47 @@ 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<string, mixed>  $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);
 
         // 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);
 
                     foreach ($filters as $filter) {
                         $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

src/MergeTags/MergeTagRegistry.php

@@ -93,10 +93,10 @@ public function sampleContext(): array
      *
      * @param  array<string, mixed>|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);
     }
 }

tests/Unit/MergeTagEscapingTest.php

@@ -0,0 +1,73 @@
+<?php
+
+declare(strict_types=1);
+
+use Odden\MailBuilder\Mail\TemplateMailable;
+use Odden\MailBuilder\MailBuilder;
+use Odden\MailBuilder\MergeTags\MergeTagInterpolator;
+
+it('leaves values as they are unless asked to escape', function (): void {
+    $interpolator = new MergeTagInterpolator;
+
+    expect($interpolator->interpolate('Hi {{ name }}', ['name' => '<b>Pat</b>']))->toBe('Hi <b>Pat</b>');
+});
+
+it('escapes values from the context where asked, but not the template author\'s own text', function (): void {
+    $interpolator = new MergeTagInterpolator;
+    $html = '<p>Hi {{ name }}, {{ missing | default:"<em>friend</em>" }}</p><a href="{{ url }}">go</a>';
+
+    $out = $interpolator->interpolate($html, ['name' => '<script>alert(1)</script>', 'url' => 'https://x.test/?a=1&b="2"'], escape: true);
+
+    expect($out)->toContain('Hi &lt;script&gt;alert(1)&lt;/script&gt;')
+        ->and($out)->toContain('<em>friend</em>')
+        ->and($out)->toContain('href="https://x.test/?a=1&amp;b=&quot;2&quot;"')
+        ->and($out)->not->toContain('<script>');
+});
+
+it('puts markup in through the raw filter, and only there', function (): void {
+    $interpolator = new MergeTagInterpolator;
+
+    $out = $interpolator->interpolate('{{ table | raw }} / {{ note }}', ['table' => '<table></table>', 'note' => '<i>x</i>'], escape: true);
+
+    expect($out)->toBe('<table></table> / &lt;i&gt;x&lt;/i&gt;');
+});
+
+it('applies the other filters before escaping', function (): void {
+    $out = (new MergeTagInterpolator)->interpolate('{{ name | upper }}', ['name' => 'a&b'], escape: true);
+
+    expect($out)->toBe('A&amp;B');
+});
+
+it('escapes in the HTML of a TemplateMailable, not in the subject or the plain text', function (): void {
+    $mail = new TemplateMailable(
+        template: '<p>Hello {{ name }}</p>',
+        data: ['name' => "O'Brien <x>\r\nBcc: evil@example.com"],
+        subjectLine: 'Receipt for {{ name }}',
+    );
+
+    expect($mail->compiledHtml)->toContain('O&#039;Brien &lt;x&gt;')
+        ->and($mail->subjectLine)->toBe("Receipt for O'Brien <x> Bcc: evil@example.com")
+        ->and($mail->subjectLine)->not->toContain("\n")
+        ->and($mail->compiledPlainText)->toContain("O'Brien");
+});
+
+it('only sends a List-Unsubscribe header for an http(s) or mailto address with nothing that can end the value', function (string $url, bool $sent): void {
+    $mail = new TemplateMailable(template: '<p>Hi</p>', data: ['unsubscribe_url' => $url], subjectLine: 'S');
+
+    $headers = $mail->headers()->text;
+
+    expect(isset($headers['List-Unsubscribe']))->toBe($sent);
+})->with([
+    'https' => ['https://example.com/u/1?x=1&y=2', true],
+    'mailto' => ['mailto:unsubscribe@example.com?subject=stop', true],
+    'javascript' => ['javascript:alert(1)', false],
+    'data' => ['data:text/html,x', false],
+    'closing bracket' => ['https://example.com/u>, <https://evil.example/x', false],
+    'newline' => ["https://example.com/u\r\nBcc: x@example.com", false],
+    'space' => ['https://example.com/a b', false],
+]);
+
+it('keeps MailBuilder::interpolate unescaped by default for the campaign code that escapes its own values', function (): void {
+    expect(MailBuilder::interpolate('{{ a }}', ['a' => '<b>']))->toBe('<b>')
+        ->and(MailBuilder::interpolate('{{ a }}', ['a' => '<b>'], escape: true))->toBe('&lt;b&gt;');
+});