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

Support compression #299

Merged
merged 2 commits into from
Jan 21, 2015
Merged

Support compression #299

merged 2 commits into from
Jan 21, 2015

Conversation

nkzawa
Copy link
Contributor

@nkzawa nkzawa commented Dec 22, 2014

Add the permessage-deflate extension support for WebSocket, and http compression for polling. They are enabled by default.
Also you can disable the compression per message through the option of send method.

Related: socketio/socket.io#1148, #298 #271

@nkzawa
Copy link
Contributor Author

nkzawa commented Dec 22, 2014

It seems we should have a minimum size threshold for http compression, since compressing small data makes them larger.

@leearmstrong
Copy link

When will this make it into the live SocketIO code? Anyway to use it now?

@defunctzombie
Copy link
Contributor

@nkzawa Why wouldn't we just let the proxy handle this?

@nkzawa
Copy link
Contributor Author

nkzawa commented Dec 31, 2014

@defunctzombie mainly because we want to turn the compression on/off per message(payload) to emulate the permessage-deflate extension of WebSocket. I think we can't support this with proxy.
And also we can't use Content-Type header to decide whether to compress data or not like a http server.

@3rd-Eden
Copy link
Contributor

@defunctzombie you can still have your proxy handle this by turning compression off in engine.io right?

@nkzawa
Copy link
Contributor Author

nkzawa commented Jan 4, 2015

@lpinca updated, thx!

@@ -212,6 +212,11 @@ to a single process.
to (`['polling', 'websocket']`)
- `allowUpgrades` (`Boolean`): whether to allow transport upgrades
(`true`)
- `perMessageDeflate` (`Object|Boolean`): paramters of the WebSocket permessage-deflate extension
(see [ws module](https://github.com/einaros/ws) api docs). Set to `false` to disable. (`true`)
- `httpCompression` (`Object|Boolean`): paramters of the http compression for the polling transports
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

@leearmstrong
Copy link

Any ETA when this will be merged. This could be huge!!

@nkzawa
Copy link
Contributor Author

nkzawa commented Jan 5, 2015

@Leeus I'm working to reflect code review. Maybe this will be merged after that if there is no problem anymore.

@nkzawa
Copy link
Contributor Author

nkzawa commented Jan 5, 2015

@rauchg fixed and rebased. could you check again?

@rauchg
Copy link
Contributor

rauchg commented Jan 5, 2015

When we call sendPacket manually, should the compress flag be undefined so that it obtains it from the default provided by the configuration in the constructor?

Otherwise, it seems that if we disable compression globally, it'll still force to true when we send non-user packets (like noop)

@rauchg
Copy link
Contributor

rauchg commented Jan 5, 2015

Before, it seems that it was always defaulting to compress: true if it was undefined?

@nkzawa
Copy link
Contributor Author

nkzawa commented Jan 5, 2015

compress: true for sendPacket(send) is just one of the conditions, so it's ignored when compression is disabled globally and when client doesn't support compression.

@defunctzombie defunctzombie mentioned this pull request Jan 9, 2015
@lpinca lpinca mentioned this pull request Jan 17, 2015
@rauchg
Copy link
Contributor

rauchg commented Jan 19, 2015

@nkzawa please rebase so I can merge.

@nkzawa
Copy link
Contributor Author

nkzawa commented Jan 21, 2015

@rauchg rebased

rauchg added a commit that referenced this pull request Jan 21, 2015
@rauchg rauchg merged commit ddc64a2 into socketio:master Jan 21, 2015
darrachequesne pushed a commit that referenced this pull request May 8, 2020
@darrachequesne darrachequesne mentioned this pull request Oct 5, 2020
2 tasks
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.

6 participants