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

PING causes receive() problems #94

Open
EnTerr opened this issue May 26, 2016 · 9 comments
Open

PING causes receive() problems #94

EnTerr opened this issue May 26, 2016 · 9 comments

Comments

@EnTerr
Copy link

EnTerr commented May 26, 2016

A bit surprising, this is not about #14. I don't care about "proper" implementation of ping/pong but am having problems because the ws server (which is beyond my control) sends a ping on connect and the client on my side reads 1 byte too many and the frames on my side got royally screwed up from there on.

The problem is around here: https://github.com/lipp/lua-websockets/blob/master/src/websocket/sync.lua#L14
The first chunk read apparently is always of size 3 - but it turns out PING is a frame of only 2 bytes.

On example i tracked with wireshark, the first chunk read is "89 00 82", which gets returned to me as an empty message (which is fine by me) - however turns out the PING is actually "89 00" and the 82 is part of the next frame "82 0b ..." (11 byte binary frame). Because 82 was mis-read, the 2nd frame turns into "0b ..." and everything goes downhill afterwards.

Seems that simply changing that line to bytes = 2 might be okay, given a quick skim at frame.decode() - but is that so?

ps. also bytes = 2 on line 60

@lipp
Copy link
Owner

lipp commented May 30, 2016

🎱 !
As far as I can say, that's ok. Do you get the correct opcode (PING) from the receive function then?

@EnTerr
Copy link
Author

EnTerr commented May 30, 2016

Yes - i just checked and the opcode returned in that case is 9 - although i don't care nor check for that, i just treat it as "empty frame" (works for me in my specific case).

More detailed code review persuaded me that yes, changing bytes = 3 to bytes = 2 in those two places shouldn't break anything.

@moteus
Copy link
Contributor

moteus commented May 30, 2016

According rfc you have to replay PONG.

Upon receipt of a Ping frame, an endpoint MUST send a Pong frame in
response, unless it already received a Close frame. It SHOULD
respond with Pong frame as soon as is practical.

But rfc does not describe what remote side should do in case if you do not send PONG

@lipp
Copy link
Owner

lipp commented May 31, 2016

@moteus thanks for the rfc heads-up! than, maybe the "endpoint" should just auto-reply with PONG and shouldn't present this to the user at all!?

@moteus
Copy link
Contributor

moteus commented May 31, 2016

I am not sure. PING/PONG can be used to some think like OOB I think. They can have Payload.

Control frames can be interjected in the middle of a fragmented message.

So in my implementation I make option to auto response to ping message.

Also about PONG

A Pong frame MAY be sent unsolicited. This serves as a
unidirectional heartbeat. A response to an unsolicited Pong frame is
not expected.

@EnTerr
Copy link
Author

EnTerr commented May 31, 2016

"Control frames can be interjected in the middle of a fragmented message." does not imply ping/pong can carry a load, just that they can be interspersed in normal traffic.

It seems to me that a receiver should always absorb PING and respond automatically with PONG quietly (with no report up the call chain). Received PONG should not be absorbed but passed up, since presumably we sent the PING and expect the PONG (e.g. measuring latency).

Here is an interesting caveat however: https://github.com/lipp/lua-websockets/blob/master/src/websocket/sync.lua#L67 concatenates bunch of frames into a single string and returns the 1st opcode only! Meaning that if the 1st opcode was PONG, everything will get labeled so, regardless it is BINARY/TEXT. Or if the 1st opcode is BINARY and PONG is one of the next frames, it will disappear.

Hmmm, this should be a separate issue when i think of it - mix of BINARY / TEXT / etc all gets "mushed" together. Seems to me a proper handling should return only 1 frame and its corresponding 1 opcode, keeping the rest for the next call - perhaps akin to #80 (comment)

@moteus
Copy link
Contributor

moteus commented May 31, 2016

"Control frames can be interjected in the middle of a fragmented message." does not imply ping/pong can carry a load, just that they can be interspersed in normal traffic.

Any control message can have payload with size less than 128 byte.
Control message can not be fragmented. So it stream can looks like
[BINARY] [CONTINUE] [PING] [CONTINUE] [FINAL] where one fragmented message(4 fragments) and PING control message (may be with payload).
Also like i mention it is valid send PONG message without PING message.

Update. rfc does not restrict that receiver send pong immidiatly.it say it should send it in reasonable time.

@EnTerr
Copy link
Author

EnTerr commented May 31, 2016

@moteus - sure, what you say does not contradict what i said. A sender is free to send PING/PONG without limitations. Receiver always sees the PONGs but not the PINGs.

The only caveat is - thanks for pointing that out! - that if PING carries a payload (e.g. timestamp/serial), the websocket stack must (auto)reply with a PONG with exactly the same payload, see https://tools.ietf.org/html/rfc6455#section-5.5.3 . So yes, things seem lined up by RFC for automatic handling of PING->PONG

@lipp
Copy link
Owner

lipp commented Jun 1, 2016

sounds like a good strategy! would be great to have this...

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

3 participants