Expired cookie is still present in document.cookie

[rmurphey]

jsdom version: 3.1.1

I ran into a bug that is alluded to in #310 and referenced in ScottHamper/Cookies#37 and jamesplease/backbone.simple-auth#1: A cookie set with an expiration date in the past -- as recommended on MDN as the way to delete a cookie -- is still present in document.cookie inside jsdom.

jsdom test case:

var jsdom = require('jsdom');

jsdom.env('<p>expired cookie bug</p>', function (err, window) {
  var doc = window.document;

  function create(name, value, days) {
    var date = new Date();
    date.setTime(date.getTime() + (days * 24 * 60 * 60 * 1000));
    var expires = days ? ';expires=' + date.toGMTString() : '';

    var c = name + '=' +
      encodeURIComponent(value) +
      expires +
      ';path=/';

    doc.cookie = c;
  }

  // set a cookie that expires in 10 days
  create('testcookie1', '1', 10);
  console.log('unexpired cookie is present', !!doc.cookie.match('testcookie1=1'));

  // set a cookie that has already expired
  create('testcookie2', '', -1);
  console.log('expired cookie is present', !!doc.cookie.match('testcookie2'));
});

This results in the following output:

unexpired cookie is present true
expired cookie is present true

Here is a browser test case:

var doc = window.document;

function create(name, value, days) {
  var date = new Date();
  date.setTime(date.getTime() + (days * 24 * 60 * 60 * 1000));
  var expires = days ? ';expires=' + date.toGMTString() : '';

  var c = name + '=' +
    encodeURIComponent(value) +
    expires +
    ';path=/';

  doc.cookie = c;
}

// set a cookie that expires in 10 days
create('testcookie1', '1', 10);
console.log('unexpired cookie is present', !!doc.cookie.match('testcookie1=1'));

// set a cookie that has already expired
create('testcookie2', '', -1);
console.log('expired cookie is present', !!doc.cookie.match('testcookie2'));

The output is as expected:

unexpired cookie is present true
expired cookie is present false

[inikulin]

Seems like jsdom's cookies mechanism doesn't meet RFC 6265. Maybe we should adopt https://github.com/goinstant/tough-cookie? I can take care of it somewhere around next week. /cc @domenic @Sebmaster

[Sebmaster]

I had a look at though-cookie once already, but as far as I remember there was something else missing or not compliant, however I currently can't remember exactly what it was.

I'll take another look.

[inikulin]

We use it currently heavily in production and haven't encountered any problems on Alexa top 500 websites.

[inikulin]

I found test suite for the HTTP-state: https://github.com/abarth/http-state/tree/master/tests I'll try to run it against tough-cookie on weekend. If we will have something missing/broken I can take care of PR/fork.

[domenic]

Would love to farm out cookie handling to another module. Happy to take anyone's patches that does that :). Either that or we can keep patching our current implementation.

Also worth checking out: https://github.com/assaf/zombie/blob/master/src/cookies.js

[domenic]

Oh and feel free to create the module under github.com/jsdom, I can add you to it as a collaborator if you want. (You don't have to though.)

[inikulin]

@domenic I hope we will be able to end up with the tough-cookie, without dealing with new module.
Here is the plan:

• Run IETF's http state tests against tough-cookie to figure out it's spec compliance state
• Fix falling tests (if we will have one)
• Issue the PR to the tough-cookie repo
• Delegate cookies handling to tough-cookie in jsdom. Move cookies getters/setters to the living/document.js(?). Optionally it would be nice to refactor resource loader into the separate module (browser/resource_loader.js)
• Optionally: If at that moment we will not have our PR merged to the tough-cookie upstream we can consider to go with our own fork at jsdom org.

I hope I will be able to start tomorrow, I need to rollout pending updates to parse5 first.

[domenic]

Sounds like a lovely plan <3.

[inikulin]

Ooook, we have 71 failing tests from the IETF's suite at the moment (Vows reporter is extremely shitty BTW). Moreover IETF data is a little bit obsolete. I'll issue them a PR with the update tool once I finish fixing current failing tests. Seems like It will be a looong road 🔨

[inikulin]

I've fixed all issues in tough cookie and issued them a PR (salesforce/tough-cookie#30). Now we can move to the jsdom part.

[inikulin]

@domenic I have a question. My first intention was to move document.cookie to Document interface to the living/document.js. Surpisingly, I haven't found cookie into the Document interface of the Living Standard. According to the Living standard goals:

This specification standardizes the DOM. It does so as follows:
By consolidating DOM Level 3 Core [DOM3CORE], Element Traversal [ELEMENTTRAVERSAL], - Selectors API Level 2 [SELECTORSAPI], the "DOM Event Architecture" and "Basic Event Interfaces" chapters of DOM Level 3 Events [DOM3EVENTS](specific type of events do not belong in the base specification), and DOM Level 2 Traversal and Range [DOM2TR]

Meanwhile, document.cookie declared in the Document Object Model Level 2 HTML, therefore at the moment cookie is not a part of the Living Standard.
So, should I keep document.cookie in level2/html.js to conform current specs state?

[domenic]

@inikulin cooke is in the HTML Standard now, which supplements the DOM Standard. We don't have a clear division between HTML and DOM in living/, and I'm not sure we want or need one. So living/document.js is fine, or maybe living-html/document.js

[inikulin]

Thank you fo the clarification, found them in the HTML Standard.

[rmurphey]

Thanks everyone!