Closed
Bug 69744
Opened 24 years ago
Closed 23 years ago
Enter a long choice in *No proxy for* and lose your preference!
Categories
(Core :: Preferences: Backend, defect, P1)
Core
Preferences: Backend
Tracking
()
RESOLVED
FIXED
People
(Reporter: francis, Assigned: bnesse)
References
()
Details
(Keywords: dataloss, testcase)
Attachments
(3 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; 0.8) Gecko/20010221
BuildID: 2001022105
If your paste a long line of choices in No proxy for, you will lose your prefs
when restarting mozilla.
Reproducible: Always
Steps to Reproduce:
1.Have mozilla properly configured
2.Go to EDIT -> PREFERENCE -> ADVANCE -> PROXIES
3.Choose MANUAL PROXY CONFIGURATION
4.Enter this line : 207-87-18-203.wsmg.digex.net Garden.ngadcenter.net
Ogilvy.ngadcenter.net ResponseMedia-ad.flycast.com Suissa-ad.flycast.com
UGO.eu-adcenter.net VNU.eu-adcenter.net a32.g.a.yimg.com ad-adex3.flycast.com
ad.adsmart.net ad.ca.doubleclick.net ad.de.doubleclick.net ad.doubleclick.net
ad.fr.doubleclick.net ad.jp.doubleclick.net ad.linkexchange.com
ad.linksynergy.com ad.nl.doubleclick.net ad.no.doubleclick.net
ad.preferences.com ad.sma.punto.net ad.uk.doubleclick.net ad.webprovider.com
ad08.focalink.com adcontroller.unicast.com adcreatives.imaginemedia.com
adex3.flycast.com adforce.ads.imgis.com adforce.imgis.com adfu.blockstackers.com
adimage.blm.net adimages.earthweb.com adimg.egroups.com admedia.xoom.com
adpick.switchboard.com adremote.pathfinder.com ads.admaximize.com ads.bfast.com
ads.clickhouse.com ads.enliven.com ads.fairfax.com.au ads.fool.com
ads.freshmeat.net ads.hollywood.com ads.i33.com ads.infi.net ads.jwtt3.com
ads.link4ads.com ads.lycos.com ads.madison.com ads.mediaodyssey.com ads.msn.com
ads.ninemsn.com.au ads.seattletimes.com ads.smartclicks.com ads.smartclicks.net
ads.sptimes.com ads.tripod.com ads.web.aol.com ads.x10.com ads.xtra.co.nz
ads.zdnet.com ads01.focalink.com ads02.focalink.com ads03.focalink.com
ads04.focalink.com ads05.focalink.com ads06.focalink.com ads08.focalink.com
ads09.focalink.com ads1.activeagent.at ads10.focalink.com ads11.focalink.com
ads12.focalink.com ads14.focalink.com ads16.focalink.com ads17.focalink.com
ads18.focalink.com ads19.focalink.com ads2.zdnet.com ads20.focalink.com
ads21.focalink.com ads22.focalink.com ads23.focalink.com ads24.focalink.com
ads25.focalink.com ads3.zdnet.com ads3.zdnet.com ads5.gamecity.net
adserv.iafrica.com adserv.quality-channel.de adserver.dbusiness.com
adserver.garden.com adserver.janes.com adserver.merc.com adserver.monster.com
adserver.track-star.com adserver1.ogilvy-interactive.de adtegrity.spinbox.net
antfarm-ad.flycast.com au.ads.link4ads.com banner.media-system.de banner.orb.net
banner.relcom.ru banners.easydns.com banners.looksmart.com
banners.wunderground.com barnesandnoble.bfast.com beseenad.looksmart.com
bizad.nikkeibp.co.jp bn.bfast.com c3.xxxcounter.com califia.imaginemedia.com
cds.mediaplex.com click.avenuea.com click.go2net.com click.linksynergy.com
cookies.cmpnet.com cornflakes.pathfinder.com counter.hitbox.com
crux.songline.com erie.smartage.com etad.telegraph.co.uk fp.valueclick.com
gadgeteer.pdamart.com gm.preferences.com gp.dejanews.com hg1.hitbox.com
image.click2net.com image.eimg.com images2.nytimes.com jobkeys.ngadcenter.net
kansas.valueclick.com leader.linkexchange.com liquidad.narrowcastmedia.com
ln.doubleclick.net m.doubleclick.net macaddictads.snv.futurenet.com
maximumpcads.imaginemedia.com media.preferences.com mercury.rmuk.co.uk
mojofarm.sjc.mediaplex.com nbc.adbureau.net newads.cmpnet.com
ng3.ads.warnerbros.com ngads.smartage.com nsads.hotwired.com
ntbanner.digitalriver.com ph-ad05.focalink.com ph-ad07.focalink.com
ph-ad16.focalink.com ph-ad17.focalink.com ph-ad18.focalink.com rd.yahoo.com
realads.realmedia.com redherring.ngadcenter.net redirect.click2net.com
regio.adlink.de retaildirect.realmedia.com s2.focalink.com
sh4sure-images.adbureau.net spin.spinbox.net static.admaximize.com
stats.superstats.com sview.avenuea.com thinknyc.eu-adcenter.net
tracker.clicktrade.com tsms-ad.tsms.com v0.extreme-dm.com v1.extreme-dm.com
van.ads.link4ads.com view.accendo.com view.avenuea.com w113.hitbox.com
w25.hitbox.com web2.deja.com webads.bizservers.com www.PostMasterBannerNet.com
www.ad-up.com www.admex.com www.alladvantage.com www.burstnet.com
www.commission-junction.com www.eads.com www.freestats.com www.imaginemedia.com
www.netdirect.nl www.oneandonlynetwork.com www.targetshop.com www.teknosurf2.com
www.teknosurf3.com www.valueclick.com www.websitefinancing.com www2.burstnet.com
www4.trix.net www80.valueclick.com z.extreme-dm.com z0.extreme-dm.com
z1.extreme-dm.com
5.Close mozilla and restart
Actual Results: I lose all my prefs (Main web page, mail, etc)
Expected Results: It should not accept the line or give a error message if the
line is too long.
Comment 1•24 years ago
|
||
WORKSFORME
Platform: PC
OS: Windows 98
Mozilla Build: 2001031120
Reporter have you tried downloading the latest nightly from
http://www.mozillazine.org/build_comments/ and creating a new profile? Are you
able to reproduce the problem?
Keywords: dataloss
Reporter | ||
Comment 2•24 years ago
|
||
I'm unable to create a new profile with the 2001031204 version, but with the 8
march, I can reproduce this.
1) Create a new profile
2) Go to preference -> enter a new start homepage
2) Create a new ISP e-mail account (Mail)
3) Go to http://www.ecst.csuchico.edu/~atman/spam/adblock.shtml
4) Copy the line with all the server (Line after <This is the line to paste into
the No Proxy For/Exception box:>)
5) Paste it in the preference -> Advance -> Proxy -> No proxy for
6) Close mozilla
7) Start mozilla
8) Look at the start page and at the mail account.
9) Result = No more start page and no more mail account.
Comment 3•24 years ago
|
||
Yep -- Confirmed in Win2k 2001050204-Trunk: When I copied the line in question
into my proxy exclude list, I not only lost *all* my proxy settings, but *all*
my mail settings as well.
Lets blame Prefs-Backend for now.
Status: UNCONFIRMED → NEW
Component: Networking → Preferences: Backend
Ever confirmed: true
Comment 4•24 years ago
|
||
Reassigning bug to Prefs backend owner
Assignee: neeti → dveditz
QA Contact: tever → sairuh
Comment 6•24 years ago
|
||
Such a long list is probably an abuse of the "no proxy for" feature anyway --
have you looked at other ways you could do this? From the host names this
looks like a workaround for an ad-filter. If you're trying to block ads the
image-blocker would be a more appropriate tool. If you really are concerned
about all access to those hosts the undocumented nsIWebFilters functionality
would work wonders.
http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsFilters.js
put the above file in your "components" directory, edit the blocking list at
the bottom, then either run "regxpcom" to register it or delete components.reg
to force a full re-registration.
Eventually the nsIWebFilters interface might be eliminated when the various
"Content Policy" hooks are combined, but have no fear that the capability
itself will go away; it just might require using a different interface.
Meanwhile, I believe bnesse@netscape.com is working on prefs backend these
days.
Assignee: dveditz → bnesse
Assignee | ||
Comment 8•23 years ago
|
||
It seems that there is a 2K limit on the length of entries in the hash table.
There also appear to be issues with unexpected error codes being returned to
NSPR, error conditions not being appropriately tested for and handled, and the
fact that the pref file has already been overwritten at the point the error
occurs, thus it's too late to save it... this should be fun...
Status: NEW → ASSIGNED
OS: Windows ME → All
Priority: -- → P1
Hardware: PC → All
Assignee | ||
Comment 9•23 years ago
|
||
It appears that in 4.x (where the backend prefs code came from) the FE limited
the number of characters that could be entered into the textedit fields. eddyk
suggests that there is a xul property called "maxlength" which might be used for
this purpose. cc'ing eddy and ben for input.
This could potentially affect any string related preference, but will really only
show up in a case like this where the preference is being (ab)used to hold a list
of data.
So far, it appears that the 2k limit is artificially imposed by the buffer array
in the preferences saving callback. I can fix this by allocating a buffer of the
appropriate size instead. That sounds like a lot of allocation/disposal overhead
to every callback, but it turns out that we are strdup'ing the buffer anyway, so
this will just save a step.
Updated•23 years ago
|
QA Contact: sairuh → benc
Assignee | ||
Comment 10•23 years ago
|
||
Comment 11•23 years ago
|
||
Instead of tedious string length calculating and explicit PR_Malloc-ing space,
why not just replace PR_snprintf() with PR_smprintf()?
if (tmp_str) {
prefEntry = PR_smprintf(FORMAT, (char*)he->key, tmp_str);
}
You should consider using PR_smprintf() for the INT and BOOL cases, too, just
in case someone in the future thinks a 2K pref name is a good idea and blows
up. Could be done by a malicious component trying to zap a user's prefs,
or even malicious *content* if it could figure out an action that indirectly
caused a pref to get set.
Not sure why you changed the last two else if()s to an else{} with an internal
if. The "When in Rome" philosophy argues against gratuitous style changes. But
change the code to PR_smprintf() in those blocks.
Comment 12•23 years ago
|
||
I'd rather see someone put a limit on the size of these entries, and have a good
error message.
I think this enormous "no proxy" list is mindless, but I would like to hear
reasons why people think this is good.
Assignee | ||
Comment 13•23 years ago
|
||
Dan:
I didn't use PR_smprintf() because of the implication that one must call
PR_smprintf_free() to release the memory. But I see that it simply calls
PR_Free() so I suppose there's no harm there. The reason for moving into the
seperate else case was because I could no longer do the prefArray[i] =
PL_strdup(buf); for all cases. I figured that rather than adding it to both else
if cases, I'd make an enclosing else that I could move it into.
Good point on the safety factor. I chose not to do it because it seemed like
overkill for the need. Using PR_smprintf() in all cases does make sense to avoid
intentional damage.
Benjamin:
I agree that the input should be limited, but I also believe that this is a good
fix. There are issues here that can't be resolved by FE limitations such as
setting a really long preference from JS. This would be a really good way to
trash someones preference file if you knew of the limitation and were malicious
enough.
Comment 14•23 years ago
|
||
Thanks for clearing up the prefArray[i] = thing. I misread that part of the
patch and didn't notice the PR_strdup() was still being used. Or maybe I
re-wrote the code in my head to use PR_smprintf() and eliminated that line :-)
Comment 15•23 years ago
|
||
why not use PR_smprintf? seems like it would clean up the code a bit
Assignee | ||
Comment 16•23 years ago
|
||
Assignee | ||
Comment 17•23 years ago
|
||
New improved patch attached. Please review & super review.
Comment 18•23 years ago
|
||
r=dveditz
remember that a super-review request needs to be directed "to" a particular
reviewer and ""cc"" reviewers@mozilla.org (or reviewers-internal@netscape.com
for commercial tree patches).
Comment 19•23 years ago
|
||
awesome - much simpler :)
sr=alecf
Assignee | ||
Comment 20•23 years ago
|
||
Fix checked in.
Assignee | ||
Comment 21•23 years ago
|
||
Opps. Apparently I didn't check the radio button.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•