Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Check if request uses GET method before attempting to generate a csrf token #376

Closed
wants to merge 1 commit into from

4 participants

@EddieRingle

I was getting the following error on just about every request I made:

TypeError: Cannot read property '_csrf' of undefined
    at Object.handle (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/middleware/csrf.js:77:28)
    at next (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/http.js:201:15)
    at Object.session [as handle] (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/middleware/session.js:249:47)
    at next (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/http.js:201:15)
    at Object.cookieParser [as handle] (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/middleware/cookieParser.js:44:5)
    at next (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/http.js:201:15)
    at Object.bodyParser [as handle] (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/middleware/bodyParser.js:59:61)
    at next (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/http.js:201:15)
    at Object.handle (/home/eddie/dev/skatter.us/node_modules/stylus/lib/middleware.js:183:7)
    at next (/home/eddie/dev/skatter.us/node_modules/express/node_modules/connect/lib/http.js:201:15)
@EddieRingle EddieRingle Check if request uses GET method before attempting to generate a csrf…
… token

Fixes TypeError message complaining about reading from an undefined object

Signed-off-by: Eddie Ringle <eddie@eringle.net>
cdde78c
@tj
Owner
tj commented

was it because of the favicon?

@defunctzombie

Yea, if you don't have a favicon.ico file then it complains like the above. I had the same problem happen. Fixed it by just adding a favicon.ico.

@tj
Owner
tj commented

maybe we should default req.session to {} even when sessions are not active for that request, or I can remove /favicon.ico from the session() middleware so that it has sessions haha, seems to be a pretty common issue

@EddieRingle

Despite the cause, it's best to exit as early as possible in order to prevent cpu cycles from being wasted (i.e., generating a random string that won't even be used).

@tj
Owner
tj commented

we ignore validation on GET, not the token

@tj tj closed this
@EddieRingle

Ah right, that completely slipped my mind. Not sure what I was thinking when I made this change.

@tj
Owner
tj commented

no worries, I do that all the time haha

@c4milo

I can confirm that ^^

@tj
Owner
tj commented

ahahahah :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 24, 2011
  1. @EddieRingle

    Check if request uses GET method before attempting to generate a csrf…

    EddieRingle authored
    … token
    
    Fixes TypeError message complaining about reading from an undefined object
    
    Signed-off-by: Eddie Ringle <eddie@eringle.net>
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 4 deletions.
  1. +4 −4 lib/middleware/csrf.js
View
8 lib/middleware/csrf.js
@@ -73,12 +73,12 @@ module.exports = function csrf(options) {
, value = options.value || defaultValue;
return function(req, res, next){
- // generate CSRF token
- var token = req.session._csrf || (req.session._csrf = utils.uid(24));
-
// ignore GET (for now)
if ('GET' == req.method) return next();
+ // generate CSRF token
+ var token = req.session._csrf || (req.session._csrf = utils.uid(24));
+
// determine value
var val = value(req);
@@ -102,4 +102,4 @@ function defaultValue(req) {
return (req.body && req.body._csrf)
|| (req.query && req.query._csrf)
|| (req.headers['x-csrf-token']);
-}
+}
Something went wrong with that request. Please try again.