Closed Bug 118373 Opened 23 years ago Closed 20 years ago

escape(null) causes cookies to not be recognized

Categories

(Core :: DOM: Core & HTML, defect)

x86
Windows NT
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: asteele, Unassigned)

References

()

Details

Attachments

(7 files)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.7) Gecko/20011221
BuildID:    2001122106

The site (www.theissue.org) sets two cookies on the first visit to identify
whether the users preference is to have sound played or not.

In Netscape 4.7x, 6.2 and IE 6 this works correctly.  On second and subsequent
visits the browser loads either the with-sound or no-sound site pages based on
the last cookie set.

With Mozilla 0.9.7 the cookies is not recognised and Mozilla loads the default
page which takes the visitor through the initial set-up.

The cookie appears to be written into the cookies.txt file OK but is not
interpreted by Mozilla.

Reproducible: Always
Steps to Reproduce:
1.Go to http://www.theissue.org
2.Site loads default page offering sound or no-sound
3.Select no-sound for simplicity
4.Close browser return to the URL and the initial selection pages load again

Actual Results:  As described...

Expected Results:  Interpreted the cookie and based on the initial selection
by-passed the initial page and gone straight to the no-sound and with-sound
default page.

I'm happy for any gentle chiding if I have mis-coded the cookie stuff...

As mentioned though, it all works OK in IE6, 5.5 and Netscape 4.7x and 6.2
site is doing:

  document.cookie = sound + "=" + soundset + "; expires=" + now.toGMTString();

I bet this is bug 118266
I don't think this is bug 118266, but I could be wrong.

My initial test indicated that the cookie.txt file in n4 had this cookie set 
with a value of "null" (characters "null" actually appear in file) whereas in 
mozilla the value was the empty string.  I suspect therein lies the problem 
although I was unable to follow-up on this because my browser broke for other 
reasons.  As soon as get my tree fixed, I intend to continue pursuing this.
Yes, I was right.  It's the value of "null" that is causing the problem.  I 
prooved that to myself by forcing the cookie module to return the string "null" 
for the cookie value when the value itself is the empty string.  Attaching a 
patch to demonstrate that.

This is not a proposed patch to fix the problem but rather a proof of concept as 
to what the problem is.  Need to determine if this is an evangelism issue on the 
site for not recognizing the cookies that it itself set (it indeed set a cookie 
with no value).  Also need to look at the 4.x code and see why/how it forces the 
value of "null" into the cookie.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla0.9.8
Looked at the code for 4.x.  There is nothing in the cookie module itself that 
forces a value of "null" into a cookie with an empty value.  I no longer have a 
4.x debug environment so I can't trace where the value of "null" is coming from, 
but since this is a javascript cookie I would venture a guess that js in 4.x was 
returning the value of "null" whereas in mozilla it is returning the value of 
"".

So it seems like we have three choices:

1. If a cookie is being set with a value of "", replace that with "null".
2. Same as one but only for javascript cookies
3. Tell site to accept their own cookies having a value of ""

Solution 2 is safe in that it won't break existing sites because it exactly 
mimics the 4.x behavior (if my theory about where the "null" is coming from is 
correct).  However it seems very strange to treat js cookies one way and http 
cookies another.

Solution 1 is uniform at least, but it could break some sites that set http 
cookies of "".
Which code exactly is setting the cookie to null?
jst: Funny you should ask.  That's just what I'm investigating right now.  Hope 
to have an answer soon.
Oops, I didn't mean to put "jst:" on my previous comment.  I meant to put 
"bzbarsky:" there instead.
OK, I think I know what's happening and I believe it is a js error.

Attaching a simplified test case.  Consists of two files, theissue.html and 
welcome.html.  Put both files in same directory and bring up theissue.html in 
the browser.

Will explain what's going on after I attach the simplified test case.
Attached file theissue.html
Attached file welcome.html
Well html files are hard to see in attachments.  So I'll also put these two 
files inline right here.

************ theussue.html *********************

<HTML>
  <HEAD>
    <SCRIPT LANGUAGE="JavaScript">

      function setCookie(name, value) {
        dump("setCookie: {"+name+"} = {"+value+"}\n");
        document.cookie = name + "=" + escape(value);
      }

      function getCookie(name) {
        var prefix = name + "=";
        var cookieStartIndex = document.cookie.indexOf(prefix);
        if (cookieStartIndex == -1) {
          dump("getCookie: no cookie for {"+name+"} found\n");
          return null;
        }
        var cookieEndIndex =
          document.cookie.indexOf(";", cookieStartIndex + prefix.length);
        if (cookieEndIndex == -1) {
          cookieEndIndex = document.cookie.length;
        }
        var value = 
          document.cookie.substring
            (cookieStartIndex + prefix.length, cookieEndIndex);
        dump("getCookie: {"+name+"} = {"+unescape(value)+"}\n");
        return unescape(value);
      }

      var visits = getCookie("newuser")
      if (!visits) {
        dump("site believes that no newuser cookie is set\n");
        location = "welcome.html"
        setCookie("newuser", visits)
      }
    </SCRIPT>
  </HEAD>
  <BODY>
  </BODY>
</HTML>

***************** welcome.html ************************

<HTML>
  <BODY>
    Welcome
  </BODY>
</HTML>
Hmm.. are we being bitten by our correct support for null DOMStrings?  What 
should null.ToString() be?
Well I don't yet understand it, but let me post the results of running my 
simplified test case on both nav4 and on mozilla:

1. Mozilla:

   First visit to theissue.html:

   getCookie: no cookie for {newuser} found
   site believes that no newuser cookie is set
   setCookie: {newuser} = {null}

   Subsequent visits to theissue.html:

   getCookie: {newuser} = {}
   site believes that no newuser cookie is set
   setCookie: {newuser} = {}

2. Nav4

   First visit to theissue.html:

   getCookie: no cookie for {newuser} found
   site believes that no newuser cookie is set
   setCookie: {newuser} = {null}

   Subsequent visits to theissue.html:

   getCookie: no cookie for {newuser} found
   site believes that no newuser cookie is set
   setCookie: {newuser} = {null}
3. IE

   First visit to theissue.html:

   getCookie: no cookie for {newuser} found
   site believes that no newuser cookie is set
   setCookie: {newuser} = {null}


   Subsequent visits to theissue.html:

   getCookie: {newuser} = {null}

And one more bit of information.  If the escape/unescape is removed from the 
simpified test case, then the results for Mozilla, Nav4, and IE are all 
identical -- they are the results that was previously obtained for IE.
OK, here's where the difference lies.  In Nav 4 and in IE

   escape(null) = "null"

wheras in mozilla

   escape(null) = ""

Here's a still more simplified test program to demonstrate this:

  <html>
    <head>
      <script>
        alert("escape(null)={"+escape(null)+"}");
      </script>
    </head>
    <body>
    </body>
  <html>
And for my final proof, I modified my local copy of the http://www.theissue.com 
website by removing the escape in the setcookie routine, and everything worked 
fine.

So now I'm convinced that this is not a cookie problem but rather a javascript 
one per my observation in comment #17.  Reassigning.

(Now that I understand this better, I retract the three choices I stated in 
comment #5.  None of them were valid.)
Component: Cookies → Javascript Engine
Reassigning to javascript engine component.
Assignee: morse → rogerl
Status: ASSIGNED → NEW
QA Contact: tever → pschwartau
Modifying summary from

   cookies not recognized

to

   escape(null) causes cookies to not be recognized
Summary: Cookies not recognised → escape(null) causes cookies to not be recognized
Just for the record, I'm attaching three files that make up my local copy of 
the http://www.the issue.org website.  These files are:

   theissue.html (starting file)
   welcome.html
   sound.html
   page2.html

The escape that I spoke about in comment 18 is in the setCookie routine of 
theissue.html and the identical routine in sound.html.

Note that when using these files, make sure to check of the box saying you want 
sound.  That will cause the sound.html file to be loaded in.  If you check off 
the no-sound box, it will attempt to fetch a different file which I have not 
included here.
Sorry, I lied in comment #21.  It was four files, not three.
From Comment #17 above: 

> OK, here's where the difference lies.  In Nav 4 and in IE
>    escape(null) = "null"
> whereas in Mozilla
>    escape(null) = ""


Confirming this behavior. Note that in the browser, the DOM escape()
function supersedes the JavaScript Engine escape() function. In fact,
in the current standalone JS Engine, we show the Nav4/IE behavior:

            js> escape(null);
            null

Whereas in a current Mozilla build, the following javascript: URL 
shows that in the DOM, escape(null)=== the empty string, as Stehen says:

            javascript: alert(escape(null) === '');


Therefore reassigning to DOM Level 0 component - 
Assignee: rogerl → jst
Component: Javascript Engine → DOM Level 0
QA Contact: pschwartau → desale
Boris was right, this is beause we pass the new null string to
nsGlobalWindow::Escape().
I think something as simple as
if (isNullDOMString(aStr)) {
  setDOMStringToNull(aReturn);
}
would do the job (and would avoid constructing all the encoder stuff we
currently build in this case).
As always a most helpful and thorough contribution from everyone.  I've asked
the author to take note of the comments and adjust the code which has resolved
the problem for the particular site.  Thus the live site should very shortly
function OK.  This won't, though, be because the bug has been fixed but because
the page code has been changed :-)

I hope that the discussion has been helpful for future
implementations/development - I've certainly found it instructive.
Even if the site changes their code to work around the problem, we can still 
reproduce it using the captured files form the site that I attached in this 
report.
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Pushing to mozilla1.0
Target Milestone: mozilla0.9.9 → mozilla1.0
Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to adt@netscape.com.  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2
Target Milestone: mozilla1.2alpha → ---
Mass-reassigning bugs to dom_bugs@netscape.com
Assignee: jst → dom_bugs
OK, so in IE and NS4 escape(null) == "null" (the string, not a null value).  Do
we want to emulate this behavior?
The JS spec as the following to stay about escape():
1. Call ToString(string)
2. Compute the number of characters in Result(1)
3. Let R be the empty string
4. Let k be 0
5. If k equals Result(2), return R

So per spec, we are doing the right thing. I searched for some uses of
escape(null) on the web and couldn't find any, so I guess if this bug doesn't
block the site anymore, we should leave the behavior as is.
Just IMHO, of course.
Um.. What is ToString(null)?  Is it ""?
does it matter? the number of chars in it is 0...
As long as ToString(null) is not "null"  ;)

Ok, I'm fine with just marking this wontfix.
Marking WONTFIX, please feel free to reopen if you disagree.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WONTFIX
I don't know the EcmaScript spec in enough detail to know what ToString(null)
is, but I know JS_ValueToString(null) in Mozilla gives the string "null"...
Ref: http://www.mozilla.org/js/language/E262-3.pdf

I don't know if the EcmaScript spec is relevant here, since in the browser
escape() is superseded by the DOM escape() (right?). But FWIW, here is the
EcmaScript specification for |escape(null)|:

B.2.1 escape(string)
When the escape() is called with argument string, the following steps are taken:

1. Call ToString(string)
        etc.


9.8 ToString
The operator ToString converts its argument to a value of type String
according to the following table:

Input Type      Result
Undefined     "undefined"
Null            "null"
        etc.


So in pure, browser-independent EcmaScript, escape(null) === "null"
Reopening, then.  Because both IE and NS4 return "null" while we return "".

Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
We now use the JS engine's escape(), so this is fixed (by the patch in bug 44272).
Status: REOPENED → RESOLVED
Closed: 21 years ago20 years ago
Depends on: 44272
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: