vapor-community / sockets

🔌 Non-blocking TCP socket layer, with event-driven server and client.
MIT License
575 stars 54 forks source link

Socks times out when receiving all data where the `messagedata.count % 512 == 0` #82

Closed Joannis closed 7 years ago

Joannis commented 8 years ago
extension TCPReadableSocket {

    public func recv(maxBytes: Int = BufferCapacity) throws -> [UInt8] {
        let data = Bytes(capacity: maxBytes)
        let flags: Int32 = 0 //FIXME: allow setting flags with a Swift enum
        let receivedBytes = socket_recv(self.descriptor, data.rawBytes, data.capacity, flags)
        guard receivedBytes > -1 else { throw SocksError(.readFailed) }
        let finalBytes = data.characters[0..<receivedBytes]
        let out = Array(finalBytes.map({ UInt8($0) }))
        return out
    }

    public func recvAll() throws -> [UInt8] {
        var buffer: [UInt8] = []
        let chunkSize = 512
        while true {
            let newData = try self.recv(maxBytes: chunkSize)
            buffer.append(contentsOf: newData)
            if newData.count < chunkSize {
                break
            }
        }
        return buffer
    }
}

Here at the line if newData.count < chunkSize every user will get a timeout when the last chunk received (newData) has a count of 512 bytes. This has been haunting us for days until we realised the socket library might be at fault.

czechboy0 commented 8 years ago

Damn, can you please send a PR with a failing test showing this? Thanks so much for reporting it!

czechboy0 commented 8 years ago

Hmm isn't that a general issue of receiving data that evenly divides into your chunk size? If we're receiving 1024, for instance, in the first loop, we'll receive newData.length == 512, we'll append it to buffer, and check: newData.count (512) < chunkSize (512) needs to fail, otherwise we wouldn't read the rest.

The real issue is that recv doesn't return 0 bytes when there's no more data, instead of blocking.

Joannis commented 8 years ago

Nothing specifically. But if you take a custom TCP server, make a TCP client with Socks, let the server send 512 bytes and keep the socket open. The client will never receive the 512 bytes. Same goes for messages of 1024, 2048 bytes etc

czechboy0 commented 8 years ago

A way to fix this would be to find out how to ensure recv returns 0 bytes when there's no data left instead of blocking.

Joannis commented 7 years ago

Since nobody picked this up yet I'll propose my (semi-)solution. Increase the default recv chunk size to UInt16.max or UInt16.max - 1.