Repository navigation
Client Ping/Pong broken in 1.6 #170
Description
Pong request payload does not match ping response payload (which causes the websocket server I try to interact with to close the connection)
In my scenario the ping from the websocket server is an empty string. When producing the pong response, the pushFrame is invoked receiving the following $frame argument: [true,"","pong",false]. This in turn produces a $data of �\u0000" which is written to the stream.
I could quickfix it for my scenario by changing this line
$this->write($data);
to
$this->write($opcode === 'pong' ? '' : $data);
But I instead solved it by downgrading to v1.5.8 where the ping/pong interaction seems to work fine.
Reproduced with PHP 8.1.12 (cli)
version 1.6.3
Activity
sirn-se commentedon Nov 8, 2022
Did some checks, and the autoresponder do reply as expected on empty message. The issue you have is probably related to masking. This has been wrong all along, but it appears my attempt to improve things in
v1.6caused other issues instead. I'll have a look at that when I've got some time for it.As a side note, the quick fix you mention means the client do not send a pong when receiving ping at all. The
$datavariable is the entire frame, not only the payload.sirn-se commentedon Nov 12, 2022
Hi @MarcusOhman91
I did a rewrite of masking policy that (hopefully) will solve the issue you're experiencing; #171
However, the servers I'm testing with work with
v1.6as is. So I would appreciate if you're able to confirm that this fix solves things for you before I merge it to master. You should be able to test by specifyingdev-1.6-fixesin Composer.Thanks in advance!
sirn-se commentedon Nov 22, 2022
@MarcusOhman91
Please switch to
phrity/websocket(official fork of this repo), version1.6.4should solve your issue.thorewi commentedon Jan 1, 2023
The PR was closed and not merged, why?
MarcusOhman91 commentedon May 11, 2023
delayed response, but can confirm that this worked.
I guess this issue should be left open if any of the remaining maintainers decides to fix it in this repo