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

Websocket compression [DONT MERGE] #343

Closed
wants to merge 2 commits into from

Conversation

objectiveSee
Copy link

This Pull Request adds support for websocket compression, which was added to Socket.io in version 1.4. This is a basic implementation which still has some outstanding issues, therefore [DONT MERGE] was added to the title of the issue.

  • Basic support for Sec-WebSocket-Extensions header and the negotiation of the permessage-deflate extension.
  • Websocket compression using the zlib library. Required using an Objective-C helper class to interface with zlib as it wasn't possible or easy to use that library's C API from within Swift.

Remaining issues:

  • Use https://github.com/tidwall/DeflateSwift or similar instead of ObjC
  • Turn compression off by default and expose API to enable it.
  • Verify that response.bytesLeft -= Int(dataLength) is correct for handling partial frames. Test when extra data is present (ie. next frame)
  • Pick error code to use when compressed frames are unable to be decompressed.
  • Handle error cases and TODOs in the DeflateHelper class. There are also optimizations that can be done such as not building a new inflator with each packet.
  • Determine whether we should re-use the deflate context to achieve better compression. From Because the client and server may reuse their DEFLATE context between messages, the compression improves as more similar messages are sent over the connection. See https://blog.jcoglan.com/2015/03/30/websocket-extensions-as-plugins/ (Because the client and server may reuse their DEFLATE context between messages...)
  • Requires more testing.
  • Change version in Podspec

- Basic support for `Sec-WebSocket-Extensions` and the negotiation of the `permessage-deflate` extension. Several outstanding TODOs noted in the code.
- Websocket compression using the zlib library. Required using an Objective-C helper class to interface with zlib as it wasn't possible or easy to use that library's C API from within Swift.
@nuclearace
Copy link
Member

I'm not accepting anything that's in Objective-C. Should be rewritten in Swift. Also, this would need to go added to the upstream WebSocket, and then I'd update the dependency.

@nuclearace
Copy link
Member

daltoniam/Starscream#169

@objectiveSee
Copy link
Author

@nuclearace I'd be happy to port changes into starscream, though it would be better to just include that as a sub-pod. What's the current reason for not using starscream as a dependency on this pod?

@nuclearace
Copy link
Member

Because it makes it more difficult to support other package managers and installing manually more difficult. And also because I want to support swift3 ahead of time. https://github.com/socketio/socket.io-client-swift/blob/swift3/Source/WebSocket.swift and it makes it easier to make changes releated to that.

@nuclearace nuclearace closed this Jun 6, 2016
@yosiat
Copy link

yosiat commented May 15, 2017

@nuclearace Hi, do you still open for pull requests to add per message deflate support?

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.

3 participants