Closed Bug 1741820 Opened 3 years ago Closed 3 years ago

Cannot open attachments from IMAP when username contains "-" symbol

Categories

(Thunderbird :: Folder and Message Lists, defect)

Thunderbird 95
defect

Tracking

(thunderbird_esr91+ fixed, thunderbird95 fixed)

RESOLVED FIXED
96 Branch
Tracking Status
thunderbird_esr91 + fixed
thunderbird95 --- fixed

People

(Reporter: daniel.verlan-bugzilla, Assigned: darktrojan)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Steps to reproduce:

My IMAP username is an email address and it contains the dash ("-") symbol in the host part. Try to open any attachment except pdf with an external program

Actual results:

The attachment is not opened. The console gives the following message (I deleted the personal information below) :

Uncaught (in promise) TypeError: Window.fetch: imap://username%40a-site%2Ecom@.... is an url with embedded credentials.
saveToFile chrome://messenger/content/msgHdrView.js:2013
saveAndOpen chrome://messenger/content/msgHdrView.js:2030
launchWithApplication chrome://messenger/content/msgHdrView.js:2085
onOK resource://gre/modules/HelperAppDlg.jsm:1084
handleEvent resource://gre/modules/HelperAppDlg.jsm:1125
_fireButtonEvent chrome://global/content/elements/dialog.js:495
_doButtonCommand chrome://global/content/elements/dialog.js:474
_handleButtonCommand chrome://global/content/elements/dialog.js:468

Expected results:

The attachment should have been opened.

In fact, the line 2085 in chrome://messenger/content/msgHdrView.js has the following regexp replace :

     let response = await fetch(url.replace(/^imap:\/\/[\w%]+@/, "imap://"));

which does not take into account the "-" symbol. By changing it to

     let response = await fetch(url.replace(/^imap:\/\/[\w%-]+@/, "imap://"));

the issue disappears.

The userinfo part can contain more characters:

userinfo = unreserved / pct-encoded / sub-delims
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
                 / "*" / "+" / "," / ";" / "="
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Regressed by: 1737711
Summary: Cannot open attachments when username contains "-" symbol → Cannot open attachments from IMAP when username contains "-" symbol

I hadn't really intended that regular expression hack to make it into the final patch, but ended up changing approach about 5 times and here we are. I think it's probably easiest to just strip everything between the imap:// and the @.

Assignee: nobody → geoff
Status: NEW → ASSIGNED

(In reply to Geoff Lankow (:darktrojan) from comment #2)

I hadn't really intended that regular expression hack to make it into the final patch, but ended up changing approach about 5 times and here we are. I think it's probably easiest to just strip everything between the imap:// and the @.

It will break imap://aaa/bbb@ or imap://aaa?bbb@ or imap://aaa#bbb@ etc.
Why don't you use the URL constructor? Can't generic URL parser parse imap URLs?

Hmm, that works differently in Firefox and Thunderbird and appears to be hooked up to the actual URL classes (nsImapUrl etc.). I'd dismissed it because I didn't get any useful results in Firefox when I first read the bug.

We shouldn't avoid the embedded username. It is needed if we have multiple accounts on the same domain.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0967a19bfea6
Use NetUtil instead of fetch to get data when opening attachments. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch

Please note that when you backport bug 1737711 to ESR, this will need to go there, too, so status-thunderbird_esr91: --- → unaffected may be misleading.

96.0a1 2021-11-23 resolves the issue in bug 1741510

May be this is not completely resolved, i.e. the fix is only partially working.
I have 2 emails, each with a PDF attached.
Email 1,
in the message pane I click the attachment and it immediately opens in my PDF reader. Same for right click - open.
A copy of the PDF is saved in my tmp folder.
--6264dded_8025_4922_a861_e150d753fed6
Content-Type: application/pdf;name="TGL Shipment{0} {1}00009366.pdf"
Content-Transfer-Encoding: base64
Content-ID: <c244ad4b-d086-4640-8e9b-5acd4b4e01bf>

Email 2,
in the message pane I click the attachment and nothing is seen to happen.
A copy of the file is saved in my tmp folder.
--619d60f17ec83
Content-type: application/unknown; name=WANlwKYIo.pdf
Content-disposition: attachment; filename=WANlwKYIo.pdf
Content-transfer-encoding: base64

In TB Edit>Settings - Files & Attachments I have 2 entries for PDF, I don't know what they are as the text is too long and I can only see half of it and it doesn't scroll or float an explanation. For the above both are set to use my PDF reader (Okular).

If I set the lower of the 2 entries to "Always ask" then when I click on Email 2 a dialogue opens asking "Open with Okular" or "Save File". Neither are checked. If I select "Open with Okular" the dialogue disappears and nothing happens (although the file is saved to /tmp). If I select "Save File" I get a file save dialogue and can save the PDF to a folder.

Comment on attachment 9251805 [details]
Bug 1741820 - Use NetUtil instead of fetch to get data when opening attachments. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): bug 1737711
User impact if declined: can't open or save attachments in some conditions (2 or more accounts on the same domain, or certain characters in the account username)
Testing completed (on c-c, etc.): landed yesterday
Risk to taking this patch (and alternatives if risky): better than the alternative

Attachment #9251805 - Flags: approval-comm-beta?

Further to Comment 10, 96.0a1 2021-11-24 has regressed again.
When attachment settings Edit>Settings - Files & Attachments is set to "Always ask" attachments cannot be opened from TB and no file is created in /tmp

Attachments as below (Libreoffice writer) no longer open when preferences are set to "Always ask" and are no longer saved to the /tmp folder in the attempt.
--------------nPWrjaT5WCYwwIoNgisqa7o5
Content-Type: application/vnd.oasis.opendocument.text;
name="bluetooth_logging.odt"
Content-Disposition: attachment; filename="bluetooth_logging.odt"
Content-Transfer-Encoding: base64

I am not sure this fits the description in this bug 1741820, but does prevent testing to confirm resolution of this bug.

I'm deferring beta approval pending more info about comment 12

Flags: needinfo?(geoff)

It works for me.

Steve, I suggest removing the entries from the list on the Settings page and trying again. Failing that please open a new bug and post any errors from the Error Console.

Flags: needinfo?(geoff)

Not sure if it's this bug or something else. If I have an attached png file, and set those to "Always ask", it asks but opening doesn't work from there. (If set to open in default app, that does work.)

Just uplift it, it can't get any worse. Users of our fork are happy with the fix we shipped together with bug 1737711 (the regressor) (which we shipped to fix bug 1738490).

Not sure why I couldn't find this yesterday when I looked, but what now seems to happen for me seems like bug 1737711 happening with every file.
If you want any logs let me know, I will await the outcome of Comment 16 .

Comment on attachment 9251805 [details]
Bug 1741820 - Use NetUtil instead of fetch to get data when opening attachments. r=mkmelin

[Triage Comment]
Approved for beta

Attachment #9251805 - Flags: approval-comm-beta? → approval-comm-beta+

Comment on attachment 9251805 [details]
Bug 1741820 - Use NetUtil instead of fetch to get data when opening attachments. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): bug 1737711
User impact if declined: opening attachments totally broken for some users
Testing completed (on c-c, etc.): in 95.0b5
Risk to taking this patch (and alternatives if risky): low

Attachment #9251805 - Flags: approval-comm-esr91?

Comment on attachment 9251805 [details]
Bug 1741820 - Use NetUtil instead of fetch to get data when opening attachments. r=mkmelin

[Triage Comment]
Approved for esr91

Attachment #9251805 - Flags: approval-comm-esr91? → approval-comm-esr91+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: