Last Comment Bug 203629 - Attached images don't display inline if "Do not load remote images..." is turned on
: Attached images don't display inline if "Do not load remote images..." is tur...
Status: VERIFIED FIXED
[for QA, images put into msg compose ...
: regression
Product: MailNews Core
Classification: Components
Component: Attachments (show other bugs)
: Trunk
: x86 Windows XP
: -- normal with 1 vote (vote)
: mozilla1.4beta
Assigned To: (not reading, please use seth@sspitzer.org instead)
: Ninoschka Baca
Mentors:
: 204125 (view as bug list)
Depends on: 51631
Blocks: 203940
  Show dependency treegraph
 
Reported: 2003-04-28 01:29 PDT by Paul Hardacre
Modified: 2008-07-31 01:21 PDT (History)
12 users (show)
sspitzer: blocking1.4b+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
fix, I suck (1.91 KB, patch)
2003-04-29 23:26 PDT, (not reading, please use seth@sspitzer.org instead)
no flags Details | Diff | Review

Description Paul Hardacre 2003-04-28 01:29:59 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030427
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030427

When you turn on "Do not load remote images in Mail & Newsgroup messages" in the
Iamges section of Privacy & Security preferences, attached images no longer
display inline.

Reproducible: Always

Steps to Reproduce:
1. Turn on "Do not load remote images in Mail & Newsgroup messages" in preferences.
2. View a message with attached images.

Actual Results:  
Attached images did not display although the separator HRs where displayed and
white space was left where the image should be.

Expected Results:  
Show the attached images.
Comment 1 Hermann Schwab 2003-04-28 02:25:21 PDT
I do not expect Mozilla to act against the preference I´ve set:
"Do not load remote images in Mail & Newsgroup messages"

In what cases should it respect this pref, and when not?
Comment 2 Paul Hardacre 2003-04-28 02:46:44 PDT
Maybe I was unclear. Up until recently (I don't recall noticing it with the
20030423 build) attached images displayed fine inline and the option applied to
truly remote images such as those dragged in from remote web servers in HTML
messages. In this build it seems to apply to all images, including attachments
and inline images within messages.
Comment 3 Stefan Borggraefe 2003-04-28 07:29:44 PDT
This is a very recent regression. This works fine in build 2003042504 and is
broken in build 2003042604 (tested on Windows XP).

I´m CCing some developers who commited Mail/News changes in this period of time.

Maybe this should block 1.4b since this a very commonly used feature and the fix
should be easy to find because of the small amount of code in question.
Comment 4 (not reading, please use seth@sspitzer.org instead) 2003-04-29 14:56:59 PDT
taking.

if this is a recent regression, it might be from my checkin for
http://bugzilla.mozilla.org/show_bug.cgi?id=51631

I'll investigate.
Comment 5 (not reading, please use seth@sspitzer.org instead) 2003-04-29 23:12:19 PDT
whoa, this pref also affects inserting messages into message compose!
Comment 6 (not reading, please use seth@sspitzer.org instead) 2003-04-29 23:16:45 PDT
whoa, this is so me from bug #51631

on it.
Comment 7 (not reading, please use seth@sspitzer.org instead) 2003-04-29 23:26:34 PDT
Created attachment 122078 [details] [diff] [review]
fix, I suck
Comment 8 (not reading, please use seth@sspitzer.org instead) 2003-04-29 23:28:46 PDT
fixed.

has r/sr=bienvenu, and makes sense, we you look at what I changed.  (d'oh!)
Comment 9 Michiel van Leeuwen (email: mvl+moz@) 2003-04-30 02:36:50 PDT
What did you change in bug 51631? (I can not access it)
Because you changed from a list of protocols not to chech to a short list of
protocols that need to be checked. I don't know if that is a good approach. New
protocols can show up, or old protocols used to show images.
Comment 10 dwitte@gmail.com 2003-04-30 03:40:56 PDT
Seth, can you please elaborate on your patch here?

As I understand it, you didn't check with module owner/peers (darin, myself, and
mvl) before checking this in, and it sounds like mvl has some reservations about it.

If there's security-sensitive stuff here, then we'd appreciate it if you could
try to catch one of us on irc so we can go over it.
Comment 11 dwitte@gmail.com 2003-04-30 03:42:59 PDT
reopening; hopefully this will generate bugmail to seth...

Seth: please see previous comment.
Comment 12 Michiel van Leeuwen (email: mvl+moz@) 2003-04-30 08:56:45 PDT
From looking at bonsai I learned that originally we only checked a few protocols
anyway. But I liked the other approach. Is there some protocol for inline images
in mail messages that can be checked?
Comment 13 Boris Zbarsky [:bz] 2003-04-30 09:09:25 PDT
From a security standpoint, this should absolutely be a whitelist of protocols
we can load images from.  As the code stands, it misses gopher, for example. 
And any time an extension that allows a new protocol is installed it will be
opening a mail security hole.
Comment 14 (not reading, please use seth@sspitzer.org instead) 2003-04-30 10:22:00 PDT
the old code did this:

if !http:// and !https://, return early.  don't bother to check if we are
blocking "remote" images in a mail message.  the reason (I mistakenly thought)
was to optimize and avoid having to do all this checking for chrome:// and
resource://.

so I switched it, in my other patch, to just check for those, and if chrome or
resource, bail, otherwise proceed, to make this check:

if ((a mail message) && (ftp || mBlockRemote)) then block.  (in a mail message,
I never allow <img src="ftp://">,)

but this broke inline (mailbox://, imap://, etc) images, as well as images in
html msg compose and editor.

so, I reverted to:

if !http && !https && !ftp
  return early.
else
  if ((a mail message) && (mBlock || ftp))
    block

to respond to bz:

"As the code stands, it misses gopher, for example.  And any time an extension
that allows a new protocol is installed it will be opening a mail security hole."

not a security issue, but a bug with "block remote images".

so the current code will allow <img src="gopher://"> images to be shown,
assuming gopher doesn't require a username/password.  (bug #51631? will deny any
thing requiring a password prompt)

I guess alternatively, we could do this:

// whitelist
if (chrome, resource, file (for editor), mail related)
  return early;

// everything else, black list
else (http, https, ftp, gopher, etc)
  if ((a mail message) && (mBlock || ftp))
    block

dwitte/bz/darin/et al, what do you think?

this regression (about blocking inline images, horking msg compose, etc), is fixed.

but I'll spin up a new one about the gopher issue, and the change to whitelist /
black list to improve blocking remote images.

Comment 15 (not reading, please use seth@sspitzer.org instead) 2003-04-30 10:32:44 PDT
the issue about catching more protocols is bug #203940

we can take the discussion there, too.
Comment 16 dwitte@gmail.com 2003-04-30 21:35:15 PDT
Great, thanks for clarifying Seth... and thanks for opening the new bug, btw. ;)
Comment 17 Boris Zbarsky [:bz] 2003-05-01 21:51:37 PDT
*** Bug 204125 has been marked as a duplicate of this bug. ***
Comment 18 (not reading, please use seth@sspitzer.org instead) 2003-05-22 14:37:04 PDT
nbaca points out:
"With the pref selected:  I went to a website and drag-n-dropped a graphic file
into a compose window and the image does not appear. Sent the message to myself
and a test account. In both accounts I can view the image whether the pref is
selected or not. Why wouldn't I want to see the image when composing the message?"

she's right, that's a bug.  

what I fixed, when I check in that last patch, was a similar type of problem,
but when you'd try to insert local (file://) images

I had accidentally busted that.


but, I forgot about dnd from web pages
that's http (not ftp) and we should not be blocking that for compose, *ever*,
like you point out.

this pref is supposed to only affect message display
when loading a message, we check the pref
and if we aren't supposed to load remote images
(currently, if the image has the scheme of http:// or https://) we block it.

but again, that should only be in message display, not compose


note, even if we are allowing remote images.
we will never allow ftp:// images


the reason is a privacy issue
if I'm a spammer, I could do
<img src="ftp://abc/foo.gif">
and you could have set it so we send your email address
as your password, for anonyous ftp access!
(see Edit | Prefs | Advanced | Send this email address as anonyous ftp password)

I'll look into the dnd from http compose issue.
Comment 19 Ninoschka Baca 2003-05-22 14:55:05 PDT
Trunk build 2003-05-20: WinMe, Mac 10.1.5
Verified Fixed so local file images now appear.

Logged bug# 205009 to track the problem during a dnd of images from a web page
when composing a new message. 
Comment 20 Ninoschka Baca 2003-05-22 14:57:02 PDT
Oops, not bug# 205009, use bug# 206793 for the dnd of images from a web page
when composing a new message problem.

Note You need to log in before you can comment on or make changes to this bug.