1946168 - Endless Loop Trying to Display a Specific Mailinglist Digest (with "view attachments inline")

Status: REOPENED

(Thunderbird :: General, defect, P1)

Description

[Jochen Bern]

Attachment: Screenshot_20250205_183835.png

Steps to reproduce:

Received and wanted to read an e-mail from the dovecot mailinglist (where I'm subscribed in digest mode).

TB 128.6.0esr (64-bit) on Fedora (KDE Spin) 40, Updates applied just this week, IMAP server is an M365 EXO, installed extensions: see attached screenshot.

Actual results:

TB attempts to render the e-mail and enters an endless loop (alternating between one pane content saying that DKIM verification is ongoing, and one warning that (only) parts of the e-mail are signed). UI remains reactive, I can end the loop by having another e-mail displayed. (319 kB screen video available.)

Hiding the message pane, I can select the e-mail in the list and save it to a file (not attached right now, I'm apparently limited to one attachment, sorry). Loading the file into TB again triggers the loop (it seems to loop faster then.)

Expected results:

Display the e-mail content in the message pane as usual.

Comment 1

[Jochen Bern]

Can't add another attachment to a later comment, either. I seem to able to "Forward As Attachment" from the Message List, though ...

Comment 2

[Jochen Bern]

Disabled all extensions, restarted TB, switched between Plain Text and Original HTML display modes, loop still happens (except a lot faster, and without the DKIM Verifier's status message, of course).

Comment 3

[Magnus Melin [:mkmelin]]

Please attach the message, as .eml (using the Attach New File button above).

Comment 4

[Jochen Bern]

Attachment: Saved E-Mail that triggers the bug in my TB

Saved E-Mail as requested

Comment 5

[Jochen Bern]

No idea whether that's related, just mentioning it here in case it is: I just found that trying to create an e-mail from a saved template yields an e-mail editing window (screenshot upcoming) with just subject and recipients - no text, no signature ... ?

Comment 6

[Jochen Bern]

Attachment: Screenshot_20250206_101408.png

Open template, get empty mail body ...

Comment 7

[Magnus Melin [:mkmelin]]

Thanks for the testcase, I can reproduce that in daily
(Empty body was bug 1940605)

Comment 8

[Hartmut Welpmann [:welpy-cw]]

I found out two regression windows:

Before
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=a491f73e867f6a7a9dca9621ab1a0943466e0525&tochange=3e86564b260176f87f67dd22e3bdcb5e8f432d02
it worked, after that there's a blank message window indicating 3 attachments, which aren't accessible.

After
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=8305a158c2acdcf296cc1c86ead9f58fe7d05d3b&tochange=8d89958be85decdeafa5cfc7998818ddffeaa83b
it loops.

Comment 9

[Magnus Melin [:mkmelin]]

From https://hg.mozilla.org/comm-central/rev/f9d2da9881a6345f252342ce308d7fb3b42d260e perhaps (didn't try yet)

Comment 10

[Hartmut Welpmann [:welpy-cw]]

Great guess, backing that out fixes this!

Comment 11

[Magnus Melin [:mkmelin]]

Bug 1878664 may be the same.

Comment 12

[Hartmut Welpmann [:welpy-cw]]

(In reply to Magnus Melin [:mkmelin] from comment #11)

Bug 1878664 may be the same.

It's the same underlying cause, since comment 10 applies there as well.

Comment 13

[Jochen Bern]

Attachment: Empty Forward from S/MIME signed+encrypted E-Mail in "Sent" Folder

I suppose that this might well be related, too: I can reply to this S/MIME signed+crypted e-mail from my "Sent" folder, but when I try to forward it, the edit window comes up with an empty body (and the window displaying the original e-mail seems to flicker / get redisplayed for a moment).

Tried this with a signed-but-not-encrypted e-mail in my "Sent" folder (I autosign virtually all e-mails I send) and a SBNEE from someone else in my INBOX, same result.

Forwarding an S/MIME-free e-mail from my INBOX works as expected.

Comment 14

[Jochen Bern]

("Edit As New Message" doesn't work, either. Guess I'll be copy-pasting into a new e-mail and manually attaching the spreadsheet again now.)

Comment 15

[Hartmut Welpmann [:welpy-cw]]

Jochen, if you are still on 128.6.0 this seems to be bug 1940605, otherwise maybe bug 1843860.

Comment 16

[Hartmut Welpmann [:welpy-cw]]

(In reply to Magnus Melin [:mkmelin] from comment #11)

Bug 1878664 may be the same.

This is actually the same bug, duping to the older one.

Comment 17

[Hartmut Welpmann [:welpy-cw]]

As it turns out, bug 1878664 is just a subset of this problem.

Comment 18

[Hartmut Welpmann [:welpy-cw]]

Attachment: Bug 1946168 - Avoid reload loop when displaying messages signed with both PGP and S/MIME. r=KaiE,mkmelin

Comment 19

[Kai Engert [:KaiE:]]

After applying the fix from bug 1878664,
to reproduce this bug it's necessary to have the setting "view attachments inline" enabled.

Comment 20

[Kai Engert [:KaiE:]]

I'd like to explain/document what's happening.

Our code uses two alternative content-type handlers for the multipart/signed type.
Our C++ code registers the S/MIME code as the handler,
and our running JS code registers the OpenPGP handler.

Whenever we process a MIME part, the handler that was registered most recently will be used.
While processing that part, we check whether the protocol listed in the content-type matches the currently registered handler.
If there's a mismatch, we switch the registration to the other handler, and reload.

If the switch/reload was triggered by a subordinate mime part, and a parent part is of the other type, the reload and the repeated loading of the parent type will switch/reload again, resulting in the loop.

Comment 21

[Kai Engert [:KaiE:]]

I think we should not switch/repeat when processing a subordinate mime part that isn't intended for display.
It's unnecessary and undesirable.
We don't automatically decrypt subordinate mime parts anyway.
We don't want to show the signature status of subordinate mime parts.

We should check whether the currently processed mime part is one of the top mime parts that we are trying to display.
Only in that scenario we should allow the reload.

When adding those checks, we need to be careful to handle the scenarios we're allowing.

I'll add more explanations later today.

I don't know yet, whether those better checks will be sufficient to fix this bug, or whether we need the proposed patch in addition.

Comment 22

[Kai Engert [:KaiE:]]

I'm afraid we cannot use this bug as is. I suspect it would introduce a functional regression in the following scenario:

• the most recently processed message,
  and consequently the currently registered content-type,
  if for OpenPGP
• the user opens a message with an outermost S/MIME signature
• that message contains an inner OpenPGP payload,
  as described in bug 1594253

To make that work, I believe the code currently relies on switching the content-type handler.

In the above scenario, the scenario from bug 1594253 will probably no longer work and the inner message no longer gets decrypted,
because it would depend on switching twice.

I'm not completely sure, we must test that scenario prior to landing.

Comment 23

[Kai Engert [:KaiE:]]

I've filed bug 1955042 for a general rework of the multipart/signed content-type handler, which would avoid having to re-register and reload.

Comment 24

[Kai Engert [:KaiE:]]

For the hotfix (prior to working on bug 1955042),
the following might be a sufficient fix:

• if we're processing MIME part "1", then allow switch/reload

• if we're processing MIME part "1.1" and top MIME part "1" contained multipart/signed,
  and 1.1 contains multipart/encrypted or multipart/signed, allow switch/reload

• or, if we're processing MIME part "1.1" and top MIME part "1" contained multipart/encrypted,
  and 1.1 contains multipart/signed, allow switch/reload

I think in all other scenarios we should forbid switching.

Comment 25

[Kai Engert [:KaiE:]]

Attachment: WIP: Bug 1946168 - Limit switching of the multipart/signed content-type handlers to avoid loading loops.

Comment 26

[Kai Engert [:KaiE:]]

Please see the pseudo code in the new attachment.

Comment 27

[Hartmut Welpmann [:welpy-cw]]

Thank you, Kai, for your in-depth analysis of this issue. Your much more precise approach definitely makes more sense.