Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Set-Cookie Header getting corrupted because we end up with array inside array #893

Merged
merged 3 commits into from

4 participants

anuj123 fengmk2 jongleberry TJ Holowaychuk
anuj123

Prevents Set-Cookie Header from messing up with one Set-Cookie having multiple cookies because we end up with array inside array

fengmk2
anuj123

@fengmk2 thanks for pointing it out, I have pushed my test case

anuj123

if val is an array and prev is string we end up with ["prev", [val] ], This causes one Set-Cookie header to have multiple cookie values

jongleberry

ideally, this is something that should be fixed in node instead of connect. everything in patch.js should eventually be removed. do you know what the status of this in node is? this could be something to push for in 0.12 since they're overhauling HTTP/S

TJ Holowaychuk tj was assigned
jongleberry jonathanong merged commit 30d8025 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 12, 2013
  1. Set-Cookie Header have mupltile cookies as its value while prxiing fr…

    Anuj Thapliyal authored
    …om Node to Rails
Commits on Sep 13, 2013
  1. #893 Test cases for Set-Cookie header

    Anuj Thapliyal authored
Commits on Sep 16, 2013
This page is out of date. Refresh to see the latest.
Showing with 67 additions and 4 deletions.
  1. +7 −3 lib/patch.js
  2. +60 −1 test/patch.js
10 lib/patch.js
View
@@ -47,9 +47,13 @@ if (!res._hasConnectPatch) {
// special-case Set-Cookie
if (this._headers && 'set-cookie' == key) {
if (prev = this.getHeader(field)) {
- val = Array.isArray(prev)
- ? prev.concat(val)
- : [prev, val];
+ if (Array.isArray(prev)) {
+ val = prev.concat(val);
+ } else if (Array.isArray(val)) {
+ val = val.concat(prev);
+ } else {
+ val = [prev, val];
+ }
}
// charset
} else if ('content-type' == key && this.charset) {
61 test/patch.js
View
@@ -82,5 +82,64 @@ describe('patch', function(){
})
})
+ describe('should handle set-cookie setting multiple cookies as array', function(){
+ it('should be emitted', function(done){
+ var app = connect();
+
+ app.use(function(req, res, next){
+ res.on('header', function(){
+ res.setHeader('Set-Cookie', ['foo1', 'bar1']);
+ res.setHeader('Set-Cookie', ['foo2', 'bar2']);
+ });
+
+ next();
+ });
+
+ app.use(function(req, res){
+ res.end();
+ })
+
+ app.request()
+ .get('/')
+ .end(function(res){
+ res.headers['set-cookie'].should.include('foo1');
+ res.headers['set-cookie'].should.include('bar1');
+ res.headers['set-cookie'].should.include('foo2');
+ res.headers['set-cookie'].should.include('bar2');
+ done();
+ });
+ })
+ })
+
+ describe('should handle set-cookie setting multiple cookies as array and string', function(){
+ it('should be emitted', function(done){
+ var app = connect();
+
+ app.use(function(req, res, next){
+ res.on('header', function(){
+ res.setHeader('Set-Cookie', ['foo1', 'bar1']);
+ res.setHeader('Set-Cookie', 'foo2');
+ });
+
+ next();
+ });
+
+ app.use(function(req, res){
+ res.end();
+ })
+
+ app.request()
+ .get('/')
+ .end(function(res){
+ res.headers['set-cookie'].should.include('foo1');
+ res.headers['set-cookie'].should.include('bar1');
+ res.headers['set-cookie'].should.include('foo2');
+ done();
+ });
+ })
+ })
+
+
+
})
-})
+})
Something went wrong with that request. Please try again.