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

Allow custom session ids #250

Merged
merged 1 commit into from
Jul 6, 2015
Merged

Conversation

imkira
Copy link
Contributor

@imkira imkira commented Jul 2, 2015

Every time a new connection is made to a sockjs server, a session id is
generated in the client and sent to the server. This session id helps
the server distinguish connections. By default, the current code
generates a random 8-character-long string as session id. There is
currently no way to increase the entropy or even generate your own
session ids, so this commit is an attempt to tackle that limitation.

@brycekahle
Copy link
Contributor

What do you think about having a single option sessionId that takes either a number or a function?

@imkira
Copy link
Contributor Author

imkira commented Jul 3, 2015

Thank you for your prompt reply.
Yeah, it looks good to me. What do you say about:

  • if it is a "number", then we use it as the number of characters of the string to generate using the default random function
  • if it is a "function", then we call it and use the result as is
  • if it is of type "string" then we use it as is (although session ids should change between different sessions, would implementing this make sense to you?)
  • if none of the above cases apply, use the default random.string(8)

Let me clear this question before I change the code, test, and docs and commit/rebase again.

@brycekahle
Copy link
Contributor

number and function make sense to me. I would leave out string.

Every time a new connection is made to a sockjs server, a session id is
generated in the client and sent to the server. This session id helps
the server distinguish connections. By default, the current code
generates a random 8-character-long string as session id. There is
currently no way to increase the entropy or even generate your own
session ids, so this commit is an attempt to tackle that limitation by
specifying the option called sessionId in the options passed to SockJS.
@imkira imkira force-pushed the allowCustomSessionIds branch from 4f677b9 to e668284 Compare July 3, 2015 04:48
@imkira
Copy link
Contributor Author

imkira commented Jul 3, 2015

@brycekahle how does it look now

@brycekahle
Copy link
Contributor

Looks great!

brycekahle added a commit that referenced this pull request Jul 6, 2015
@brycekahle brycekahle merged commit 85cb659 into sockjs:master Jul 6, 2015
@imkira
Copy link
Contributor Author

imkira commented Jul 7, 2015

Thanks a lot

@imkira
Copy link
Contributor Author

imkira commented Jul 7, 2015

By the way sorry, I don't know much about the release cycle of sockjs-client. Can you tell me when is the next release that includes this commit? Thanks

@brycekahle
Copy link
Contributor

I can try and get a new version out this week.

@imkira
Copy link
Contributor Author

imkira commented Jul 8, 2015

Looking forward to!
Thanks a lot in advance!

@brycekahle
Copy link
Contributor

@imkira I just released 1.0.1

@imkira
Copy link
Contributor Author

imkira commented Jul 21, 2015

@brycekahle thanks a lot!

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.

2 participants