Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Session middleware bug while used by Flatiron #646

Closed
maiah opened this Issue Aug 19, 2012 · 6 comments

Comments

Projects
None yet
3 participants

maiah commented Aug 19, 2012

Hi Connect team,

I created Flatiron app and used Connect's Session middleware to have a basic sample of authentication w/ it:
https://github.com/maiah/flatiron-auth

But when I try to use Connect's session.js middleware with Flatiron I get the error below:

d:\tools\flatiron-auth\node_modules\connect\lib\middleware\session.js:213
if (0 != req.originalUrl.indexOf(cookie.path || '/')) return next();
^
TypeError: Cannot call method 'indexOf' of undefined
at Array.session as 3
at dispatch (d:\tools\node_modules\union\lib\routing-stream.js:110:21)
at g (events.js:185:14)
at EventEmitter.emit (events.js:85:17)
at RoutingStream.route (d:\tools\node_modules\union\lib\routing-stream.js:114:23)
at Array.cookieParser as 2
at dispatch (d:\tools\node_modules\union\lib\routing-stream.js:110:21)
at g (events.js:185:14)
at EventEmitter.emit (events.js:85:17)
at RoutingStream.route (d:\tools\node_modules\union\lib\routing-stream.js:114:23)

I checked session.js file and go to line 213 as what the error indicates and noticed that it is accessing "originalUrl" method of "req" object which is not supported by Flatiron. So to fix this I changed the if-statement with the one below.

[session.js line 213]

Before:
if (0 != req.originalUrl.indexOf(cookie.path || '/')) return next();

After:
if (req.originalUrl !== undefined && 0 != req.originalUrl.indexOf(cookie.path || '/')) return next();

This fix works well now w/ Flatiron because Flatiron doesn't support "originalUrl" method in its "req" object, but I do understand that Express framework supports this. But the thing is the session.js middleware seems to be biased for Express framework only rather than being a generic middleware for Node.

Do you think the code change is correct? If this is correct can I push this to Connect's master branch so that everyone can use session.js easily w/ Flatiron framework?

Thanks!
Maiah

Member

tj commented Aug 19, 2012

req.originalUrl isn't an express thing, it's a Connect thing, and they are necessary

@tj tj closed this Aug 19, 2012

maiah commented Aug 20, 2012

If that's the case how can I configure Flatiron server to have originalUrl property in req object?

Member

tj commented Aug 20, 2012

req.originalUrl = req.url;

in the case of Connect this just ensures that any modifications to req.url can be ignored, which is useful in many cases (logging etc)

maiah commented Aug 20, 2012

I see. I thought of this solution also and I just thought that this little configuration seems very specific. And it might be good to document this in Session's API.
Thanks!

Member

tj commented Aug 20, 2012

it's not specific to sessions at all, lots of features use it

kilianc commented Oct 5, 2012

Shouldn't it mentioned into docs? Couldn't the middleware check for originalUrl existence?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment