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

(TCP fragmentation) Strange error on iOS Simulator on socket close #261

Closed
serieuxchat opened this issue Sep 30, 2016 · 10 comments
Closed

Comments

@serieuxchat
Copy link

Sometimes I have a strange error when running on iOS Simulator.
The server sends a socket close frame, which gets split into 2 TCP segments (2 bytes each).
The first 2 bytes contain the opcode (8) and the payload length (2).
The other two bytes contain the status code (1000 - normal closure).

However, for some reason the function processInputStream() only gets the first two bytes when it reads the stream with:
let length = inputStream!.read(buffer, maxLength: BUFFER_MAX)

During subsequent processing (processOneRawMessage) it all fails (line 614) here:
code = WebSocket.readUint16(baseAddress, offset: offset)
if code < 1000 || (code > 1003 && code < 1007) || (code > 1011 && code < 3000) {
code = CloseCode.ProtocolError.rawValue
}

The code is read from the memory area beyond the length that was actually read from the stream, so it incorrectly interprets it and fails with the protocol error.

Although I don't have a clue (right now) as to why it does not read the last two bytes (from the stream), I think, the code should not try to read beyond the length determined by the number of bytes actually read from the stream (in readUint16 and other similar functions).

I was wondering if you encountered similar problems?

Kind regards,
Sergey A. Novitsky

@serieuxchat serieuxchat changed the title Strange error on iOS Simulator (iOS9 and iOS10) on socket close (TCP fragmentation) Strange error on iOS Simulator on socket close Sep 30, 2016
@serieuxchat
Copy link
Author

By looking at the code, it seems that the library is not taking possible TCP fragmentation into account at all... (please correct me if I am mistaken).

@nuclearace
Copy link
Contributor

I would think that would be up to the Streams library.

@serieuxchat
Copy link
Author

Apparently, it's not... A colleague just told me that a similar Java library (in TomCat) keeps reading from the stream until the whole web socket frame has been read... Perhaps, something similar can be done in Starscream...

@daltoniam
Copy link
Owner

Hmm, not exactly sure what you are seeing. It uses the readUint16 to ensure it is reading aligned bytes (added in #146). As far as TCP fragmentation, NSStream is handling that, but there is also WebSocket frame fragmentation that is dealt with in Starscream. This can be seen here:

https://github.com/daltoniam/Starscream/blob/master/Source/WebSocket.swift#L445
and
https://github.com/daltoniam/Starscream/blob/master/Source/WebSocket.swift#L604

Basically Starscream continues to collect bytes until it has at least 2 bytes to read before processing a WebSocket frame. The first 2 bytes give the type of packet and the length it will be as to account for how many bytes to read before process another WebSocket frame.

All of that being said, is there any sample data or program that can be provided to reproduce this issue?

@serieuxchat
Copy link
Author

No, I am not talking about WebSocket fragmentation, only TCP fragmentation.

  • Are you sure that NSStream is, indeed, handling TCP fragmentation? By that I mean that a single read is guaranteed to deliver the whole TCP frame, even it has been split? (I have not seen any Apple docs specifying that, but maybe I was not looking carefully...).
  • As far a reproduction is concerned, our project is quite big (and NDA-ed on top of that) to be able to publish anywhere openly, but here is as detailed a description that I can disclose:
    • We have TomCat at the other end (tried both version 7 and 8), and I see with Wireshark that the socket close frame gets split into 2 segments: 2bytes + 2bytes. (This happens not always, but approximately in 30% of cases).
    • Starscream does a single read from the stream (supplying the 4K buffer) and assumes that this read would deliver both segments (i.e. 4 bytes). Instead, what I see happening is that only the first segment (2bytes) gets delivered.
    • Starscream starts processing received data and at some moment hits this line:
      code = WebSocket.readUint16(baseAddress, offset: offset)
      Here the assumption is that the status code has already been read from the stream, but it was not (yet). As a result, we get zeroes (instead of 1000, normal close), and bail out with the protocol error.

So, if you think that NSStream must really take care of TCP fragmentation (i.e. block the read function until all segments have been received and assembled), perhaps, we have a bug on Apple's side.
If, on the other hand, NSStream does not guarantee that a single read would get the whole reassembled frame, we should account for that lack inside Starscream.

@commjoen
Copy link

commjoen commented Oct 6, 2016

We are encountering the same problem: it seems that the library misinterprets the split frame, because it looks at uninitialized memory. Can we get a fix on this for both swifth 2.3 and swift 3 please?

@daltoniam
Copy link
Owner

ok. I think we are getting at the same thing, but I think our understanding of TCP fragmentation is different. I see TCP fragmentation as something handled by the OS TCP/IP stack the is responsible for assembling TCP packets into their "stream". At the application level it would just be a chuck or fragment of the TCP data "stream". Starscream handles the stream fragmentation by ensuring there is at least 2 bytes in a WebSocket fragment (as to ensure there is enough to read for the WebSocket frame header) and continues to collect and read until it has all the data for a packet before moving to the next one. I haven't specifically check reading a Close frame message, which could be where the issue lies.

All that being said, I believe there could be an issue with the close message reading in a Close frame, but I would need an example to ensure I could reproduce and fix it. I understand the difficult nature of an NDA, but even a simple example app that reproduced the behavior would be beneficial in tracking down the issue. Outside of that I will try and do some guess work as time permits to attempt to reproduce the issue.

@commjoen
Copy link

I have emailed you some details. For now, you can use https://github.com/spring-projects/spring-boot/tree/master/spring-boot-samples/spring-boot-sample-websocket-tomcat as a sample :).

daltoniam added a commit that referenced this issue Oct 27, 2016
@daltoniam
Copy link
Owner

I release a new version (2.0.1) that should fix the issue with large close messages not getting read out. Any issues please reopen this.

@karthik55
Copy link

I am getting ProtocolError With close Code 1000 and Message is Empty, any one come across issue. If so could you tell me the reason for the issue.

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

No branches or pull requests

5 participants