Skip to content

Commit

Permalink
Ignore expires and maxAge in res.clearCookie() (#5792)
Browse files Browse the repository at this point in the history
* add test for removing user provided expires

* rework impl and tests to ignore maxAge, do not set it

this is to take into account the built-in relative expires when passing
a maxAge to res.cookie

I realized that using maxAge to invalidate cookies inherrently hit this
relativee expires behavior, and the goal of this PR is not to rework
that relative expires behavior w/ maxAge, but to prevent users from
overwriting these values by accident when clearing cookies

* update history.md

* explicitly delete maxAge instead of setting as undefined

* drop the spread, use object.assign

* wording, review comment on history.md

Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>

* ♻️ use spread, update supported ecmascript version

---------

Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>
  • Loading branch information
jonchurch and ctcpip authored Aug 2, 2024
1 parent 160b91c commit 82fc12a
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
root: true
env:
es6: true
es2022: true
node: true
rules:
eol-last: error
Expand Down
2 changes: 2 additions & 0 deletions History.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ unreleased
* `res.status()` accepts only integers, and input must be greater than 99 and less than 1000
* will throw a `RangeError: Invalid status code: ${code}. Status code must be greater than 99 and less than 1000.` for inputs outside this range
* will throw a `TypeError: Invalid status code: ${code}. Status code must be an integer.` for non integer inputs
* change:
- `res.clearCookie` will ignore user provided `maxAge` and `expires` options

5.0.0-beta.3 / 2024-03-25
=========================
Expand Down
5 changes: 4 additions & 1 deletion lib/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,10 @@ res.get = function(field){
*/

res.clearCookie = function clearCookie(name, options) {
var opts = merge({ expires: new Date(1), path: '/' }, options);
// Force cookie expiration by setting expires to the past
const opts = { path: '/', ...options, expires: new Date(1)};
// ensure maxAge is not passed
delete opts.maxAge

return this.cookie(name, '', opts);
};
Expand Down
26 changes: 26 additions & 0 deletions test/res.clearCookie.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,31 @@ describe('res', function(){
.expect('Set-Cookie', 'sid=; Path=/admin; Expires=Thu, 01 Jan 1970 00:00:00 GMT')
.expect(200, done)
})

it('should ignore maxAge', function(done){
var app = express();

app.use(function(req, res){
res.clearCookie('sid', { path: '/admin', maxAge: 1000 }).end();
});

request(app)
.get('/')
.expect('Set-Cookie', 'sid=; Path=/admin; Expires=Thu, 01 Jan 1970 00:00:00 GMT')
.expect(200, done)
})

it('should ignore user supplied expires param', function(done){
var app = express();

app.use(function(req, res){
res.clearCookie('sid', { path: '/admin', expires: new Date() }).end();
});

request(app)
.get('/')
.expect('Set-Cookie', 'sid=; Path=/admin; Expires=Thu, 01 Jan 1970 00:00:00 GMT')
.expect(200, done)
})
})
})

0 comments on commit 82fc12a

Please sign in to comment.