Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix overwriting of session.cookie, change tests to use uniform data with real cookie instance #77

Merged
merged 2 commits into from Jul 23, 2013

Conversation

lazd
Copy link
Contributor

@lazd lazd commented Jul 10, 2013

This PR fixes #76 by making the default _serialize_session function copy session properties to a new object and handle converting the cookie instance to an object without changing the original session.

In addition, tests have been changed to use uniform test data that includes an instance of connect.session.Cookie and each test now checks for deep equality between the input data and the session data in a more consistent fashion. This makes tests more real-world as the data is more representative of actual data that will be encountered.

@lukaszfiszer
Copy link

Does this fixed the bug mentioned by@mmagi in #63 (TypeError: Object #<Object> has no method 'serialize')? If yes, 👍 for this PR.

The most current npm version (0.3.3) is broken, so I would love to see this merged and pushed to npm asap.

@lazd
Copy link
Contributor Author

lazd commented Jul 18, 2013

@lukaszfiszer Yes, it fixes that bug (which one of my previous PRs introduced). See #76 for more info.

@lukaszfiszer
Copy link

Great! Thank for info

kcbanner added a commit that referenced this pull request Jul 23, 2013
Fix overwriting of session.cookie, change tests to use uniform data with real cookie instance
@kcbanner kcbanner merged commit 33767b6 into jdesboeufs:master Jul 23, 2013
@kcbanner
Copy link
Collaborator

Thanks for this. I'll bump the version and publish soon.

@lazd
Copy link
Contributor Author

lazd commented Sep 26, 2013

@kcbanner, can you bump the version and publish this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

session.set overwrites cookie property of session
3 participants