-
Notifications
You must be signed in to change notification settings - Fork 85
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
Supporting wss:// and https:// protocol #55
Comments
@snowmantw Thanks for the patch. Some thoughts:
|
@dhruvbird any reason why TLS processing would be particularly heavy for node-xmpp-bosh/Node.js? |
@sonnyp My reasons are not founded in numbers (they should ideally be), but it seems that people have casually mentioned that TLS processing is heavy. This is especially bad if you're running on a single CPU (which we are). Plus, I would expect a production deployment to have nginx or some other reverse proxy to handle gzip and TLS termination. Plus it makes it easy to manage TLS certs. if done that way rather than having everyone update SSL certs. at the node.js application layer. Hey, I could be totally wrong, and these might not be valid deployment concerns - just saying based on conversations I've had with the ops. folks at my previous org. |
@dhruvbird We proxy the traffic using haproxy, I was interested in technical implications and your arguments against performances make sense. |
@sonnyp (y) - let me know if you have any numbers online suggesting that it is a bad idea since profiling before concluding is a good idea. We didn't have a pressing need to support TLS on http/ws since we were deploying stuff behind nginx/elastic-LB since https termination almost always happened elsewhere... Good to know that you guys also use a proxy in front of node.js. |
I did some modifications in my forked project, but they're all dirty hack. So, maybe @dhruvbird can support them officially ?
What I modified:
The text was updated successfully, but these errors were encountered: