Skip to content

http: append Cookie header values with semicolon - #11259

Merged
mscdex merged 2 commits into
nodejs:masterfrom
mscdex:http-use-semicolon-for-cookie-header
Mar 9, 2017
Merged

http: append Cookie header values with semicolon#11259
mscdex merged 2 commits into
nodejs:masterfrom
mscdex:http-use-semicolon-for-cookie-header

Conversation

@mscdex

@mscdexmscdex commented Feb 9, 2017

Copy link
Copy Markdown
Contributor

Previously, separate incoming Cookie headers would be concatenated with a comma, which can cause confusion in userland code when it comes to parsing the final Cookie header value. This commit concatenates using a semicolon instead.

Fixes: #11256

I've tentatively marked this as semver-major for now to be on the safe side.

/cc @nodejs/http ?

CI: https://ci.nodejs.org/job/node-test-pull-request/6312/

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)
  • http

@mscdexmscdex added http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Feb 9, 2017
@nodejs-github-botnodejs-github-bot added the http Issues or PRs related to the http subsystem. label Feb 9, 2017
@mscdex
mscdexforce-pushed the http-use-semicolon-for-cookie-header branch from 95b0257 to a94162dCompareFebruary 9, 2017 12:07
@jasnell

Copy link
Copy Markdown
Member

Accepting more than one Cookie header is something, I'm afraid we'll have to continue supporting. It would break too much existing stuff to change that. We definitely need to make sure that we're only sending one Cookie header.

@kunalspathak

Copy link
Copy Markdown
Member

LGTM

@isaacs

Copy link
Copy Markdown
Contributor

Thanks for jumping on this so fast, @mscdex.

Maybe I'm missing something, but it doesn't look like this addresses the http client, right? In other words, http.request({ headers: { cookie: [ 'foo=boo', 'bar=qux' ] } }) still gets concatenated with a ,, or sent as multiple headers?

@mscdex

Copy link
Copy Markdown
ContributorAuthor

@isaacs I was under the impression that was what you were requesting (the merging of incoming Cookie headers). I can add changes for outgoing Cookie headers as well.

@mscdex

Copy link
Copy Markdown
ContributorAuthor

Actually, I'm curious what other @nodejs/collaborators think about adding this implicit concatenation for outbound client requests. Even though it may make sense to combine them, do we want to provide some way to allow users to bypass this for any reason? For example, if a user is writing an array of headers (not very common AFAIK), such as [ [ 'Cookie', 'foo=bar'], ['Cookie', 'bar=baz'] ], do we skip the implicit concatenation in that case since that format is a little more explicit compared to the more common 'object' format (e.g. { Cookie: ['foo=bar', 'bar=baz'] })?

I know I've previously seen users that have gone the array route to avoid some implicit core behavior in order to work properly with particular servers.

Comment threadlib/_http_incoming.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the comment block describing the behavior of this block be updated as well to describe this new behavior?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I was referring to the main comment block above the method / function. I'll make a new comment about it.

@sam-github

Copy link
Copy Markdown
Contributor

Headers in general, not just Cookie, have lots of requirements around what can or can not be done with them (multiple allowed, what they mean, syntax of values, etc.). Does node want to be in the business of adding that kind of logic, or kick it to user-land, which is actually choosing which headers it wants to send? If we start merging outgoing headers for Cookie, will it then start to be reported as a bug if we don't apply other rules from the HTTP specs?

@jasnell

Copy link
Copy Markdown
Member

On this I'd prefer to leave the higher order bits to userland and focus on ensuring only the minimum required for base http compliance.

@mscdex

Copy link
Copy Markdown
ContributorAuthor

On this I'd prefer to leave the higher order bits to userland and focus on ensuring only the minimum required for base http compliance.

@jasnell Wouldn't that include only sending one Cookie header? You mentioned earlier:

We definitely need to make sure that we're only sending one Cookie header.

@jasnell

Copy link
Copy Markdown
Member

Sorry, I wasn't clear. I was referring specifically to the difference in inputs described here: #11259 (comment)

That is, for the typical object input, e.g. {'cookie': ['a', 'b']}, definitely only send one. For the array input [['cookie', 'a'], ['cookie', 'b']], send multiple.

@mscdex

Copy link
Copy Markdown
ContributorAuthor

Ok, I've pushed a separate commit that automatically concatenates outgoing Cookie headers, except when passed a 2D headers array.

@mscdex

Copy link
Copy Markdown
ContributorAuthor

Comment threadlib/_http_outgoing.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about lowercasing the string and comparing?

@mscdexmscdexFeb 12, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toLowerCase() + compare is very slow (slower than a case-insensitive regexp IIRC).

@mscdex
mscdexforce-pushed the http-use-semicolon-for-cookie-header branch from 393cd97 to 5b33a27CompareFebruary 17, 2017 02:03
@mscdex

Copy link
Copy Markdown
ContributorAuthor

/cc @nodejs/collaborators any further comments/LGTMs?

@mcollina

Copy link
Copy Markdown
Member

@mscdex I'd like to see a citgm run, and check if this causes issues in userland.

@mscdex

Copy link
Copy Markdown
ContributorAuthor

@mscdex

Copy link
Copy Markdown
ContributorAuthor

@mcollina Looking through the results of a couple of the CITGM runs, I do not see any failures related to the changes in this PR.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mscdex

mscdex commented Feb 18, 2017

Copy link
Copy Markdown
ContributorAuthor

CI once more before landing: https://ci.nodejs.org/job/node-test-pull-request/6494/

@mscdex
mscdexforce-pushed the http-use-semicolon-for-cookie-header branch from 5b33a27 to def8d7dCompareFebruary 18, 2017 17:08
@mscdex

mscdex commented Feb 18, 2017

Copy link
Copy Markdown
ContributorAuthor

Actually, I think technically we need at least one more @nodejs/ctc member to LGTM this before landing since it's a semver-major...

Comment threadlib/_http_incoming.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the comment block describing the behavior of this function be updated as well to describe this new behavior?

@mscdexmscdexFeb 18, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@mscdex
mscdexforce-pushed the http-use-semicolon-for-cookie-header branch from def8d7d to 042930aCompareFebruary 18, 2017 21:05
@mscdex

Copy link
Copy Markdown
ContributorAuthor

ping @nodejs/ctc

@bnoordhuisbnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM technically but I'm -0.5 on rewriting outgoing headers, I don't think that's core business.

@mscdex

Copy link
Copy Markdown
ContributorAuthor

Any further comments/suggestions/etc. before landing, @nodejs/collaborators?

@mscdex
mscdexforce-pushed the http-use-semicolon-for-cookie-header branch 2 times, most recently from a1dc0a4 to b759843CompareMarch 9, 2017 12:18
@mscdex

mscdex commented Mar 9, 2017

Copy link
Copy Markdown
ContributorAuthor

CI once more before landing: https://ci.nodejs.org/job/node-test-pull-request/6764/

mscdex added 2 commits March 9, 2017 08:10
Previously, separate incoming Cookie headers would be concatenated
with a comma, which can cause confusion in userland code when it
comes to parsing the final Cookie header value. This commit
concatenates using a semicolon instead.
Fixes: nodejs#11256
PR-URL: nodejs#11259
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit enables automatic concatenation of multiple Cookie header
values with a semicolon, except when 2D header arrays are used.
Fixes: nodejs#11256
PR-URL: nodejs#11259
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mscdex
mscdexforce-pushed the http-use-semicolon-for-cookie-header branch from b759843 to d348077CompareMarch 9, 2017 13:10
@mscdex
mscdex merged commit d348077 into nodejs:masterMar 9, 2017
@mscdex
mscdex deleted the http-use-semicolon-for-cookie-header branch March 9, 2017 13:54
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
Previously, separate incoming Cookie headers would be concatenated
with a comma, which can cause confusion in userland code when it
comes to parsing the final Cookie header value. This commit
concatenates using a semicolon instead.
Fixes: nodejs#11256
PR-URL: nodejs#11259
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
This commit enables automatic concatenation of multiple Cookie header
values with a semicolon, except when 2D header arrays are used.
Fixes: nodejs#11256
PR-URL: nodejs#11259
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jasnelljasnell mentioned this pull request Apr 4, 2017
jasnell added a commit to jasnell/node that referenced this pull request May 29, 2017
* **Async Hooks**
* The `async_hooks` module has landed in core
[[`4a7233c178`](nodejs@4a7233c178)]
[nodejs#12892](nodejs#12892).
* **Buffer**
* Using the `--pending-deprecation` flag will cause Node.js to emit a
deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
[[`d2d32ea5a2`](nodejs@d2d32ea5a2)]
[nodejs#11968](nodejs#11968).
* `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
[[`7eb1b4658e`](nodejs@7eb1b4658e)]
[nodejs#12141](nodejs#12141).
* Many `Buffer` methods now accept `Uint8Array` as input
[[`beca3244e2`](nodejs@beca3244e2)]
[nodejs#10236](nodejs#10236).
* **Child Process**
* Argument and kill signal validations have been improved
[[`97a77288ce`](nodejs@97a77288ce)]
[nodejs#12348](nodejs#12348),
[[`d75fdd96aa`](nodejs@d75fdd96aa)]
[nodejs#10423](nodejs#10423).
* Child Process methods accept `Uint8Array` as input
[[`627ecee9ed`](nodejs@627ecee9ed)]
[nodejs#10653](nodejs#10653).
* **Console**
* Error events emitted when using `console` methods are now supressed.
[[`f18e08d820`](nodejs@f18e08d820)]
[nodejs#9744](nodejs#9744).
* **Dependencies**
* The npm client has been updated to 5.0.0
[[`3c3b36af0f`](nodejs@3c3b36af0f)]
[nodejs#12936](nodejs#12936).
* V8 has been updated to 5.8 with forward ABI stability to 6.0
[[`60d1aac8d2`](nodejs@60d1aac8d2)]
[nodejs#12784](nodejs#12784).
* **Domains**
* Native `Promise` instances are now `Domain` aware
[[`84dabe8373`](nodejs@84dabe8373)]
[nodejs#12489](nodejs#12489).
* **Errors**
* We have started assigning static error codes to errors generated by Node.js.
This has been done through multiple commits and is still a work in
progress.
* **File System**
* The utility class `fs.SyncWriteStream` has been deprecated
[[`7a55e34ef4`](nodejs@7a55e34ef4)]
[nodejs#10467](nodejs#10467).
* The deprecated `fs.read()` string interface has been removed
[[`3c2a9361ff`](nodejs@3c2a9361ff)]
[nodejs#9683](nodejs#9683).
* **HTTP**
* Improved support for userland implemented Agents
[[`90403dd1d0`](nodejs@90403dd1d0)]
[nodejs#11567](nodejs#11567).
* Outgoing Cookie headers are concatenated into a single string
[[`d3480776c7`](nodejs@d3480776c7)]
[nodejs#11259](nodejs#11259).
* The `httpResponse.writeHeader()` method has been deprecated
[[`fb71ba4921`](nodejs@fb71ba4921)]
[nodejs#11355](nodejs#11355).
* New methods for accessing HTTP headers have been added to `OutgoingMessage`
[[`3e6f1032a4`](nodejs@3e6f1032a4)]
[nodejs#10805](nodejs#10805).
* **Lib**
* All deprecation messages have been assigned static identifiers
[[`5de3cf099c`](nodejs@5de3cf099c)]
[nodejs#10116](nodejs#10116).
* The legacy `linkedlist` module has been removed
[[`84a23391f6`](nodejs@84a23391f6)]
[nodejs#12113](nodejs#12113).
* **N-API**
* Experimental support for the new N-API API has been added
[[`56e881d0b0`](nodejs@56e881d0b0)]
[nodejs#11975](nodejs#11975).
* **Process**
* Process warning output can be redirected to a file using the
`--redirect-warnings` command-line argument
[[`03e89b3ff2`](nodejs@03e89b3ff2)]
[nodejs#10116](nodejs#10116).
* Process warnings may now include additional detail
[[`dd20e68b0f`](nodejs@dd20e68b0f)]
[nodejs#12725](nodejs#12725).
* **REPL**
* REPL magic mode has been deprecated
[[`3f27f02da0`](nodejs@3f27f02da0)]
[nodejs#11599](nodejs#11599).
* **Src**
* `NODE_MODULE_VERSION` has been updated to 57
(nodejs@ec7cbaf266)]
[nodejs#12995](nodejs#12995).
* Add `--pending-deprecation` command-line argument and
`NODE_PENDING_DEPRECATION` environment variable
[[`a16b570f8c`](nodejs@a16b570f8c)]
[nodejs#11968](nodejs#11968).
* The `--debug` command-line argument has been deprecated. Note that
using `--debug` will enable the *new* Inspector-based debug protocol
as the legacy Debugger protocol previously used by Node.js has been
removed. [[`010f864426`](nodejs@010f864426)]
[nodejs#12949](nodejs#12949).
* Throw when the `-c` and `-e` command-line arguments are used at the same
time [[`a5f91ab230`](nodejs@a5f91ab230)]
[nodejs#11689](nodejs#11689).
* Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
arguments are used at the same time.
[[`8a7db9d4b5`](nodejs@8a7db9d4b5)]
[nodejs#12087](nodejs#12087).
* **Stream**
* `Stream` now supports `destroy()` and `_destroy()` APIs
[[`b6e1d22fa6`](nodejs@b6e1d22fa6)]
[nodejs#12925](nodejs#12925).
* `Stream` now supports the `_final()` API
[[`07c7f198db`](nodejs@07c7f198db)]
[nodejs#12828](nodejs#12828).
* **TLS**
* The `rejectUnauthorized` option now defaults to `true`
[[`348cc80a3c`](nodejs@348cc80a3c)]
[nodejs#5923](nodejs#5923).
* The `tls.createSecurePair()` API now emits a runtime deprecation
[[`a2ae08999b`](nodejs@a2ae08999b)]
[nodejs#11349](nodejs#11349).
* A runtime deprecation will now be emitted when `dhparam` is less than
2048 bits [[`d523eb9c40`](nodejs@d523eb9c40)]
[nodejs#11447](nodejs#11447).
* **URL**
* The WHATWG URL implementation is now a fully-supported Node.js API
[[`d080ead0f9`](nodejs@d080ead0f9)]
[nodejs#12710](nodejs#12710).
* **Util**
* `Symbol` keys are now displayed by default when using `util.inspect()`
[[`5bfd13b81e`](nodejs@5bfd13b81e)]
[nodejs#9726](nodejs#9726).
* `toJSON` errors will be thrown when formatting `%j`
[[`455e6f1dd8`](nodejs@455e6f1dd8)]
[nodejs#11708](nodejs#11708).
* Convert `inspect.styles` and `inspect.colors` to prototype-less objects
[[`aab0d202f8`](nodejs@aab0d202f8)]
[nodejs#11624](nodejs#11624).
* The new `util.promisify()` API has been added
[[`99da8e8e02`](nodejs@99da8e8e02)]
[nodejs#12442](nodejs#12442).
* **Zlib**
* Support `Uint8Array` in Zlib convenience methods
[[`91383e47fd`](nodejs@91383e47fd)]
[nodejs#12001](nodejs#12001).
* Zlib errors now use `RangeError` and `TypeError` consistently
[[`b514bd231e`](nodejs@b514bd231e)]
[nodejs#11391](nodejs#11391).
jasnell added a commit that referenced this pull request May 30, 2017
* **Async Hooks**
* The `async_hooks` module has landed in core
[[`4a7233c178`](4a7233c178)]
[#12892](#12892).
* **Buffer**
* Using the `--pending-deprecation` flag will cause Node.js to emit a
deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
[[`d2d32ea5a2`](d2d32ea5a2)]
[#11968](#11968).
* `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
[[`7eb1b4658e`](7eb1b4658e)]
[#12141](#12141).
* Many `Buffer` methods now accept `Uint8Array` as input
[[`beca3244e2`](beca3244e2)]
[#10236](#10236).
* **Child Process**
* Argument and kill signal validations have been improved
[[`97a77288ce`](97a77288ce)]
[#12348](#12348),
[[`d75fdd96aa`](d75fdd96aa)]
[#10423](#10423).
* Child Process methods accept `Uint8Array` as input
[[`627ecee9ed`](627ecee9ed)]
[#10653](#10653).
* **Console**
* Error events emitted when using `console` methods are now supressed.
[[`f18e08d820`](f18e08d820)]
[#9744](#9744).
* **Dependencies**
* The npm client has been updated to 5.0.0
[[`3c3b36af0f`](3c3b36af0f)]
[#12936](#12936).
* V8 has been updated to 5.8 with forward ABI stability to 6.0
[[`60d1aac8d2`](60d1aac8d2)]
[#12784](#12784).
* **Domains**
* Native `Promise` instances are now `Domain` aware
[[`84dabe8373`](84dabe8373)]
[#12489](#12489).
* **Errors**
* We have started assigning static error codes to errors generated by Node.js.
This has been done through multiple commits and is still a work in
progress.
* **File System**
* The utility class `fs.SyncWriteStream` has been deprecated
[[`7a55e34ef4`](7a55e34ef4)]
[#10467](#10467).
* The deprecated `fs.read()` string interface has been removed
[[`3c2a9361ff`](3c2a9361ff)]
[#9683](#9683).
* **HTTP**
* Improved support for userland implemented Agents
[[`90403dd1d0`](90403dd1d0)]
[#11567](#11567).
* Outgoing Cookie headers are concatenated into a single string
[[`d3480776c7`](d3480776c7)]
[#11259](#11259).
* The `httpResponse.writeHeader()` method has been deprecated
[[`fb71ba4921`](fb71ba4921)]
[#11355](#11355).
* New methods for accessing HTTP headers have been added to `OutgoingMessage`
[[`3e6f1032a4`](3e6f1032a4)]
[#10805](#10805).
* **Lib**
* All deprecation messages have been assigned static identifiers
[[`5de3cf099c`](5de3cf099c)]
[#10116](#10116).
* The legacy `linkedlist` module has been removed
[[`84a23391f6`](84a23391f6)]
[#12113](#12113).
* **N-API**
* Experimental support for the new N-API API has been added
[[`56e881d0b0`](56e881d0b0)]
[#11975](#11975).
* **Process**
* Process warning output can be redirected to a file using the
`--redirect-warnings` command-line argument
[[`03e89b3ff2`](03e89b3ff2)]
[#10116](#10116).
* Process warnings may now include additional detail
[[`dd20e68b0f`](dd20e68b0f)]
[#12725](#12725).
* **REPL**
* REPL magic mode has been deprecated
[[`3f27f02da0`](3f27f02da0)]
[#11599](#11599).
* **Src**
* `NODE_MODULE_VERSION` has been updated to 57
(ec7cbaf266)]
[#12995](#12995).
* Add `--pending-deprecation` command-line argument and
`NODE_PENDING_DEPRECATION` environment variable
[[`a16b570f8c`](a16b570f8c)]
[#11968](#11968).
* The `--debug` command-line argument has been deprecated. Note that
using `--debug` will enable the *new* Inspector-based debug protocol
as the legacy Debugger protocol previously used by Node.js has been
removed. [[`010f864426`](010f864426)]
[#12949](#12949).
* Throw when the `-c` and `-e` command-line arguments are used at the same
time [[`a5f91ab230`](a5f91ab230)]
[#11689](#11689).
* Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
arguments are used at the same time.
[[`8a7db9d4b5`](8a7db9d4b5)]
[#12087](#12087).
* **Stream**
* `Stream` now supports `destroy()` and `_destroy()` APIs
[[`b6e1d22fa6`](b6e1d22fa6)]
[#12925](#12925).
* `Stream` now supports the `_final()` API
[[`07c7f198db`](07c7f198db)]
[#12828](#12828).
* **TLS**
* The `rejectUnauthorized` option now defaults to `true`
[[`348cc80a3c`](348cc80a3c)]
[#5923](#5923).
* The `tls.createSecurePair()` API now emits a runtime deprecation
[[`a2ae08999b`](a2ae08999b)]
[#11349](#11349).
* A runtime deprecation will now be emitted when `dhparam` is less than
2048 bits [[`d523eb9c40`](d523eb9c40)]
[#11447](#11447).
* **URL**
* The WHATWG URL implementation is now a fully-supported Node.js API
[[`d080ead0f9`](d080ead0f9)]
[#12710](#12710).
* **Util**
* `Symbol` keys are now displayed by default when using `util.inspect()`
[[`5bfd13b81e`](5bfd13b81e)]
[#9726](#9726).
* `toJSON` errors will be thrown when formatting `%j`
[[`455e6f1dd8`](455e6f1dd8)]
[#11708](#11708).
* Convert `inspect.styles` and `inspect.colors` to prototype-less objects
[[`aab0d202f8`](aab0d202f8)]
[#11624](#11624).
* The new `util.promisify()` API has been added
[[`99da8e8e02`](99da8e8e02)]
[#12442](#12442).
* **Zlib**
* Support `Uint8Array` in Zlib convenience methods
[[`91383e47fd`](91383e47fd)]
[#12001](#12001).
* Zlib errors now use `RangeError` and `TypeError` consistently
[[`b514bd231e`](b514bd231e)]
[#11391](#11391).
jasnell added a commit that referenced this pull request May 30, 2017
* **Async Hooks**
* The `async_hooks` module has landed in core
[[`4a7233c178`](4a7233c178)]
[#12892](#12892).
* **Buffer**
* Using the `--pending-deprecation` flag will cause Node.js to emit a
deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
[[`d2d32ea5a2`](d2d32ea5a2)]
[#11968](#11968).
* `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
[[`7eb1b4658e`](7eb1b4658e)]
[#12141](#12141).
* Many `Buffer` methods now accept `Uint8Array` as input
[[`beca3244e2`](beca3244e2)]
[#10236](#10236).
* **Child Process**
* Argument and kill signal validations have been improved
[[`97a77288ce`](97a77288ce)]
[#12348](#12348),
[[`d75fdd96aa`](d75fdd96aa)]
[#10423](#10423).
* Child Process methods accept `Uint8Array` as input
[[`627ecee9ed`](627ecee9ed)]
[#10653](#10653).
* **Console**
* Error events emitted when using `console` methods are now supressed.
[[`f18e08d820`](f18e08d820)]
[#9744](#9744).
* **Dependencies**
* The npm client has been updated to 5.0.0
[[`3c3b36af0f`](3c3b36af0f)]
[#12936](#12936).
* V8 has been updated to 5.8 with forward ABI stability to 6.0
[[`60d1aac8d2`](60d1aac8d2)]
[#12784](#12784).
* **Domains**
* Native `Promise` instances are now `Domain` aware
[[`84dabe8373`](84dabe8373)]
[#12489](#12489).
* **Errors**
* We have started assigning static error codes to errors generated by Node.js.
This has been done through multiple commits and is still a work in
progress.
* **File System**
* The utility class `fs.SyncWriteStream` has been deprecated
[[`7a55e34ef4`](7a55e34ef4)]
[#10467](#10467).
* The deprecated `fs.read()` string interface has been removed
[[`3c2a9361ff`](3c2a9361ff)]
[#9683](#9683).
* **HTTP**
* Improved support for userland implemented Agents
[[`90403dd1d0`](90403dd1d0)]
[#11567](#11567).
* Outgoing Cookie headers are concatenated into a single string
[[`d3480776c7`](d3480776c7)]
[#11259](#11259).
* The `httpResponse.writeHeader()` method has been deprecated
[[`fb71ba4921`](fb71ba4921)]
[#11355](#11355).
* New methods for accessing HTTP headers have been added to `OutgoingMessage`
[[`3e6f1032a4`](3e6f1032a4)]
[#10805](#10805).
* **Lib**
* All deprecation messages have been assigned static identifiers
[[`5de3cf099c`](5de3cf099c)]
[#10116](#10116).
* The legacy `linkedlist` module has been removed
[[`84a23391f6`](84a23391f6)]
[#12113](#12113).
* **N-API**
* Experimental support for the new N-API API has been added
[[`56e881d0b0`](56e881d0b0)]
[#11975](#11975).
* **Process**
* Process warning output can be redirected to a file using the
`--redirect-warnings` command-line argument
[[`03e89b3ff2`](03e89b3ff2)]
[#10116](#10116).
* Process warnings may now include additional detail
[[`dd20e68b0f`](dd20e68b0f)]
[#12725](#12725).
* **REPL**
* REPL magic mode has been deprecated
[[`3f27f02da0`](3f27f02da0)]
[#11599](#11599).
* **Src**
* `NODE_MODULE_VERSION` has been updated to 57
(ec7cbaf266)]
[#12995](#12995).
* Add `--pending-deprecation` command-line argument and
`NODE_PENDING_DEPRECATION` environment variable
[[`a16b570f8c`](a16b570f8c)]
[#11968](#11968).
* The `--debug` command-line argument has been deprecated. Note that
using `--debug` will enable the *new* Inspector-based debug protocol
as the legacy Debugger protocol previously used by Node.js has been
removed. [[`010f864426`](010f864426)]
[#12949](#12949).
* Throw when the `-c` and `-e` command-line arguments are used at the same
time [[`a5f91ab230`](a5f91ab230)]
[#11689](#11689).
* Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
arguments are used at the same time.
[[`8a7db9d4b5`](8a7db9d4b5)]
[#12087](#12087).
* **Stream**
* `Stream` now supports `destroy()` and `_destroy()` APIs
[[`b6e1d22fa6`](b6e1d22fa6)]
[#12925](#12925).
* `Stream` now supports the `_final()` API
[[`07c7f198db`](07c7f198db)]
[#12828](#12828).
* **TLS**
* The `rejectUnauthorized` option now defaults to `true`
[[`348cc80a3c`](348cc80a3c)]
[#5923](#5923).
* The `tls.createSecurePair()` API now emits a runtime deprecation
[[`a2ae08999b`](a2ae08999b)]
[#11349](#11349).
* A runtime deprecation will now be emitted when `dhparam` is less than
2048 bits [[`d523eb9c40`](d523eb9c40)]
[#11447](#11447).
* **URL**
* The WHATWG URL implementation is now a fully-supported Node.js API
[[`d080ead0f9`](d080ead0f9)]
[#12710](#12710).
* **Util**
* `Symbol` keys are now displayed by default when using `util.inspect()`
[[`5bfd13b81e`](5bfd13b81e)]
[#9726](#9726).
* `toJSON` errors will be thrown when formatting `%j`
[[`455e6f1dd8`](455e6f1dd8)]
[#11708](#11708).
* Convert `inspect.styles` and `inspect.colors` to prototype-less objects
[[`aab0d202f8`](aab0d202f8)]
[#11624](#11624).
* The new `util.promisify()` API has been added
[[`99da8e8e02`](99da8e8e02)]
[#12442](#12442).
* **Zlib**
* Support `Uint8Array` in Zlib convenience methods
[[`91383e47fd`](91383e47fd)]
[#12001](#12001).
* Zlib errors now use `RangeError` and `TypeError` consistently
[[`b514bd231e`](b514bd231e)]
[#11391](#11391).
HaroldPutman pushed a commit to HaroldPutman/node-simplecrawler that referenced this pull request Sep 20, 2017
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues or PRs related to the http subsystem.semver-majorPRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@mscdex@jasnell@kunalspathak@isaacs@sam-github@mcollina@dougwilson@silverwind@bnoordhuis@thefourtheye@nodejs-github-bot