Closed Bug 169091 Opened 17 years ago Closed 12 years ago

[cookies] Assume <VALUE> instead of <NAME> if only one attribute specified

Categories

(Core :: Networking: Cookies, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jcarpenter0524, Assigned: dwitte)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

-In Prefs under Privacy & Security/Cookies, set cookies to "Enable cookies for
the originating web site only."

- Go to the Bank of America website
https://onlineid.bankofamerica.com/cgi-bin/sso.login.controller?state=CA

Result: you are transferred to an error page with this message:


We have determined that your browser is not currently enabled to accept cookies.
In order to present you with the most recent version of our online features, we
need you to enable your browser to accept cookies by following these steps:

    # From the menu at the top of the page select Edit and then Preferences.
    # In the resulting box, click Advanced from the "Category" menu at the left.
    # In the resulting box, click "Accept all cookies" from the "Cookies"
options at the right.
    # Select OK to close the "Preferences" box. 

OK


- Change cookies to "Enable all cookies"

- Go to BofA website again 


Result:  Again get transferred to error page.

Note:
This bug is not happening with todays trunk build.
So if it's not happening with today's trunk build, what build is it happening
with?  And based on what you said, is there any reason that we shouldn't close
this as works-for-me?
(doh!)  Sorry, forgot the important part where it *is* happening on todays N700
build:

2002-09-20-08-1.0

it's just not happening on the trunk builds.
So if it's no longer happening on the trunk, that means that it's been fixed
with some other checkin.  Any reason I shouldn't close this out as wfm?
I have a problem like this one.
With the latest Trunk Builds incl. Build-ID 2002092308 I get an Error that the
Cookies are disabled but I enabled all Cookies and get no difference with it.
The URL is the german "Deutsche Bank"
https://banking.db24.de/mod/WebObjects/db24 (there is also an link to an english
versio of the page on the upper left)
This Bug seams to be near Bug 165681
Doer, as I mentioned in bug 165681 comment 13, please open a separate bug report
if there is a problem with Deutche Bank.  That is a different websites and would
have to be evangelized separately.

Regarding this bug which has to do with BofA, I'm hearing the reporter (janc)
say in the description and in comment 2 that this does not happen on the trunk
build.  In that case the report should be closed out as wfm, and I'll do so.  If
I'm wrong and it still does happen on the trunk, then please reopen the report.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WORKSFORME
bug 171235 has just been opened for deutche bank problem.
Reopening.
I just tried this in todays trunk build: 2002-10-23-08-trunk
and have the same problem.

I have all cookies enabled, and even unchecked "ask me before storing a cookie".
 Cookies are not blocked from the site, and I checked to make sure there is no
listing for bofa or bankofamerica in my 'don't accept cookies from these sites'
list.  I have one cookie *for* onlineid.bankofamerica.com which was set today. 
If I delete that cookie and try to go to the site again, it resets the cookie,
but still won't let me onto the site.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
According to the description, all you need to do to demonstrate this bug is to
go to the url indicated.  You do not need to enter any account number nor press
any submit buttons.  Is that correct?

I just tried it and I got to the signin form just fine.  No error message
appeared.  tever, can you verify that?

I also tried entering a bogus account number just to see what would happen. 
Again I didn't get any message about cookies.  I did of course get a message
saying that the password I entered was invalid, but that was to be expected.

OK, I just noticed that this bug report requires that you first try the test
with the cookie pref set to originating server only.  So I just tried that. 
Still am not getting a message about cookies not being accepted when I go to
visit the sign-in form.

Please verify that these are indeed the correct steps:

1. Set cookie pref to originating site only
2. Go to https://onlineid.bankofamerica.com/cgi-bin/sso.login.controller?state=CA
3. Should get cookie error message at this point.  I do not get it.
4. Set cookie pref to accept all
5. Go to above site again
6. Should get cookie error message again.  I still do not get it.
I see the bug on OS/2 and Linux builds 2002120512
Changing OS -> All
OS: Windows NT → All
Ops, sorry. I've mistaken this bug for another one.

WFM OS/2 build 2002121612
WFM win2k 2002010608 trunk.

janc: are you still able to repro this bug w/ a recent trunk build?  thx!
*** Bug 194759 has been marked as a duplicate of this bug. ***
I can reproduce this bug on build 2003032804 at https://easyweb.tdcanadatrust.com/
Confirming comment 14 under 03/28-08 / XP.  Except for the fact that I've never
visited that site before so there was never an initial "cookies disabled"
message that might have set things in motion.  Cookies have *always* been
enabled (session only however) so it has nothing to do with that site
remembering anything about them having been denied in the past.  (I'm not sure
if this makes it a different bug or not.)
hrm, okay, the bug suggested by comment #14 is interesting. the site works in IE
and 1.3, but not a post-rewrite trunk.

so it turns out this site relies on a subtle behavior. the javascript code in
question is:

	document.cookie = "Y";
	var dc = document.cookie;
	document.cookie = "";
	if (dc == document.cookie)
		alert("You do not have cookies enabled, please go into your browser's
preferences and enable cookies so that EasyWeb will function correctly");

so it's trying to set a cookie "Y", overwrite it with "", and then make sure it
was overwritten properly. this sure is a strange way to check if cookies are
enabled...

the problem is, moz's behavior in this situation was changed during the rewrite
- the site is setting a cookie without an "=" delimiter. so for a <NAME>=<VALUE>
syntax, our previous behavior was to assume a blank <NAME> and assign the data
into <VALUE>. during the rewrite, I couldn't see a good reason to stick to this
(RFC2109 specifies both attributes as non-optional), and it made more sense to
change it; so we currently assign the data into <NAME> and assume a blank
<VALUE>. when the site sets both these cookies, they are treated as having
different names, so |document.cookie = "";| does not overwrite |document.cookie
= "Y";|.

i've created a patch that "fixes" this problem, so we'll assume a blank <NAME>
and the cookie will be overwritten correctly if no "=" delimiter is present.
whether we want to do this is something i'm not terribly sure of...

pros:
a) IE does it
b) it makes sense from a javascript perspective (i don't want to bother with
specifying a <NAME>, i just want to set a cookie...)

cons:
c) spec doesn't say what to do
d) the fix is kind of a half-measure: we'll still parse the string according to
the <NAME> grammar, which is different from the <VALUE> grammar (the latter
allows quoted-strings etc). to fix this would complicate the parser a little
(we'd have to "look-ahead", see if there's an equals, then decide which rule to
parse the string by). so to do it properly is a little trickier, and i've left
this for later, since we don't want the risk of a parser regression so close to
1.4a.

so should we throw this to evang (what kind of cookie-checking javascript is
that anyway?!), or use this fix?
Attached patch fix for comment#14 (obsolete) — Splinter Review
Attachment #118843 - Flags: superreview?(alecf)
Attachment #118843 - Flags: review?(darin)
-> me
Assignee: morse → dwitte
Status: REOPENED → NEW
Status: NEW → ASSIGNED
oh, i forgot one more con:

e) it doesn't make sense from moz's perspective (set your cookies according to
spec, dammit)
I would suggest fixing it on our side (after all, it was our stuff that
changed). Also, I would actually suggest you to test if IE uses the <VALUE>
grammar, so we could make an informed decision as to how important it would be
to "fix" that too.
Comment on attachment 118843 [details] [diff] [review]
fix for comment#14

sr=alecf
Attachment #118843 - Flags: superreview?(alecf) → superreview+
Comment on attachment 118843 [details] [diff] [review]
fix for comment#14

r=darin
Attachment #118843 - Flags: review?(darin) → review+
Comment on attachment 118843 [details] [diff] [review]
fix for comment#14

requesting approval1.4a.

this is a fairly simple fix for a regression introduced in the cookie rewrite,
which affects cookies that don't conform to the cookie spec. with this fix,
we'll be consistent with IE's behavior. while it's not a serious flaw, many
sites use non-conforming cookies, so we're sure to see some site breakage in
1.4a without this.
Attachment #118843 - Flags: approval1.4a?
heikki, darin: point taken, consistency with IE is paramount. ;)

I'll look into IE's parsing more closely at some point; it may be that we just
want to simplify the parser to use the same grammar for both <NAME> and <VALUE>.

thanks for the advice!
changing summary, since the current one has nothing to do with the fixes here.
Keywords: regression
Summary: Once I get a "cookies disabled" message from BofA, I can't get in even if I enable cookies → [cookies] Assume <VALUE> instead of <NAME> if only one attribute specified
Attachment #118843 - Flags: approval1.4a?
Attached patch patch v2Splinter Review
ugh, fixed a mistake. s/value/name/ in one place.
Attachment #118843 - Attachment is obsolete: true
Comment on attachment 119029 [details] [diff] [review]
patch v2

carrying over r/sr, requesting approval1.4a (previous request comment still
applies).
Attachment #119029 - Flags: superreview+
Attachment #119029 - Flags: review+
Attachment #119029 - Flags: approval1.4a?
Attachment #119029 - Flags: approval1.4a?
checked in to trunk by bz @ 2013. thx!

this didn't make alpha, so we're likely to see some bugs reported on 1.4a, "my
cookies don't work". as noted in the approval request, the problem will only
affect sites which set noncompliant cookies in this specific way - _and_ only
then if they rely on cookie deletion/replacement operations. unfortunately this
can't be considered a site evang issue since IE tolerates & handles this case
correctly. i'll do my best to dupe these bugs if I see any.

so the checked-in fix will get things working on trunk again, and i'll do some
further testing and consider updating the parser at some point before beta.
leaving this bug open for that.
> unfortunately this can't be considered a site evang issue since IE tolerates
> & handles this case correctly.

I tend to disagree with that statement - at face value anyway.  Just because IE
does it, doesn't mean that Mozilla should do it.  Especially if, as you say, the
problem is with sites that "set noncompliant cookies" that don't adhere to
RFC2109.  (Surely, even if Mozilla does accept such cookies, it would *still* be
beneficial, overall, to evangelize the issue with any site that may be unaware
of what it's doing.)  I'm also not certain that saying we should work with such
sites now because we used to so in the past (comment 20) is a legitimate conclusion.

I'm not saying that we're doing the wrong thing here - perhaps we should have
Mozilla handle such cases - but we should be doing it because it makes sense. 
Not just because IE does it or Mozilla *used* to do it... 

(Perhaps, if we continue to accept them, we could add information about such
questionable cookies to any properties dialogue that's displayed about them.)
I agree with the general sentiment, but in this case it just makes much more
sense to deal with it.

1) there doesn't really exist a clear distinction between "compliant" and
"noncompliant" cookies, since pretty much the entire web is using a defacto
cookie standard (i.e. _most_ websites set noncompliant cookies, wrt RFC2109). so
we implement this defacto standard anyway, which is unwritten and thus basically
comes down to what myself and the module owner (darin) think we should be doing.
this is the reason for this regression in the first place, because what I
thought we should do wasn't what the web wanted. ;)

2) IE does it. IE represents >90% market share.

3) it's really not a difficult thing to support. we're talking 32 bytes codesize
increase from aforementioned checkin.

we've got bigger fish to fry. ;)
btw, sorry, your original point was correct... what I said was:

> unfortunately this can't be considered a site evang issue since IE tolerates
> & handles this case correctly.

implicit in that was the point I just mentioned, that the web mostly runs off a
defacto cookie standard anyway, and IE is effectively responsible for creating
this standard. so the "we are more standards-compliant than IE" argument doesn't
apply here. sorry for leaving out an important qualifier ;)
jasonb: just to emphasize what dwitte said... a strict implementation of RFC2109
will fail _miserably_ on a large percentage of websites.  sad, but true.
I'm seeing the bug as originally described.

Comment #28 says something was checked in to the trunk on 2003-04-01, but I'm
getting the same error (B of A thinks I'm blocking cookies I'm not blocking.) in
1.5 final, which branched 29-Aug-2003, and the bug isn't marked Fixed.

My test was to turn on <allow all cookies> and try to login at the given url,
and get shunted to the <your browser is not currently enabled to accept cookies>
page.

Workaround: Hit stop at JUST the right time (after the page loads, before the
script runs) and you can login; the rest of the site works fine.

(OT: is there a way to save the page locally, edit out the JS, make the relative
URLs refer to the https://onlineid.bankofamerica.com/ site without changing each
one?  My attempt (comment out the call to checkForCookies, make all 3 action
URLs absolute) didn't work, perhaps because referrer was 'wrong'.)

It looks like the code on the page changed too; it now reads:
function checkForCookies()
{
var today = new Date();
var expire = new Date();
expire.setTime(today.getTime() + 3600000 * 24);
document.cookie = "cookieEnabledPersist=true; expires=" + expire.toGMTString();
document.cookie = "cookieEnabledSession=true";
var cookieString = ""+document.cookie;
if (cookieString.indexOf("cookieEnabledPersist")==-1 ||
cookieString.indexOf("cookieEnabledSession")==-1)
{
document.location.href = "/cgi-bin/sso.login.controller?state=CA&reason=noCookie";
}
}

Which isn't setting cookies w/o a value.
Err, sorry for the bugspam:  I was fiddling around and realized I had turned off
JavaScript's ability to read and write cookies in Preferences | Advanced |
Scripts... 
Enabled 'em; works now.  (Shall I mark fixed?)
please leave the bug open for now, per comment 28.
dwitte, can we mark this fixed yet or are you still waiting on something?  Its
been a year at this point, and no dupes.
QA Contact: tever → cookieqa
per comment 35 per comment 28, i still need to look at this, it's just v. low
priority.

good morning mconnor ;)
not really worried about this one anymore -> fixed
Status: ASSIGNED → RESOLVED
Closed: 17 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.