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

Don't drop query variables on handshake #2745

Merged
merged 3 commits into from Nov 16, 2016
Merged

Don't drop query variables on handshake #2745

merged 3 commits into from Nov 16, 2016

Conversation

perrin4869
Copy link
Contributor

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

Right now parameters passed during handshake, such as tokens, are being dropped.

New behaviour

Parameters are preserved as intended

Other information (e.g. related issues)

Fixes #2712

"has-binary": "0.1.7",
"debug": "2.2.0"
"object-assign": "^4.1.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please pin the dependency here please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Missed it sorry!

@darrachequesne darrachequesne merged commit 4c5dbd8 into socketio:master Nov 16, 2016
@jasperck
Copy link

Hi, just wanna ask why require object-assign usage rather than native object.assign,
is there any issue about shallow/deep clone? or just because the polyfill

@perrin4869
Copy link
Contributor Author

@jasperck node 0.10 and 0.12 are still being supported and they don't have native Object.assign. You can check Travis build logs, my first commit here used the native one and it passed for node 4 and 6 but failed for the older node... Hence why I introduced the polyfill. Maybe a note should be made to remove the dependency once node older than 4.0 stops being supported.

@jasperck
Copy link

@perrin4869 Thanks for the feedback. Yep I saw the build and got it now. Adding a note for removing the dependency in the future sounds good.

@perrin4869
Copy link
Contributor Author

@jasperck maybe opening an issue as a remainder might do it, but considering the number of open issues right now, it would just get buried and forgotten...
@darrachequesne not to sound too impatient, but can we expect a patched release sometime soon? Thanks!

@mck-
Copy link

mck- commented Nov 19, 2016

Yes please! This is breaking for us too 😄

@darrachequesne darrachequesne added this to the 1.6.0 milestone Nov 20, 2016
@darrachequesne
Copy link
Member

1.6.0 is out!

Regarding node 0.10 and 0.12, I think we'll deprecate those version in the next major bump (which will be eventually needed if we want to update ws).

@mck-
Copy link

mck- commented Nov 21, 2016

Excellent – thanks! I'll test it later today 👍 that was very fast 👏

@mck-
Copy link

mck- commented Nov 21, 2016

Works great 👌

dzad pushed a commit to dzad/socket.io that referenced this pull request May 29, 2023
Parameters passed during handshake, such as tokens, were being dropped.
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.

socket.io 1.5.0 backwards incompatible with socket.io-client < 1.5.0
5 participants