301649 - Invalid local image filenames cause forwarded-inline messages to spin indefinitely on send.

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: 2005-07-21-06, Windows XP SP2 SeaMonkey trunk.

Overview: When an HTML-formatted message contains and invalid filename (i.e.
non-existing), and this email message is forwarded inline, on send SeaMonkey
Mail spins indefinitely, 'Attaching filename.gif...'

Steps to Reproduce:

1. Load and select/view the attached .eml file in SeaMonkey Mail.
2. Choose Message -> Forward As -> Inline from the top-level menu.
3. Click Send.

Expected Results:

As in Thunderbird 0.9, I'd expect "There was a problem including the file
filename.gif in the message.  Would you like to continue sending the message
without this file?"

(OK) (Cancel)

Actual Results:

We spin saying "Attaching filename.gif..."

Additional Info:

We actually have code in place that is designed to prevent this.

http://lxr.mozilla.org/seamonkey/source/mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties#139

139 ## @name NS_MSG_FAILURE_ON_OBJ_EMBED_WHILE_SAVING
140 12533=There was a problem including the file %.200s in the message. Would
you like to continue saving the message without this file?

See
http://lxr.mozilla.org/seamonkey/source/mailnews/compose/src/nsMsgAttachmentHandler.cpp#1062

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Phishing email which has a broken remote image

Note that I actually meant to point to
NS_MSG_FAILURE_ON_OBJ_EMBED_WHILE_SENDING, not '..._SAVING'.

Comment 2

[David :Bienvenu]

I was able to reproduce this and started debugging it - we're never getting an
OnStopRequest. I'll debug further.

Comment 3

[David :Bienvenu]

the img src is valid, but we're still failing to attach it. My guess is that
we're blocking the url somewhere in the content policy stuff, but I'm not
hitting a few relevant breakpoints.

Comment 4

[David :Bienvenu]

ok, my new guess is that this is a temp file issue. That message has a bunch of
images that we're going to fetch. Each one is fetched to a temp file. I moved
the footer url to the top of the message (the one that was failing before), and
we were able to fetch that url fine, but some other image failed. Somehow we
might be colliding on temp files...I suspect this just because the temp file
code has changed recently.

Comment 5

[David :Bienvenu]

I believe the problem is these img's <img alt="" src="scrisoare_files/s.gif"
height="5" width="1"> 

which don't look very valid. So the problem seems to be that we don't get the
OnStopRequest that these urls have failed.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

do you get onStartRequest?

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

As we discovered in testing this, the following much-simpler testcase will also
trigger the same behavior, but may have a different cause:

1. In Mail compose, do an Insert | Image.
2. Under the Location tab, type any non-valid filename you want.
3. Click OK (ALT text isn't important here).
4. Click Send.

You'll see that we also spin here doing "Attaching ..."

Comment 8

[Scott MacGregor]

I suspect the original case reported here is no longer an issue now that 303752
is fixed. But you could still see this using stephen's simpler steps by
attaching a file that doesn't exist to a new message. Plussing for now, but I
wouldn't hold the release for it if we don't get a fix.

Comment 9

[Scott MacGregor]

The only remaining issue I see here is when you manually type in a file path
into editor's insert image dialog, we will spin on send. This is because that
dialog expects a file:// url and not a OS specific file path (like c:\temp\foo.png)

I suspect this dialog has always only worked with file urls.

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #9)
> The only remaining issue I see here is when you manually type in a file path
> into editor's insert image dialog, we will spin on send. This is because that
> dialog expects a file:// url and not a OS specific file path (like
c:\temp\foo.png)
> 
> I suspect this dialog has always only worked with file urls.

Scott, David: I've just verified that my original case works fine now--I can
forward inline that LaSalle Banking spoof message.

Builds: 2005-08-19-06 of SeaMonkey Mail trunk and version 1.6a1 (20050819) of
Thunderbird trunk.

Thanks so much for fixing that!

Comment 11

[Ian Neal]

Attachment: FixupURI patch v2.0

This patch:
* Adds a FixupURI to ValidateImage so valid urls are generated

Comment 12

[David :Bienvenu]

Comment on attachment 193748 [details] [diff] [review]
FixupURI patch v2.0

if this file is used by editor/composer, we'll need moa, probably from
glazou...

Comment 13

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 193748 [details] [diff] [review]
FixupURI patch v2.0

Sorry, this patch is harmful. It is going to break img insertion relatively to
a document in Composer/Nvu.

1. Create a new doc and type some chars
2. save the document
3. insert an image from your hard disk (like c:\foo.png) making sure the
checkbox "URL is relative to page location" is checked
4. see the image is broken
5 look at the source and start screaming

Comment 14

[Ian Neal]

Attachment: Revised patch v2.1 (Checked in trunk and branch)

Changes since v2.0:
* Added check for make relative so fixup is not done if it is relative

Comment 15

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 194332 [details] [diff] [review]
Revised patch v2.1 (Checked in trunk and branch)

seems to be better.
r=[e-mail]

Comment 16

[Ian Neal]

Comment on attachment 194332 [details] [diff] [review]
Revised patch v2.1 (Checked in trunk and branch)

Requesting approval for simple low risk regression-fix patch

Comment 17

[Ian Neal]

Comment on attachment 194332 [details] [diff] [review]
Revised patch v2.1 (Checked in trunk and branch)

Checking in
EdImageOverlay.js;
new revision: 1.18; previous revision: 1.17
done

Comment 18

[Asa Dotzler [:asa]]

Please resolve the bug when it lands on the trunk so that we can get a trunk
verification before evaluating for the branch. Thanks

Comment 19

[Ian Neal]

-> Fixed as per request by Asa

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED in version 1.6a1 (20050917) of Thunderbird trunk and SeaMonkey
1.1a:Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050917
Mozilla/1.0

Comment 21

[Ian Neal]

Comment on attachment 194332 [details] [diff] [review]
Revised patch v2.1 (Checked in trunk and branch)

Checking in (branch)
EdImageOverlay.js;
new revision: 1.17.18.1; previous revision: 1.17
done

Comment 22

[Charles Faulk]

Assuming that 319818 and 324110 (under discussion) are due to the same or similar bug(s) then the following applies to both.

Worikng with bugs 319818 and 324110 [e-mail] shows that this bug or bug(s) first showed itself to the public after TB release ver 1.5 (20051201).   Looking at the Bugs FIxed for each release that are similar to the bug(s) above, one finds that the only bug listed as fixed or changed in this area is 301649 - the "spin indefinitely" part is especially relevant.

"TB ver 1.5  Bug 301649 'Fixed 8/30/05'
-------------------------------------------------------
301649: Invalid local image filenames cause forwarded-inline messages to spin indefinitely on send.

Bug 301649 Build ID: 2005-07-21-06, Windows XP SP2 SeaMonkey trunk.

Overview: When an HTML-formatted message contains and invalid filename (i.e.
non-existing), and this email message is forwarded inline, on send SeaMonkey
Mail spins indefinitely, 'Attaching filename.gif...'

Steps to Reproduce:

1. Load and select/view the attached .eml file in SeaMonkey Mail.
2. Choose Message -> Forward As -> Inline from the top-level menu.
3. Click Send.

Expected Results:
As in Thunderbird 0.9, I'd expect "There was a problem including the file
filename.gif in the message.  Would you like to continue sending the message
without this file?"
(OK) (Cancel)

Actual Results:
We spin saying "Attaching filename.gif...""

"Verified FIXED in version 1.6a1 (20050917) of Thunderbird trunk and SeaMonkey
1.1a:Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050917
Mozilla/1.0"

The behavior of the system in the above bug is strikingly similar to that of the bugs under discussion.  The above bug either wasn't fixed,  wasn't completely fixed or had an underlying cause.  While investigating this problem I also discovered another Bug, 317970, with is also similar to the named bugs and which was thought to be a duplicate of Bug 324110.

"Bug 317970  Description:  [reply]  	 Opened: 2005-11-27 21:10 PDT
------------------------------------------------------------------------------------------------------
With SeaMonkey trunk CVS I was copmosing an email.  I have the "automatically
save every x minutes" pref enabled and when it tried to do so, it hung.  After
restarting, I pulled up the draft (saving seemed to be successful) and sent it
and I got another hang.  I was using a debug build and got this both times:

CopyListener::OnStartCopy()
nsMsgComposeSendListener::OnStartCopy()
CopyListener: SUCCESSFUL ON THE COPY OPERATION!
nsMsgComposeSendListener: Success on the message copy operation!
[hang]

------ Comment #16 From Adam Guthrie  2006-01-20 12:36 PDT  [reply] -------
*** Bug 324110 has been marked as a duplicate of this bug. ***"

I believe that there is a reasonable probability that bugs:
301649
317970
319818
324110
either are the same bug, manifestations of the same bug, or have a common root cause.  If they were solved earlier, they wouldn't be problems now.

It therefore seens reasonable that the four different bugs be combined into a single bug and their votes be likewise combined.

Respectfully submitted,
Charles Faulk, MD