Message header display shows stale `Sender: ...` header for all messages viewed after displaying S/MIME message which shows `Sender: ...` if `From:` != signing address (with mailnews.headers.showSender = false)
Categories
(Thunderbird :: Security, defect, P1)
Tracking
(thunderbird_esr78 affected, thunderbird91? fixed, thunderbird92 fixed)
People
(Reporter: as2014+bugzilla, Assigned: KaiE)
References
(Regression)
Details
(4 keywords, Whiteboard: [Better STR comment 5])
Attachments
(4 files, 1 obsolete file)
(deleted),
message/rfc822
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
thomas8
:
review+
wsmwk
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:83.0) Gecko/20100101 Firefox/83.0
Steps to reproduce:
Starting with Thunderbird 78, running Windows 10 or Linux x64, we observe the erroneous displaying of a "Sender:" header if a particular type of message was previewed. The "Sender:" header of the triggering message is then display on all subsequently viewed messages.
Steps to reproduce:
Thunderbird 78.x, tested up to 78.5.1;
add the attached minimal example S/MIME signed message smimesendertest.eml
to a folder (IMAP or local);
preview the message (or open it in a tab).
Actual results:
The example message includes the header Sender: FALSE SENDER <postmaster+somebody@fam.tuwien.ac.at>
which is correctly displayed in the preview. After this, the same header will be shown on all subsequently previewed messages (or ones open in tabs) which is of course incorrect.
Testing showed that a crucial condition of messages that trigger this bug is the fact that the RFC5322-From of the S/MIME signed message does not match the signers address. We observed this on a local mailing list that changes the "From:".
Reporter | ||
Comment 1•4 years ago
|
||
Screenshot 1 shows preview of triggering example smimesendertest.eml
featuring the correct display of the "Sender:" header.
Reporter | ||
Comment 2•4 years ago
|
||
Screenshot 2 shows a preview of a message sent by Bugzilla opened after previewing smimesendertest.eml
. The screenshot shows the erroneous display of a Sender:
header which is not part of the Bugzilla message.
Updated•4 years ago
|
Comment 4•3 years ago
|
||
Comment on attachment 9191420 [details]
Testcase 1: sMimeSenderHeaderTest.eml with From
!= signer address
...
Comment 5•3 years ago
|
||
str |
Magnus, you've set this as P1 due to the very exposed wrong UI state (9 duplicates). Now that it's actionable, could you assign someone?
I'll add a possible starting point in code in my next comment.
Thank you very much Andreas Sch. for this crystal-clear bug report in perfect shape!
Let's reopen this one for a clean start to work on bug 1670243, which doesn't have a good description and is currently already at comment 29 due to a worthwhile effort of trying to figure out bit by bit what's going on...
Confirmed for Release 78.12.0 (32-bit), Beta 91.0b2 (64-bit), and Daily 91.0b2 (64-bit).
- Happens exactly as described after previewing the testcase message of attachment 9191420 [details].
- Obviously highly irritating that TB keeps showing a stale
Sender:
header on any unrelated messages. - Priority P1 set by mkmelin on original bug 1670243, which has 8 more duplicates.
STR
- Copy message of minimal testcase 1 (attachment 9191420 [details], sMimeSenderHeaderTest.eml) into any TB folder (IMAP or local).
S/MIME signed message
From: != signing address
, which triggers display ofSender:
header (important) even when...mailnews.headers.showSender = false
(important; bug may not happen otherwise)
- First view other messages before viewing the testcase (for comparison).
- View testcase 1 (in preview pane or in a tab).
- Then view any unrelated messages in any folder of any account after viewing the testcase.
Actual result
- Before viewing testcase, all messages will display with correct headers.
- After viewing testcase, all messages will henceforth display with the stale
Sender:
header of testcase 1.
Expected result
Sender:
header must be hidden from message header display for any message which does not have its ownSender:
header which we need to show; while still respectingmailnews.headers.showSender
pref.- See comment 7 for the current code which seems to attempt just that.
Updated•3 years ago
|
Comment 6•3 years ago
|
||
Alice, could you kindly find the regression range?
It's a high-value bug ;-)
Comment 7•3 years ago
|
||
Possible starting point in code
(updated from Gerald's Bug 1670243 Comment 24 - thanks for the pointer!)
The function onSMIMEBeforeShowHeaderPane()
looks as if it should do exactly what is needed to prevent this bug.
So probably for some reason, either it's not running at all or it fails to achieve.
No errors in console afsics.
function onSMIMEBeforeShowHeaderPane() {
// For signed messages with differing Sender as signer we force showing Sender.
// If we're now in a different message, hide the (old) sender row and remove
// it from the header view, so that Sender normally isn't shown.
if (
"sender" in gExpandedHeaderView &&
!Services.prefs.getBoolPref("mailnews.headers.showSender")
) {
gExpandedHeaderView.sender.enclosingRow.collapsed = true;
delete gExpandedHeaderView.sender;
}
}
https://searchfox.org/comm-central/search?q=onSMIMEBeforeShowHeaderPane&path=
Updated•3 years ago
|
Updated•3 years ago
|
Comment 10•3 years ago
|
||
Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=3d544985b7296fd45fcd15123e607acf58bc1057&tochange=284f2743822fb496e79f927afcfa23dc4ee7c5cb
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=79b08a0a1c3fa2834e668b205fd69ff79c8630d1&tochange=c31591e0b66f277398bee74da03c49e8f8a0ede0
Assignee | ||
Comment 11•3 years ago
|
||
Alice, thanks a lot.
Thomas, thanks for pointing us to the related code.
This was regressed by bug 1537729 which missed the code that Thomas quoted in comment 8.
Assignee | ||
Comment 12•3 years ago
|
||
Updated•3 years ago
|
Comment 13•3 years ago
|
||
Awesome, thanks Kai for the patch, which indeed seems to fix this from my local test.
Comment 14•3 years ago
|
||
To speed up landing this one-liner patch, I've fixed the nit mentioned my mkmelin.
Has r=mkmelin.
# HG changeset patch
# User Kai Engert <kaie@kuix.de>
# Parent 7401d1f46d1b9d335d9fcfd954f0b87c92b96188
Bug 1680843 - Fix regression that causes a Sender: header to remain shown after switching message. r=mkmelin,ThomasD
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 15•3 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/9c82d0b39103
Fix regression that causes a Sender: header to remain shown after switching message. r=ThomasD
Updated•3 years ago
|
Comment 16•3 years ago
|
||
Comment on attachment 9233064 [details] [diff] [review]
1680843_fixSenderHeaderDisplay.diff
[Approval Request Comment]
Regression caused by (bug #): 1537729
User impact if declined: Duplicate "Sender" header visible
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Low, this is a small change to the display style.
Comment 17•3 years ago
|
||
Comment on attachment 9233064 [details] [diff] [review]
1680843_fixSenderHeaderDisplay.diff
[Triage Comment]
Approved for beta
Comment 18•3 years ago
|
||
bugherder uplift |
Thunderbird 91.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/373bfbdc06d4
Comment 19•3 years ago
|
||
Great teamwork!
Verified FIXED on 91.0b5 (64-bit), Win10
Description
•