Skip to content

Commit

Permalink
Send proper outbound cookie header
Browse files Browse the repository at this point in the history
Outbound cookies should only include name and value in the header.
In Node.js versions < 8 if `header.cookie` is set to an array, the values a concatenated with commas which is wrong [according to the specs](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cookie). This was [fixed in Node 8](nodejs/node#11259), but for compatibility if we join the cookie values and set `header.cookie` to a string it will always be right.

To avoid breakage, `crawler.cookies.getAsHeader()` still returns an array, but without the unexpected attributes. When it is added to request options it is joined into a semicolon-separated string.
  • Loading branch information
Lexmark-haputman committed Sep 20, 2017
1 parent 9c70222 commit 82f73b0
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 9 deletions.
10 changes: 9 additions & 1 deletion lib/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ CookieJar.prototype.getAsHeader = function(domain, path, callback) {
}
})
.map(function(cookie) {
return cookie.toString();
return cookie.toOutboundString();
});

if (callback instanceof Function) {
Expand Down Expand Up @@ -329,6 +329,14 @@ Cookie.fromString = function(string) {
);
};

/**
* Outputs the cookie as a string, in the form of an outbound Cookie header
* @return {String} Stringified version of the cookie
*/
Cookie.prototype.toOutboundString = function() {
return this.name + "=" + this.value;
};

/**
* Outputs the cookie as a string, in the form of a Set-Cookie header
* @param {Boolean} [includeHeader] Controls whether to include the 'Set-Cookie: ' header name at the beginning of the string.
Expand Down
2 changes: 1 addition & 1 deletion lib/crawler.js
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ Crawler.prototype.getRequestOptions = function(queueItem) {
// send/accept cookies
if (crawler.acceptCookies && crawler.cookies.getAsHeader()) {
requestOptions.headers.cookie =
crawler.cookies.getAsHeader(queueItem.host, queueItem.path);
crawler.cookies.getAsHeader(queueItem.host, queueItem.path).join("; ");
}

// Add auth headers if we need them
Expand Down
4 changes: 0 additions & 4 deletions test/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,6 @@ describe("Cookies", function() {

parsedCookie2.name.should.equal(parsedCookie.name);
parsedCookie2.value.should.equal(parsedCookie.value);
parsedCookie2.expires.should.equal(parsedCookie.expires);
parsedCookie2.path.should.equal(parsedCookie.path);
parsedCookie2.domain.should.equal(parsedCookie.domain);
parsedCookie2.httponly.should.equal(parsedCookie.httponly);
});
});

Expand Down
25 changes: 22 additions & 3 deletions test/testcrawl.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,28 @@ describe("Test Crawl", function() {

crawler.on("fetchstart", function(queueItem, requestOptions) {
if (i++) {
requestOptions.headers.cookie.should.be.an("array");
requestOptions.headers.cookie.should.have.lengthOf(1);
requestOptions.headers.cookie[0].should.match(/^thing=stuff/);
requestOptions.headers.cookie.should.be.a("string");
requestOptions.headers.cookie.should.match(/^thing=stuff$/);
done();
}
});

crawler.start();
});

it("should send multiple cookies properly", function(done) {
var crawler = makeCrawler("http://127.0.0.1:3000/"),
i = 0;

crawler.cookies.addFromHeaders([
"name1=value1",
"name2=value2",
"name3=value3"
]);
crawler.on("fetchstart", function(queueItem, requestOptions) {
requestOptions.headers.cookie.should.be.a("string");
requestOptions.headers.cookie.should.match(/^(name\d=value\d; ){2}(name\d=value\d)$/);
if (i++ === 6) {
done();
}
});
Expand Down

0 comments on commit 82f73b0

Please sign in to comment.