Testing needed: new tick() method

[ghedipunk]

Probably very straightforward, but just in case I made a bonehead mistake...

New tick() method added. Blocking timeout changed from indefinite to 1 second.

Passed initial smoke test. Would appreciate others taking a look, too.

Updated branch dev-legacy.

[ghedipunk]

Preliminary testing shows that it's possible to send a message to a user before their handshake is completed, causing that handshake to fail.

Pretty sure that I'm going to call this a bug, and not an "undocumented feature."

My initial thought is to stick a check into the send() method that will ensure that the handshake is complete first... but then what to do with the message?

[ghedipunk]

Branch dev-legacy updated with bug fix introduced with trying to send a message before the handshake is complete.

Code to test was to add the following method into testwebsock.php (was originally causing handshake failures before the bugfix):

protected function tick() {
  foreach ($this->users as $user) {
    $this->send($user, 'tick: ' . time());
  }
}

[Xaraknid]

Hello, that a great feature to add. I'll test it when I have time.

Before that with quick looking it's need one thing : a set timeout to proc the tick. For preventing multiple proc per sec and it will give more precision.

here some pseudocode

$timeout = dev_set_time_in_second;

before calling tick check :
if timeelapse_since_last_proc > timeout then proc the tick and reset timeelapse to 0 else timeelapse + microtime();

also about the bug it's something I see in development branch with built-in broadcast method.

First you need to transform $user->handshake into $user->handshaked as boolean By default handshaked is false.
With that you can easily skip sending data to those who have not yet completed their handshake session.

if $user->handshaked you can safely send data

I hope this will help!

[ghedipunk]

The way I'm handling the bug is, in the send() method, if the $user->handshake property is false, then add the message to a new message queue. Then, on $server->_tick() (which I'm considering changing to private rather than protected), see if there are any messages in that queue that can be delivered or, if the user is no longer connected, if the message can be discarded.

As for setting a throttle to prevent the tick() method from firing too often... I considered it and may implement it. My only reservation is that I don't want people to think that a cycle is most certainly 1 second, and that there will always be a tick() every second, since I can't guarantee that. I'll have to work on the wording for the documentation for that bit...

[ghedipunk]

Already merged into master (via dev-legacy -> legacy -> master) in order to start a TLS branch with this update included. Would still appreciate testing, though.

[ghedipunk]

Posted to wrong ticket. Left here only for posterity's sake.

Note: I will not allow any version of SSL. They are all confirmed to be broken.

I'm also reluctant to allow TLS 1.0. To quote OWASP: "TLSv1.0 should only be used only after risk analysis and acceptance."

Risk of using TLS 1.0: Servers and clients get compromises easily.

Risk of not using TLS 1.0: Clients that do not support higher than 1.0 can not connect... (To research, but I suspect the answer is "0": How many of the big browsers had versions that use TLS 1.0 only, and also support version 13 WebSockets?)

If I do decide to allow TLS 1.0, it will be implicitly disallowed through the configuration, with the documentation giving big bold warnings immediately above and below the instructions to enable it.

Primary focus will be on TLS 1.2. After 1.2 is implemented, we'll fork again and implement 1.1. 1.3 is still very much a draft, but efforts should focus on it as it nears completion. Also, will do everything I can to keep up with changing best practices, such as PFS and no compression.

[Xaraknid]

Sorry my fault to write a reply in a hurry.

1. about $user->handshake should allways stay the same type boolean "false" or "true". Right now it "false" or a string containning the full handshake headers.
2. about buffering data while the client is between connection and fully complete the handshake. Personnaly the client didn't exist for me untill he fully complete the handshake, but surely some people will want this feature. So here some tought :
3. heldmessage should be in user class as it's allready handle the disconnection and flush all data relative to the user disconnneting.
4. heldmessage should concatenate all message to flush it in one single send after he complete is handshake if heldmessage != null.
5. dev-legacy = master should be switch to NONBLOCKING socket same as development branch this will fix an issue where server seem to stall caused by blocking on send.
6. To improve broadcasting the same message over multiple user $this->frame() MUST be call only ONCE. Because applymask is time consuming check the development branch it's a faster one.

[ghedipunk]

I was in a hurry, too... both to get a fix out, and to try to get some of my ideas out.

Yes, I agree that $user->handshake should not be what's checked, and something else should be added. I'm thinking of a getHandshakeStatus() method, that will return certain constants, i.e., 0 == not started, 1 == in progress, 2 == finished... though I haven't looked at the actual handshake process lately to see what would be appropriate.

As for holding messages until connections are in a state where they can accept them... I wanted to avoid having to check every connection every time we went through the _tick() method to see if there was action needed... I'm considering adding a second array of connections that require special attention as a long term solution.

Cleaning up the blocking in the legacy branches should be a different thread since it obviously needs addressed, and I'm liking the ideas of having pre-framed messages available in bulk and delayed sends.

[Xaraknid]

$user-handshake I think is the best candidate for that purpose because right now in master branch have no purpose. The only thing it keep when not false is the full raw handshake request. $user->headers allready have all parsed handshake within. By setting it to true it will take less memory, if handshake have session and TLS it will take more space maybe over 1K multiply this by 1000 users $user->handshake wil take 1M memory.

I don't think we need this precision about the state of handshake. false == 0 not started and 1 in progress. true == 2 finished.

I agree with you about the server must not check every connection every time in _tick() that why I said holdmessage should be within $user class.

1. doHandshake() allready check when handshake have been complete so inside this method you can add a flushholdmessage() method if $user->holdmessage is not null.
2. disconnect() allready clear all data related to connection and more precisely the $user class itself so it will clear $user->holdmessage in the same process
3. By doing this you can remove _tick() completely