88550 - javascript strict warnings in password.js

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P2)

Description

[Henrik Gemal]

Warning: reference to undefined property window.arguments
Source File: chrome://pippki/content/password.js
Line: 38

Please add the following line to your prefs.js file, so we could avoid all the 
strict warning fixup...:
user_pref("javascript.options.strict", true);

Comment 1

[Stephane Saux]

p2
t->2.1
v->2.0

Comment 2

[(Don't use, old account) - Kai Engert]

I assume the the author wanted to check whether object "window" has a property
named "arguments". I'm not a JavaScript expert, and couldn't find a
documentation on how to do it, but in many places spread all over the Mozilla
JavaScript sources, the
    if ("property" in object)
notation is used, and changing the code this way makes the warning message go
away. Therefore I'm suggesting the attached patch to fix the problem.

Comment 3

[(Don't use, old account) - Kai Engert]

Attachment: This patch might be correct, but I'm not sure.

Comment 4

[(Don't use, old account) - Kai Engert]

David, please review.

Comment 5

[David P. Drinan]

r=ddrinan.

Comment 6

[Christopher Blizzard (:blizzard)]

I'm not sure that one is correct.  Isn't that just supposed to be an empty array?

Comment 7

[(Don't use, old account) - Kai Engert]

My understanding is, it is not guaranteed that "window" will have a property
"arguments". It could have one, but only if previous code during runtime added
this property to the current window instance.

I searched the existing sources and found some other occurences of similar code:

./xpfe/browser/resources/content/fastnav.js:  if ("arguments" in window &&
window.arguments.length > 1 && window.arguments[1]) {
./xpfe/browser/resources/content/fastnav.js:      if ("arguments" in window &&
window.arguments.length >= 1 && window.arguments[0])
./xpfe/browser/resources/content/fastnav.js:    if ("arguments" in window &&
window.arguments.length >= 3 && window.arguments[2] ==
true)./xpfe/browser/resources/content/navigator.js:  if ("arguments" in window
&& window.arguments.length > 1 && window.arguments[1]) {
./xpfe/browser/resources/content/navigator.js:    if ("arguments" in window &&
window.arguments.length >= 1 && window.arguments[0])
./xpfe/browser/resources/content/navigator.js:    if ("arguments" in window &&
window.arguments.length >= 3 && window.arguments[2] == true)
./xpfe/browser/resources/content/pageInfo.js:  if ("arguments" in window) {
./xpfe/browser/resources/content/viewsource.js:    if ("arguments" in window &&
window.arguments.length >= 2) {
./xpfe/components/bookmarks/resources/addBookmark.js:  if ("arguments" in
window) {
./xpfe/components/bookmarks/resources/bookmarks.js:  if ("arguments" in window
&& window.arguments[0]) {
./xpfe/components/history/resources/history.js:    if ("arguments" in window &&
window.arguments[0] && window.arguments.length >= 1) {
./xpfe/components/prefwindow/resources/content/pref-applications-new.js:  if (
"arguments" in window && window.arguments[0] ) {
./xpfe/components/prefwindow/resources/content/pref-applications-new.js:  if (
"arguments" in window && window.arguments[0] ) {
./xpfe/components/prefwindow/resources/content/pref-charset.js:  if ("arguments"
in window && window.arguments[0])
./xpfe/components/prefwindow/resources/content/pref-languages.js:  if
(!("arguments" in window)) {
./mailnews/addrbook/resources/content/abAddressBookNameDialog.js:	if
("arguments" in window && window.arguments[0])
./mailnews/addrbook/resources/content/abCardOverlay.js:  if ("arguments" in
window && window.arguments[0])
./mailnews/base/resources/content/msgMail3PaneWindow.js:  if ("arguments" in
window && window.arguments[0])
./mailnews/base/search/resources/content/FilterEditor.js:    if ("arguments" in
window && window.arguments[0]) {
./mailnews/news/resources/content/downloadheaders.js:    if ("arguments" in
window && window.arguments[0]) {

This looks like we could deduce this code is correct. It is used to execute code
accesing the property "arguments" only if the above code succeeds.

Do you agree, or do you suggest we should find a JavaScript expert who could
tell whether the code is correct? Do you know someone we could ask?

Comment 8

[Christopher Blizzard (:blizzard)]

No, that's fine.  sr=blizzard

Comment 9

[Henrik Gemal]

who's up for a checkin?

Comment 10

[(Don't use, old account) - Kai Engert]

-> Kai

Comment 11

[Stephane Saux]

adding nsenterprise to all P1, P2 PSM bugs with target milestone of 2.1

Comment 12

[Henrik Gemal]

I'll say again: just needs checkin...

Comment 13

[John Unruh]

Mass assigning QA to ckritzer.

Comment 14

[Javier Delgadillo]

Checked in patch for Kai.

Comment 15

[Stephane Saux]

keywords->verifyme

Comment 16

[John Unruh]

Verified.