243170 - Page can open JavaScript console even when popups are disabled. (Remove the feature that "javascript:" opens the console.)

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Met - Martin Hassman]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7) Gecko/20040501
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7) Gecko/20040501

The <img src="javascript:"> in the HTML page opens JavaScript console during
onload. This is wrong behaviour, isn't? It should not be able from the HTML
page. And it is not affected by popup blocker. (Yes, if you turn off JavaScript,
it doesn't happened.)

And what more - if repeated, it can open more instances of the JS-console and so
block Mozilla and consume computer resources. This is strange, because only one
instance of JavaScript console can be opener in common usage.

Indefinitive loop of opening new and new and new and new JavaScript console can
practically completely block Win98 system (you have to press CTRL+ALT+DELETE and
kill the browser) or significantly slow Win2000.

If you have opened some JavaScript console before loading of the testcase, new
instances are not created, only the console get focus.

Same problem is, if you use <iframe scr="javascript:"></iframe> or <link
rel="stylesheet" href="javascript:" /> and maybe in some other URL too.

Observed in: Mozilla1.7b, Firefox 0.8 and Mozilla/5.0 (Windows; U; Windows NT
5.0; cs-CZ; rv:1.7) Gecko/20040501

Reproducible: Always
Steps to Reproduce:

Comment 1

[Met - Martin Hassman]

Attachment: Img testcase

Testcase

Comment 2

[Met - Martin Hassman]

Attachment: Link testcase

Other testcase

Comment 3

[Met - Martin Hassman]

Attachment: Iframe testcase

Other testcase

Comment 4

[Matthew Imakyure]

I see this with 
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040509
Firefox/0.8.0+ on Windows 2000

Image test case opens 1 instance of js console
Link test case opens 2
IFrame opens 10 (as many as specified)

This seems somewhat bad...(critical?)

Comment 5

[Anders Pedersen]

Confirmed.

- Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040510
Firefox/0.8.0+
- Microsoft Windows 2000 Pro 5.00.2195 SP4

Comment 6

[Dan M]

Yes, it's real enough. So what's the right solution, then? I believe the naked
"javascript:" URL is intended as a shortcut for opening the Console. Should we
somehow disallow it from anything but the URLbar? Should we allow a page to open
only one copy of the window? Should we treat it like a popup window?

Its obnoxiousness is limited. All a website can do with this hack is open the
JavaScript Console. Many times. And by the way it opens full-screen on my build.
That's kind of weird and extra annoying.

Comment 7

[Brendan Eich [:brendan]]

Naked javascript: could be made to open the JS console only if entered from the
location toolbar.  Netscape 4.x didn't worry about the potential for abuse here,
though.  This bug reports no real-world abuse, so I'm wondering why we shouldn't
WONTFIX it.

Danm, can you worry about the other glitches you noticed in separate bug(s)?

/be

Comment 8

[Met - Martin Hassman]

Attachment: Hang - tescace

(In reply to comment #7)
> This bug reports no real-world abuse, so I'm wondering why we shouldn't
> WONTFIX it.

And what about freezing the browser? See attachment with indefinitive loop of
opening "javascript:". Can anyone stop it and not kill the browser process?

Satisfactory solution should be do not allow opening more instances of the
JS-console. So the page can't hang the browser.

Comment 9

[Matthew Imakyure]

(In reply to comment #7)
> Naked javascript: could be made to open the JS console only if entered from the
> location toolbar.  Netscape 4.x didn't worry about the potential for abuse here,
> though.  This bug reports no real-world abuse, so I'm wondering why we shouldn't
> WONTFIX it.

This also seems to bypasses the built in script warning / alert window
"A script on this page is causing mozilla to run slowly.  If it continues to run
your computer may become unresponsive.  Do you want to abort the script?
[ok/cancel]"
Which gives the user the option to cancel the script and seems designed to put a
check on this type of behavior.  The behavior in this case is inconsistent (no
warning is displayed).

My own opinion would be to get rid of the "javascript:" shortcut all together as
it seems somewhat inconsistent/unnecessary, but that's probably doing a bit too
much.  Restricting it to the location toolbar seems the next best thing (any
reason why a web page should need to launch the JS Console?)

Comment 10

[Brendan Eich [:brendan]]

I don't see why we need to support a web page opening the JS console.  There is
no web standard, _de facto_ or _de jure_, among browsers other than those in the
Mozilla family.

D.O.S. attacks are plentiful, but if we can knock this one down, great.

/be

Comment 11

[Met - Martin Hassman]

Seems tat this not affect only javascript: but also some other protocols. I have
created a separate bug #243503

Comment 12

[Peter van der Woude [:Peter6]]

*** Bug 263635 has been marked as a duplicate of this bug. ***

Comment 13

[Peter van der Woude [:Peter6]]

*** Bug 265170 has been marked as a duplicate of this bug. ***

Comment 14

[Brett Johnson]

The Iframe testcase does it to my browser aswell, quite annoying really - and no
visable option to disable it happening

Comment 15

[Pavel Penaz]

This bug could trigger some bad press from people who test Firefox against the
mangle.cgi script. JS console opens automatically rather often and every few
minutes Firefox hangs because of the JS console..

Comment 16

[HARUNAGA Hirotoshi]

-> all/all

Comment 17

[Dan M]

*** Bug 284453 has been marked as a duplicate of this bug. ***

Comment 18

[Pavel Penaz]

Sorry for asking but should I file a new bug for Firefox? This bug appears to be
for Mozilla only.

Comment 19

[Jonathan Haas]

Would be nice, if there would be a Firefox-specific-bug.

Can somebody change the Product or should a seperate bug be filed?

(Why seems this rather critical bug to be forgotten?)

Comment 20

[Brian Polidoro]

*** Bug 351417 has been marked as a duplicate of this bug. ***

Comment 21

[Andrew Schultz]

> Should we somehow disallow it from anything but the URLbar?

This seems the most reasonable thing to do.  This would mean that nsJSThunk::BringUpConsole would go away and the browser would be responsible for intercepting the URL before trying to "load" it.

> Should we allow a page to open only one copy of the window?

The code already attempts to do this, but apparently fails.
http://lxr.mozilla.org/seamonkey/source/dom/src/jsurl/nsJSProtocolHandler.cpp#294

Comment 22

[Andrew Schultz]

Attachment: separate console from protocol

embedding/components/jsconsole/ could probably get removed as well if that's desired.
http://lxr.mozilla.org/seamonkey/search?string=jsconsole-service

If the approach is OK, I'll get FF/SM folks to review the chrome changes.

Comment 23

[Brendan Eich [:brendan]]

Comment on attachment 237079 [details] [diff] [review]
separate console from protocol

It looks good to me (less C++, more JS).  I'd be grateful if bz approved the jsurl changes.  Seeking mconnor blessing as you proposed (unless there's a better FE reviewer?).

/be

Comment 24

[[e-mail]]

(In reply to comment #21)
>>Should we allow a page to open only one copy of the window?
>The code already attempts to do this, but apparently fails.
>http://lxr.mozilla.org/seamonkey/source/dom/src/jsurl/nsJSProtocolHandler.cpp#294
Unfortunately windows get asynchronously added to the window mediator.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 237079 [details] [diff] [review]
separate console from protocol

> embedding/components/jsconsole/ could probably get removed

Yes, please.  It's pretty useless for embedding as it stands (only usable in XUL apps and doesn't really do uniqueness checking).  I think this only existed so embeddors could override with their own console for purposes of this code, but if we're now completely pushing this off to the embedding layer, that's not needed.

>Index: dom/src/jsurl/nsJSProtocolHandler.cpp
>     // If mURI is just "javascript:", we bring up the Error console
>     // and return NS_ERROR_DOM_RETVAL_UNDEFINED.

Fix this comment?

>     if (script.IsEmpty()) {

Do we really need this check?  That is, won't we just get undefined from the JS engine if we run an empty string through it?  If so, can we just remove this block completely?

>Index: xpfe/browser/resources/content/navigator.js

> function handleURLBarCommand(aUserAction, aTriggeringEvent)
> {
>+  if (gURLBar.value == "javascript:") {
>+    toJavaScriptConsole();

So this doesn't work if I, say, paste "javascript:" with the middle mouse button.  Is that what we want?  I'd almost say "yes", but then again I think this "javascript: opens JS console" thing is stupid.  ;)  But check with the UI folks.

sr=bzbarsky with the nits.

Also asking Neil for review on the XFPE bit.

Comment 26

[Andrew Schultz]

(In reply to comment #25)
> (From update of attachment 237079 [details] [diff] [review] [edit])
> > embedding/components/jsconsole/ could probably get removed
> 
> Yes, please.  It's pretty useless for embedding as it stands (only usable in

OK, I'll attach a patch to do that shortly.

> >     if (script.IsEmpty()) {
> 
> Do we really need this check?  That is, won't we just get undefined from the 

No, apparently not.  I'll remove it.

> So this doesn't work if I, say, paste "javascript:" with the middle mouse
> button.  Is that what we want?  I'd almost say "yes", but then again I think
> this "javascript: opens JS console" thing is stupid.  ;)  But check with the 
> UI folks.

Right.  Middle-click-paste does not open the console, nor do things like the open web location dialog.

Comment 27

[Andrew Schultz]

Attachment: remove embedding/components/jsconsole

Comment 28

[jag (Peter Annema)]

Personally I think we should just drop support for "javascript:" opening the "Error console".

Comment 29

[[e-mail]]

Comment on attachment 237079 [details] [diff] [review]
separate console from protocol

r- on navigator.js as per comment #28.

Comment 30

[Andrew Schultz]

Comment on attachment 237079 [details] [diff] [review]
separate console from protocol

looking for r= for the browser/ changes (or the go-ahead to drop support for "javascript:")

Comment 31

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 237079 [details] [diff] [review]
separate console from protocol

Please just drop the "javascript: opens the Error Console" feature from Firefox (don't make the /browser change).

Comment 32

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 237079 [details] [diff] [review]
separate console from protocol

Well, I guess that means this is r- and not r+, but you get the idea!

Comment 33

[Andrew Schultz]

Attachment: fix up allmakefiles.sh too

The previous 2 patches are in.

I forgot about allmakefiles.sh.  I removed jsconsole from the long lines with 

:%s#embedding/components/jsconsole/src/Makefile ##
:%s#embedding/components/jsconsole/public/Makefile ##

Comment 34

[Benjamin Smedberg]

Comment on attachment 252135 [details]
fix up allmakefiles.sh too

Don't bother, MOZ_BUILD_PROJECTS is dead.

Comment 36

[Andrew Schultz]

Hmm... this is fixed.  allmakefiles.sh got cleaned up elsewhere

Comment 37

[Michelle]

Is it a patch that needs to be uploaded or does he need to update his program?  My client still gets this problem.  If I take out href="javascript:;"---that's removing the java script all together????????

Comment 38

[Ryan VanderMeulen [:RyanVM]]

Also, should the hang testcase still hang the browser? Because it sure does for me on the trunk :-)

Comment 40

[Anji Maram]

How can I see Javascript errors in Firefox 3.0.2 if javascript: popup console feature has been dropped?
Is there nay other way to see the errors.

It was very useful to debug javascript errors.

Comment 41

[Jesse Ruderman]

Anji, you can still open the Error Console from Firefox's Tools menu.

Comment 42

[Anji Maram]

Thanks.
Got it.