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

VERIFIED FIXED

Status

MailNews Core
Composition
--
major
VERIFIED FIXED
12 years ago
9 years ago

People

(Reporter: stephend, Assigned: Ian Neal)

Tracking

({fixed1.8, regression})

Trunk
x86
Windows XP
fixed1.8, regression
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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
(Reporter)

Comment 1

12 years ago
Created attachment 190092 [details]
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'.
(Reporter)

Updated

12 years ago
Flags: blocking-aviary1.5?
Keywords: regression

Comment 2

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

Comment 3

12 years ago
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.
Assignee: nobody → bienvenu

Comment 4

12 years ago
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

12 years ago
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.
do you get onStartRequest?
(Reporter)

Comment 7

12 years ago
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 ..."

Updated

12 years ago
Depends on: 303752
(Reporter)

Updated

12 years ago
Summary: Invalid remote image filenames cause forwarded-inline messages to spin indefinitely on send. → Invalid local image filenames cause forwarded-inline messages to spin indefinitely on send.

Updated

12 years ago
Flags: blocking-aviary1.5? → blocking1.8b4?

Comment 8

12 years ago
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.
Flags: blocking1.8b4? → blocking1.8b4+

Comment 9

12 years ago
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.
(Reporter)

Comment 10

12 years ago
(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!
(Assignee)

Comment 11

12 years ago
Created attachment 193748 [details] [diff] [review]
FixupURI patch v2.0

This patch:
* Adds a FixupURI to ValidateImage so valid urls are generated
Attachment #193748 - Flags: review?(neil.parkwaycc.co.uk)

Updated

12 years ago
Attachment #193748 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Updated

12 years ago
Attachment #193748 - Flags: superreview?(bienvenu)

Comment 12

12 years ago
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...

Updated

12 years ago
Whiteboard: [needs SR bienvenu]
(Assignee)

Updated

12 years ago
Attachment #193748 - Flags: review?(daniel)

Updated

12 years ago
Attachment #193748 - Flags: superreview?(bienvenu) → superreview+
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
Attachment #193748 - Flags: review?(daniel) → review-

Updated

12 years ago
Whiteboard: [needs SR bienvenu]
(Assignee)

Comment 14

12 years ago
Created attachment 194332 [details] [diff] [review]
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
Assignee: bienvenu → iann_bugzilla
Attachment #193748 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #194332 - Flags: review?(daniel)

Updated

12 years ago
Whiteboard: [needs review daniel]
Comment on attachment 194332 [details] [diff] [review]
Revised patch v2.1 (Checked in trunk and branch)

seems to be better.
r=daniel@glazman.org
Attachment #194332 - Flags: review?(daniel) → review+
(Assignee)

Updated

12 years ago
Attachment #194332 - Flags: review?(neil.parkwaycc.co.uk)

Updated

12 years ago
Attachment #194332 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Updated

12 years ago
Attachment #194332 - Flags: superreview?(bienvenu)

Updated

12 years ago
Attachment #194332 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 16

12 years ago
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
Attachment #194332 - Flags: approval1.8b5?
(Assignee)

Comment 17

12 years ago
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
Attachment #194332 - Attachment description: Revised patch v2.1 → Revised patch v2.1 (Checked in trunk)

Comment 18

12 years ago
Please resolve the bug when it lands on the trunk so that we can get a trunk
verification before evaluating for the branch. Thanks
(Reporter)

Updated

12 years ago
Whiteboard: [needs review daniel]
(Assignee)

Comment 19

12 years ago
-> Fixed as per request by Asa
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 20

12 years ago
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
Status: RESOLVED → VERIFIED

Updated

12 years ago
Attachment #194332 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 21

12 years ago
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
Attachment #194332 - Attachment description: Revised patch v2.1 (Checked in trunk) → Revised patch v2.1 (Checked in trunk and branch)
(Assignee)

Updated

12 years ago
Keywords: fixed1.8

Comment 22

11 years ago
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 mxxxm@volny.cz 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
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.