1367191 - No authorisation prompt displayed when inserting image into email body if image URL requires authentication (regression TB 52)

Status: RESOLVED FIXED

(Thunderbird :: Security, defect)

Description

[Yury Vidineev]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.9) Gecko/20100101 Goanna/3.2 Firefox/45.9 PaleMoon/27.3.0
Build ID: 20170422232241

Steps to reproduce:

1. Write new email
2. Insert -> Image
3. Paste URL to an image that require authentication (returns HTTP/1.1 401 Unauthorized)





Actual results:

It'll not be able to fetch it (broken image icon). It worked before update to 52.1.1. I think it can be connected with data URIs for images


Expected results:

I expect that image will be downloaded and attached to email (like it worked in thunderbird 45.8.0)

Comment 1

[Yury Vidineev]

I've tested in Thunderbird 54 beta and face the same issue

Comment 2

[Jorg K (CEST = GMT+2)]

Images from http(s) resources are not converted to data: URLs, so this should not be connected to the changes we made.

However, I set this up. I placed a picture here:
http://www.jorgk.com/auth/ausflag.png and this needs authorisation (which is guest1/guest1). In TB 45 I get prompted when inserting the image, in TB 52 there is no prompt.

I'm not aware of breaking this with the changes we made to the compose window, but maybe we did.

Alice, can you find the regression here:
As I said, write an e-mail, insert an image from http://www.jorgk.com/auth/ausflag.png and enter guest1/guest1 if prompted.

The bug that might have broken it handed here, but I doubt it:
https://hg.mozilla.org/comm-central/rev/14bd3b70a7fda2bfb80b0905adb05ceef296e2d4

Or perhaps not being an editor any more will stop the prompt.

Comment 3

[Jorg K (CEST = GMT+2)]

Workaround: Enter, for example, http://guest1:[e-mail]/auth/ausflag.png as the image name ;-)

Comment 4

[Alice0775 White]

good:http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1472724143/thunderbird-51.0a1.en-US.win32.zip
bad :http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1472768360/thunderbird-51.0a1.en-US.win32.zip

https://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2016-09-01+03%3A00%3A00&enddate=2016-09-01+16%3A00%3A00
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=cddb63dcd5cf51f63d01e4c01c551c458765f042&tochange=f3f22e07b8e8beb02592cd68c83267fe570e59e9

Suspect: Bug 1266836



Error in Error Console:

GET 
http://www.jorgk.com/auth/ausflag.png [HTTP/1.1 401 Unauthorized 285ms]

TypeError: win.gBrowser is undefined[Learn More]  nsLoginManagerPrompter.js:1426:11

nsPrompter: Delegation to password manager failed: [Exception... "[JavaScript Error: "win.gBrowser is undefined" {file: "resource://gre/components/nsLoginManagerPrompter.js" line: 1426}]'[JavaScript Error: "win.gBrowser is undefined" {file: "resource://gre/components/nsLoginManagerPrompter.js" line: 1426}]' when calling method: [nsIPromptFactory::getPrompt]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: resource://gre/components/nsPrompter.js :: getPrompt :: line 42"  data: yes]  nsPrompter.js:44

uncaught exception: 2147500033  nsPrompter.js:363:5

TypeError: checkValue is undefined[Learn More]  nsPrompter.js:740:13

Comment 5

[Jorg K (CEST = GMT+2)]

Thanks, Alice!

SM ported bug 1266836 here: https://hg.mozilla.org/comm-central/rev/65bcae326215

Comment 6

[Jorg K (CEST = GMT+2)]

FRG, any idea what we need to do to get this going in the compose window? What happens when you insert
  http://www.jorgk.com/auth/ausflag.png
into a new e-mail composition in SM?

Comment 7

[Yury Vidineev]

Guys, thanks for so fast replies!
And for workaround :)

Comment 8

[Frank-Rainer Grahl (:frg)]

>> TypeError: win.gBrowser is undefined[Learn More]  nsLoginManagerPrompter.js:1426:11

I have run into this in SeaMonkey too. See bug 1347857. The autocomplete code was changed and tries to get a parent browser somewhere. This doesn't work because mail in both TB and SM hasn't a browser.

I just hadn't time yet to investigate further and for SeaMonkey there is a workaround but a change in mozilla code is probably needed. I just wonder why you ran into it in the current ESR only? Maybe my conclusion is/was wrong and Mike can shed some light on this. Overall its a little over my pay grade aka. not enough knowledge :)

Comment 9

[Frank-Rainer Grahl (:frg)]

I forgot. While this isn't autocomplete the password manager code was changed too here.

Comment 10

[Jorg K (CEST = GMT+2)]

According to comment #4 this actually regressed in TB 51 already and no one notices it.

Comment 11

[Frank-Rainer Grahl (:frg)]

> this actually regressed in TB 51 already and no one notices it.

Then it fits. Autocomplete and the login prompter code was changed for Gecko 51.

Comment 12

[Jorg K (CEST = GMT+2)]

I'm sure Mike knows everything but perhaps the author of bug 1266836 can help us as well. Johann, what do we need to do to get the authorisation prompt back for the Thunderbird compose window?

Comment 13

[Johann Hofmann [:johannh]]

Yup, sorry, this is tracked in bug 1350152. Mike Kaply agreed to take the bug but according to him all consumers had worked around the issue already. I assume that's not the case for Thunderbird, then?

Would you mind chiming in on bug 1350152? FWIW the bug shouldn't be hard to solve (see comments) and I'm happy to mentor/review but I don't really have the resources to set up testing on Thunderbird or another XUL app right now.

Thank you, and again sorry for breaking basic auth for you :)

Comment 14

[Johann Hofmann [:johannh]]

Actually since this is in Thunderbird depending on bug 1350152 might be a better choice than duping...

Comment 15

[Jorg K (CEST = GMT+2)]

(In reply to Johann Hofmann [:johannh] from comment #13)
> FWIW the bug shouldn't be hard to solve (see comments) ...
Sure if you know where to start and what you're doing. You need to understand the situation of the TB team. We need to be experts in e-mail and also experts in *most* areas of Mozilla core since we interface with most of them. So we need to be experts in DOM, Editor, Intl, Netwerk, Cache2, just to name a few. In short: It's an impossible task.

> ... and I'm happy to mentor/review but I don't really have
> the resources to set up testing on Thunderbird or another XUL app right now.
Thanks but I wouldn't know where to start.

Bug 1350152 comment #5 talks about (quote): "bringing back code doing `….chromeEventHandler.ownerDocument.defaultView` as a fallback"

Mike, are you still offering to fix this?

Comment 16

[Jorg K (CEST = GMT+2)]

Attachment: 1367191-wip.patch - Parch by FRG

Frank-Rainer Grahl, the SeaMonkey maintainer sent me this patch. If fixes the problem in Thunderbird.

Comment 17

[Jorg K (CEST = GMT+2)]

Let's try again with better grammar:
Frank-Rainer Grahl, the SeaMonkey maintainer, sent me this patch. It fixes the problem in Thunderbird.

Comment 18

[Frank-Rainer Grahl (:frg)]

Its just the old code based on the above comment:

>> Bug 1350152 comment #5 talks about (quote): "bringing back code doing 
>> `….chromeEventHandler.ownerDocument.defaultView` as a fallback"

It also fixed SeaMonkey bug 1347857 for me but I did only very limited testing.

Comment 19

[Frank-Rainer Grahl (:frg)]

Oh btw. can't be used 1:1 because it doesn't return a browser. This would need to be fixed.

FRG

Comment 20

[Frank-Rainer Grahl (:frg)]

Attachment: 1367191-wip2.patch

Could you try this one.

Comment 21

[Jorg K (CEST = GMT+2)]

That works, too.

Comment 22

[Frank-Rainer Grahl (:frg)]

Great. Now someone more familiar with the core stuff needs to take a look.

Comment 23

[Mike Kaply [:mkaply]]

No, I don't have time to work on this. When I first took a look, it didn't seem as simple as I thought.

Comment 24

[Johann Hofmann [:johannh]]

I'm looking into bug 1350152 and we might solve this soon.

Comment 25

[Jorg K (CEST = GMT+2)]

Reporter: Can you test this on today's Daily (will be compiled in a few hours) now that bug 1350152 is fixed.

We'll see how to backport this to TB 52 ESR.

Comment 26

[Jorg K (CEST = GMT+2)]

Fixed by bug 1350152, I've just tried it.

Comment 27

[Jorg K (CEST = GMT+2)]

Attachment: Don't forget to uplift M-C change to M-ESR52

Comment 28

[Yury Vidineev]

Hello
I can confirm that it's fixed in 57.0a1 (2017-08-04) (64-bit)
Thank you guys! Really appreciate it!

Comment 29

[Frank-Rainer Grahl (:frg)]

Attachment: 1350152-ESR52.patch

Rebased ESR 52 version of bug 1350152 for ESR52 SeaMonkey and TB release branches.

Comment 30

[Jorg K (CEST = GMT+2)]

Comment on attachment 8894214 [details] [diff] [review]
1350152-ESR52.patch

[Triage Comment]

Comment 31

[Jorg K (CEST = GMT+2)]

Beta uplift for TB 56 beta 1:
https://hg.mozilla.org/releases/mozilla-beta/rev/74e49a52007c609495c6cbc673487ed457838d9e

Comment 32

[Jorg K (CEST = GMT+2)]

THUNDERBIRD_52_VERBRANCH:
https://hg.mozilla.org/releases/mozilla-esr52/rev/f822bda79c28da0ad93b67d46abcb66f6d5c8c92

Will be fixed in TB 52.3.

Comment 33

[Jorg K (CEST = GMT+2)]

Trick to get this off the list of things needing check-in: releases/comm-esr52/rev