Bug: UInt64 maybe negativ

[szuniverse]

Hi.

Very often get a runtime error on the latest version.

This is around the 500. row:
var len = dataLength
if dataLength > UInt64(bufferLen) {
len = UInt64(bufferLen-offset) // runtime error position
}

the bufferLen value is 8, and the offset value is 10. I dont know how it happens but it will be negativ..
Unfortunately I didnt implement the server so I cannot check it. maybe it would be a good way to handle this case in this library.

[daltoniam]

Yeah a UInt64 should never be negative as it is unsigned. I'm not sure how the bufferLen could be 8 and the offset be 10. That would equal -2 which certainly would crash an unsigned number. The code you are referencing means the webSocket frame (packet) doesn't have a full UInt64 value to read the length of the data. Can you verify what the size of dataLength? There isn't an easy way to handle this in the library as a webSocket packet should always all of it's framing information.

[daltoniam]

@szuniverse where you able to verify with the size of dataLength was or track down any more information?

[szuniverse]

Unfortunately in the last few weeks I couldn't working on my project. So I will write more details If I will have enough time

[daltoniam]

OK sounds good. I will go ahead and close this issue out until then. Just reopen it when you are ready.

[wispur]

@daltoniam
Hello~
We are facing the same issue, in our case the dataLength is 126, which means we should have 2 more byte following to read, however the bufferLen is only 3 (offset being 4).
We suspect we would need to create a temporary WSResponse in order to postpone the processing just like if we lack normal data even though this is a rare case.

[daltoniam]

@wispur thanks for the report. If you get any details on a consistent way to reproduce this, let me know and I will look into it.

[wispur]

@daltoniam Thanks for the reply too! I am also trying to reproduce this issue reliably, so far this issue just pops randomly when I do not expect it to happen :(

[szuniverse]

Hi. Last past days I tried to reproduce this bug on a consistent way but I cant.. Its seems randomly, but its a real bug, which is always crashes the app
I think the Starscream can handle this case somehow and receive an error message or something like this would be better than the crash.
I think its time to reopen this bug, and handle this bug with an error message, and dont crash the app when this weird situation occurs

[ And one more thing... In this weekend my router has been broken, and I have to use an older router more than 7years old. And this bug appears more frequently, so I think it depends on the router. But I am not sure, this is my experience.. ]

[daltoniam]

If I knew what was causing the crash, I would just fix it 😄. It isn't something that can just be handled and not crash, we have to track down a cause and hopefully a fix. My working theory is that a huge packet is coming through, but is fragmented just right, so we can't read the expect length of the packet. I am going to try adding that protection logic and see if it resolves the issue. I will try to do that in the next few days, but no promises as it has been a busy last few weeks.

[daltoniam]

@szuniverse try adding this at line 499 (right below the dataLength checkers):

if bufferLen < offset {
    fragBuffer = NSData(bytes: buffer, length: bufferLen)
    return
}

Also here is a screenshot for reference:

[szuniverse]

Okey. thanks a lot! I will "test" it, I hope the crash will dissapear without side effect :)
Will you add these lines to the official version later, if you will have enough time?

[daltoniam]

Yes I will, hoping it works 😄 . Let me know if it fixes and I will add it into the official release.

[szuniverse]

Well, It works fine. The runtime crash has been resolved, but give me one week to test it. I will write you soon.

[daltoniam]

Great to hear. If all goes well, we can run it threw the autobahn tests again and cut a new version!

[szuniverse]

Okey. I tested it on this weekend and everything works fine. The crash has been resolved :)
I think its time to make autobahn tests again and cut a new version!

[daltoniam]

Awesome! I will try and run the test some time this week and cut the new version.

[szuniverse]

Hi! Do you merged this bug fix to the master?

[daltoniam]

I did, master should have all the fixes. Haven't create the new tag yet, was finishing the final touches on an autobahn test/example before creating the 0.9.4 tag.

[szuniverse]

Awesome! :)