Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mailer] Fix signed emails breaking the profiler #53934

Merged
merged 1 commit into from Feb 14, 2024

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Feb 14, 2024

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #53928
License MIT

@HypeMC
Copy link
Contributor Author

HypeMC commented Feb 14, 2024

For 6.4+ : HypeMC@834167c

diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig
index 386438b825..a0d2d6388b 100644
--- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig
+++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig
@@ -278,8 +278,24 @@
                             {% for event in collector.events.events(transport) %}
                                 <tr class="mailer-email-summary-table-row {{ loop.first ? 'active' }}" data-target="#email-{{ loop.index }}">
                                     <td>{{ loop.index }}</td>
-                                    <td>{{ event.message.getSubject() ?? '(No subject)' }}</td>
-                                    <td>{{ event.message.getTo()|map(addr => addr.toString())|join(', ')|default('(empty)') }}</td>
+                                    <td>
+                                        {% if event.message.subject is defined %}
+                                            {{ event.message.getSubject() ?? '(No subject)' }}
+                                        {% elseif event.message.headers.has('subject') %}
+                                            {{ event.message.headers.get('subject').toString()|split(': ', 2)[1]|default('(No subject)') }}
+                                        {% else %}
+                                            (No subject)
+                                        {% endif %}
+                                    </td>
+                                    <td>
+                                        {% if event.message.to is defined %}
+                                            {{ event.message.getTo()|map(addr => addr.toString())|join(', ')|default('(empty)') }}
+                                        {% elseif event.message.headers.has('to') %}
+                                            {{ event.message.headers.get('to').toString()|split(': ', 2)[1]|default('(empty)') }}
+                                        {% else %}
+                                            (empty)
+                                        {% endif %}
+                                    </td>
                                     <td class="visually-hidden"><button class="mailer-email-summary-table-row-button" data-target="#email-{{ loop.index }}">View email details</button></td>
                                 </tr>
                             {% endfor %}
@@ -323,18 +339,42 @@
                     <div class="tab-content">
                         <div class="card-block">
                             <p class="mailer-message-subject">
-                                {{ message.getSubject() ?? '(No subject)' }}
+                                {% if message.subject is defined %}
+                                    {{ message.getSubject() ?? '(No subject)' }}
+                                {% elseif message.headers.has('subject') %}
+                                    {{ message.headers.get('subject').toString()|split(': ', 2)[1]|default('(No subject)') }}
+                                {% else %}
+                                    (No subject)
+                                {% endif %}
                             </p>
                             <div class="mailer-message-headers">
-                                <p><strong>From:</strong> {{ message.getFrom()|map(addr => addr.toString())|join(', ')|default('(empty)') }}</p>
-                                <p><strong>To:</strong> {{ message.getTo()|map(addr => addr.toString())|join(', ')|default('(empty)') }}</p>
+                                <p>
+                                    <strong>From:</strong>
+                                    {% if message.from is defined %}
+                                        {{ message.getFrom()|map(addr => addr.toString())|join(', ')|default('(empty)') }}
+                                    {% elseif message.headers.has('from') %}
+                                        {{ message.headers.get('from').toString()|split(': ', 2)[1]|default('(empty)') }}
+                                    {% else %}
+                                        (empty)
+                                    {% endif %}
+                                </p>
+                                <p>
+                                    <strong>To:</strong>
+                                    {% if message.to is defined %}
+                                        {{ message.getTo()|map(addr => addr.toString())|join(', ')|default('(empty)') }}
+                                    {% elseif message.headers.has('to') %}
+                                        {{ message.headers.get('to').toString()|split(': ', 2)[1]|default('(empty)') }}
+                                    {% else %}
+                                        (empty)
+                                    {% endif %}
+                                </p>
                                 {% for header in message.headers.all|filter(header => (header.name ?? '')|lower not in ['subject', 'from', 'to']) %}
                                     <p class="mailer-message-header-secondary">{{ header.toString }}</p>
                                 {% endfor %}
                             </div>
                         </div>
 
-                        {% if message.attachments %}
+                        {% if message.attachments is defined and message.attachments %}
                             <div class="card-block">
                                 {% set num_of_attachments = message.attachments|length %}
                                 {% set total_attachments_size_in_bytes = message.attachments|reduce((total_size, attachment) => total_size + attachment.body|length, 0) %}
@@ -364,9 +404,10 @@
                         {% endif %}
 
                         <div class="card-block">
-                            {% set textBody = message.textBody %}
-                            {% set htmlBody = message.htmlBody %}
                             <div class="sf-tabs sf-tabs-sm">
+                            {% if message.htmlBody is defined %}
+                                {% set textBody = message.textBody %}
+                                {% set htmlBody = message.htmlBody %}
                                 <div class="tab {{ not textBody ? 'disabled' }} {{ textBody ? 'active' }}">
                                     <h3 class="tab-title">Text content</h3>
                                     <div class="tab-content">
@@ -414,6 +455,23 @@
                                         {% endif %}
                                     </div>
                                 </div>
+                            {% else %}
+                                {% set body = message.body ? message.body.toString() : null %}
+                                <div class="tab {{ not body ? 'disabled' }} {{ body ? 'active' }}">
+                                    <h3 class="tab-title">Content</h3>
+                                    <div class="tab-content">
+                                        {% if body %}
+                                            <pre class="mailer-email-body prewrap" style="max-height: 600px">
+                                                {{- body }}
+                                            </pre>
+                                        {% else %}
+                                            <div class="mailer-empty-email-body">
+                                                <p>The body is empty.</p>
+                                            </div>
+                                        {% endif %}
+                                    </div>
+                                </div>
+                            {% endif %}
                             </div>
                         </div>
                     </div>

@nicolas-grekas
Copy link
Member

Thank you @HypeMC.

@nicolas-grekas nicolas-grekas merged commit 0984e3b into symfony:5.4 Feb 14, 2024
10 of 11 checks passed
@HypeMC HypeMC deleted the fix-mailer-profiler branch February 14, 2024 16:03
@HypeMC HypeMC mentioned this pull request Feb 14, 2024
nicolas-grekas added a commit that referenced this pull request Feb 15, 2024
This PR was merged into the 5.4 branch.

Discussion
----------

[Mailer] Simplify fix

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Follow-up to #53934, I didn't notice the `getBodyAsString()` method, it simplifies the code nicely.

For 6.4+: HypeMC@1bb254d

```diff
diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig
index a0d2d6388b..e220e73d4b 100644
--- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig
+++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig
@@ -282,7 +282,7 @@
                                         {% if event.message.subject is defined %}
                                             {{ event.message.getSubject() ?? '(No subject)' }}
                                         {% elseif event.message.headers.has('subject') %}
-                                            {{ event.message.headers.get('subject').toString()|split(': ', 2)[1]|default('(No subject)') }}
+                                            {{ event.message.headers.get('subject').bodyAsString()|default('(No subject)') }}
                                         {% else %}
                                             (No subject)
                                         {% endif %}
@@ -291,7 +291,7 @@
                                         {% if event.message.to is defined %}
                                             {{ event.message.getTo()|map(addr => addr.toString())|join(', ')|default('(empty)') }}
                                         {% elseif event.message.headers.has('to') %}
-                                            {{ event.message.headers.get('to').toString()|split(': ', 2)[1]|default('(empty)') }}
+                                            {{ event.message.headers.get('to').bodyAsString()|default('(empty)') }}
                                         {% else %}
                                             (empty)
                                         {% endif %}
@@ -342,7 +342,7 @@
                                 {% if message.subject is defined %}
                                     {{ message.getSubject() ?? '(No subject)' }}
                                 {% elseif message.headers.has('subject') %}
-                                    {{ message.headers.get('subject').toString()|split(': ', 2)[1]|default('(No subject)') }}
+                                    {{ message.headers.get('subject').bodyAsString()|default('(No subject)') }}
                                 {% else %}
                                     (No subject)
                                 {% endif %}
@@ -353,7 +353,7 @@
                                     {% if message.from is defined %}
                                         {{ message.getFrom()|map(addr => addr.toString())|join(', ')|default('(empty)') }}
                                     {% elseif message.headers.has('from') %}
-                                        {{ message.headers.get('from').toString()|split(': ', 2)[1]|default('(empty)') }}
+                                        {{ message.headers.get('from').bodyAsString()|default('(empty)') }}
                                     {% else %}
                                         (empty)
                                     {% endif %}
@@ -363,7 +363,7 @@
                                     {% if message.to is defined %}
                                         {{ message.getTo()|map(addr => addr.toString())|join(', ')|default('(empty)') }}
                                     {% elseif message.headers.has('to') %}
-                                        {{ message.headers.get('to').toString()|split(': ', 2)[1]|default('(empty)') }}
+                                        {{ message.headers.get('to').bodyAsString()|default('(empty)') }}
                                     {% else %}
                                         (empty)
                                     {% endif %}
```

Commits
-------

fc7c992 [Mailer] Simplify fix
This was referenced Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants