317383 - CheckLoadURI doesn't throw an exception when setting img.src from script

Status: VERIFIED INVALID

(Core :: Security, defect)

Description

[Alexey Znayev]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru-RU; rv:1.7.10) Gecko/20050907 ASPLinux/1.7.10-1.3.1asp
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ru-RU; rv:1.7.12) Gecko/20050920 Firefox/1.0.7

Setting CheckLoadURI to true doesn't generate an exception when I'm trying to load local image as Image.src. So impossible to handle CheckLoadURI setting and inform user about it.


Reproducible: Always

Steps to Reproduce:
1. Write a script with code:
SomeFileSrc = '/home/user/pictures/bla-bla-bla.jpg';
LocalImg = new Image();
try{
 LocalImg.src = 'file://' + SomeFileSrc;
}catch(e){
 alert('Got it');
}

2. Run this script from page.

Actual Results:  
LocalImg.complete is set to true immediately.
LocalImg.src is equal to string 'file://' + SomeFileSrc.
No alerts appear at all, just scurity notification message in JavaScript console.


Expected Results:  
Browser should generate an exception, so alert should appear.


The same result is under Windows and in the Mozilla 1.7.* under Linux and Windows.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I don't think we want to throw here (since this _does_ change the "src" attribute and all)...

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yeah, I don't really think we want to throw either. Setting .src is the same as calling setAttribute, and that should clearly not throw.

Comment 3

[Alexey Znayev]

(In reply to comment #2)
> Yeah, I don't really think we want to throw either. Setting .src is the same as
> calling setAttribute, and that should clearly not throw.

Well, but _actually_ result is not achieved.
I cannot operate with LocalImg height and width, for example, and I'm not informed about it.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

While I agree, it is a tricky situation. I guess we could throw just for SetSrc, but not for SetAttribute, i'm not exited about the two functions behaving differently.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Actually, what we should do is that we should fire an error event. Or do we already do that?

Comment 6

[Boris Zbarsky [:bzbarsky]]

We fire an error event in this case, yes.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Then that should be enough IMO. We are successful in setting the attribute and so should not throw. If you're interested in if we succeed to load or not, then attach event listeners.

Comment 8

[Alexey Znayev]

(In reply to comment #7)
> Then that should be enough IMO. We are successful in setting the attribute and
> so should not throw. If you're interested in if we succeed to load or not, then
> attach event listeners.

Sure, I did this. But it's not so trivial sutuation when neither success listener  nor fault listener is called. So it's not easy to inform user about impossibility to load local picture.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> But it's not so trivial sutuation when neither success listener  nor fault
> listener is called.

What build are you testing?  Note bug 269125 was fixed on the Gecko 1.9 trunk after 1.8 branched off, so onerror only fires in trunk builds in this case.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The error listener should be called, no?

Comment 11

[Alexey Znayev]

(In reply to comment #9)
> What build are you testing?

For example 
Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.8) Gecko/20051111 Firefox/1.5

(In reply to comment #10)
> The error listener should be called, no?

No. Absolutely.
I set next listeners:
----------
LocalImg = new Image();
LocalImg.onerror = LocalImgOnError;
LocalImg.onload = LocalImgOnLoad;
----------
And neither LocalImgOnError nor LocalImgOnLoad are not called.

To catch this situation I need to do something like this:
----------
oldImgSrc = LocalImg.src;
newImgSrc = 'file://' + SomeFileSrc;
try{
 LocalImg.src = newImgSrc;
 if(!LocalImg.complete || (LocalImg.src == oldImgSrc))
  throw new Error('Not loaded ' + newImgSrc);
}catch(e){
 ...
}
----------

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

So that problem is fixed so that LocalImgOnError is called. However it was fixed too late to make FF 1.5.

We should consider porting bug 269125 to the 1.8.1 branch so that it gets fixed in FF 2.

Comment 13

[Alexey Znayev]

(In reply to comment #12)
> So that problem is fixed so that LocalImgOnError is called. However it was
> fixed too late to make FF 1.5.
> 
> We should consider porting bug 269125 to the 1.8.1 branch so that it gets fixed
> in FF 2.

Well! Thanks a lot.