Closed Bug 63117 Opened 24 years ago Closed 21 years ago

Numbers in prefs accepted in octal when started with 0 and hexadecimal when started with 0x

Categories

(SeaMonkey :: Preferences, defect, P3)

x86
All
defect

Tracking

(Not tracked)

RESOLVED FIXED
Future

People

(Reporter: matxdr, Assigned: eddyk)

References

Details

Attachments

(1 file, 2 obsolete files)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; m18) Gecko/20001216
BuildID:    2000121620

If the number you enter for the cache size begins with zero, it will change
itself to another value.  010 gives 8, 020 give 16, etc..

Reproducible: Always
Steps to Reproduce:
Go to pref, advanced, cache
Set either the disk or memory cache to 010
Click ok


Actual Results:  Go back to the prefs, the cache size is now 8


Expected Results:  Go back to the prefs, the cache size is still 10
seeing on a cvs linux build from today, over to preferences
Assignee: asa → matt
Status: UNCONFIRMED → NEW
Component: Browser-General → Preferences
Ever confirmed: true
OS: Windows ME → All
QA Contact: doronr → sairuh
Numbers entered with a leading 0 are intepreted as octal numbers.
Furthermore, numbers entered with a leading 0x are interpeted as hexadecimal
numbers (0x50 becomes 80).

updating summary.
Summary: Cache size changes when the number begins by 0 → Cache sizes interpreted as octal when started with 0 and hexadecimal when started with 0x
That is more of a feature than a bug, even though octal numbers can be 
confusing. I guess that it is the same for every text field interpreted as a 
number from JavaScript.

should this go to networking: cache...?
Assignee: matt → neeti
Component: Preferences → Networking: Cache
QA Contact: sairuh → gordon
Assignee: neeti → gordon
Target Milestone: --- → mozilla1.0
cache bugs to gordon
This works as designed.  Marking bug INVALID.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → INVALID
Gordon: you really _designed_ the cache pref to be interpreted as octal when it
begins with a leading 0?  If so, I think your design is flawed.

I'm reopening this, because I think it's confusing to users without any real
value added in return for that confusion, and I can't find another browser that
behaves this way.  If you don't want to fix it, please reassign it to me.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
The fact that this field lets you enter the `x' character at all is bug 66163.
I didn't design it.  It seems an intentional decision in the way js prefs are 
read.  However, if it is desired to have additional verification of user input, 
that's fine.

Matt, I imaging we're already doing these kind of checks elsewhere.  Could point 
me at a good example, or would you like to take this bug?  It probably should be 
a pref panel bug; it doesn't have much to do with the networking/cache code.  
What do you think?

Chris, this seems to be a general phenomenon with preferences.
Assignee: gordon → mcafee
Status: REOPENED → NEW
Component: Networking: Cache → Preferences: Backend
over to bnesse for a look.
Assignee: mcafee → bnesse
is gordon going to qa this?
I don't feel a strong need to.  Feel free to take the QA Contact.
qa to me. 
QA Contact: gordon → benc
Priority: -- → P3
An example of this bug can be found at:

http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/
content/pref-cache.xul
The backend is storing the value it receives (in this case 8) which is why it 
shows up as 8 when it gets reloaded from the preference. The value has already 
been converted to octal before being passed to the backend preferences.

Passing the buck to frontend preferences for further triage :)
Assignee: bnesse → eddyk
Component: Preferences: Backend → Preferences
I've found a call in nsPrefWindow.js:225 where we call parseInt(value) where 
value is the content of the textbox.  If I change the value to 
parseInt(value,10) we'll always interpret in base ten.  Is this what we really 
want in *all* cases?

If so, I'll need r and sr to check this in.
Attached patch fix (obsolete) — Splinter Review
I think this is what we want in all cases... if someone at some point needs a
different base, we can extend the prefwindow code to support that.

in any case, what happens if I enter "alskdjf" instead of a number? does the
pref get reset to 0? does an exception get thrown? We should try to catch the
error and not reset the pref, if at all possible.
I typed some random ascii garbage into the cache textbox and saved it.  Upon 
re-opening, it was set to 0.  I'll see what can be done to ignore garbage and 
reload the previous value.
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
*** Bug 50616 has been marked as a duplicate of this bug. ***
I feel it important to emphasize, that this bug affects not only cache sizes,
but all preference fields that expect a numeric value; perhaps the summary of
this bug should be changed to reflect that.  According to comment #18 there was
a fix in May 2001 but it never got reviewed and/or checked in(?)
->ben/moz1.0/critical, this is data loss folks, but an unlikely scenario so P3.
Assignee: eddyk → ben
Severity: minor → critical
Target Milestone: mozilla1.0.1 → mozilla1.0
nsbeta1- per Nav triage team
Keywords: nsbeta1nsbeta1-
Target Milestone: mozilla1.0 → Future
Changed summary to reflect real problem.

This has been reported to affect cache and proxy users.

In fact, one user reported they got the wrong proxy because of this.

+nsbeta1 again.
Keywords: nsbeta1-nsbeta1
Summary: Cache sizes interpreted as octal when started with 0 and hexadecimal when started with 0x → Numbers in prefs accepted in octal when started with 0 and hexadecimal when started with 0x
*** Bug 141898 has been marked as a duplicate of this bug. ***
Re: comment 19 - why even let the user type a non-numeric key in a field where 
only a numeric value is expected; NC 4.8 doesn't allow it, neither should 
Mozilla.  It would seem to me that the fix in comment 18 in conjunction with 
keystroke-checking in the appropriate preference fields would allow this bug to 
be resolved...
I agree.  Support for octal and hex is absurd here, and only makes support for
decimal seem broken.
Keywords: 4xp
Nav triage team: nsbeta1-
Keywords: nsbeta1nsbeta1-
Attached patch patch with validity check (obsolete) — Splinter Review
This patch is the one by eddyk extended by a check if the value string contains
any non-digit/non-whitespace chars. In this case it re-sets the value string to
the old/current pref value to avoid changing the pref.

This is pretty restrictive: any letter in the field will prevent changing the
pref. I included whitespace in the valid chars because a leading/trailing space
should not make changing the pref fail. Whitespace inmidst the string will end
parsing of the int (as it always did), saving " 6 7 " as "6".
However, the simplest solutions mostly are the best ones: this is the first
patch by eddyk, updated to current trunk.
Comment #19 and #20 suggest a problem when entering nonsensical values (i.e.:
no numbers). However, this is identical to the current behaviour:
"3e4" -> "3"
"xyz" -> "0"
etc. So I think this patch does clearly improve the situation without making
anything worse.
Comment on attachment 140438 [details] [diff] [review]
first fix updated

...therefore requesting r= from Neil for the updated version of eddyk's patch.
Attachment #140438 - Flags: review?(neil.parkwaycc.co.uk)
Another error that's a previously not found part of this bug: 08 and 09 are
parsed as octal numbers - where 8 and 9 are invalid digits. So entering those
will result in setting the pref to 0 (as will entering 08346 etc.).
Attachment #140438 - Flags: review?(neil.parkwaycc.co.uk) → review+
Comment on attachment 140438 [details] [diff] [review]
first fix updated

Requesting sr= from alecf.
Attachment #140438 - Flags: superreview?(alecf)
Attachment #140437 - Attachment is obsolete: true
Attachment #35169 - Attachment is obsolete: true
Assignee: bugs → eddyk
Comment on attachment 140438 [details] [diff] [review]
first fix updated

sr=alecf
Attachment #140438 - Flags: superreview?(alecf) → superreview+
last attachment checked in.
Checking in xpfe/components/prefwindow/resources/content/nsPrefWindow.js;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/nsPrefWindow.js,v
 <--  nsPrefWindow.js
new revision: 1.46; previous revision: 1.45
done
Status: NEW → RESOLVED
Closed: 23 years ago21 years ago
Resolution: --- → FIXED
This checkin may have caused bug 233167 
Blocks: 233167
Second patch was checked in instead of the third one. Biesi will correct this
later today. Reopening to not forget it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
wrong patch backed out, right patch checked in. marking fixed again.

sorry for the screwup :(
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: