WebSocketConnection sendMessage does not process hixie message correctly

austgl / phpws

Automatically exported from code.google.com/p/phpws

0 stars 0 forks source link

WebSocketConnection sendMessage does not process hixie message correctly #9

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago

sendMessage does not check if passed input message is compatible with $this 
WebSocketConnection type. As a result, if sendMessage is called for 
WebSocketConnectionHixie with WebSocketMessage instance as input (not 
WebSocketMessage76 as it is supposed) - than nothing happens - no error and no 
sending data.

Should fire exception or handle such cases internaly by coverting input message 
to proper type. Or better yet, WebSocketConnection should provide a method to 
construct a message of proper type, and direct creation via 
WebSocketMessage::create must be eliminated.

Original issue reported on code.google.com by mo...@tushino.ru on 15 Dec 2011 at 8:54

GoogleCodeExporter commented 9 years ago

Valid point, the second method sounds the cleanest to me and method should 
throw exception if type is incorrect.

Will fix it tonight.

Thanks for the report!

Chris

Original comment by ch...@devristo.com on 15 Dec 2011 at 9:35

Changed state: Accepted

GoogleCodeExporter commented 9 years ago

IMHO, still I think the approach with an exception is acceptable only as 
temporary workaround. Lets consider a simple scenario. An application accepts 
new client connection, and gets an instance of WebSocketConnection $user. The 
application normally should not know what type of underlying connection (hybi 
ar hixie) is used. The app may "want" simply send a message, so it should be 
able to call some general message factory method which will internally create a 
message of appropriate type. Until this is not done, and if the exception trap 
is added, this will compell a developer to check WebSocketConnection type and 
invoke either WebSocketMessage::create or WebSocketMessage76::create by hands. 
It's not convenient.

Original comment by mo...@tushino.ru on 15 Dec 2011 at 10:27

GoogleCodeExporter commented 9 years ago

I agree with your post. But I think IWebSocketConnection::sendString($str) 
should be used over the sendMessage(). In most cases the message type is 
irrelevant (as you say). The sendString method creates the correct message type 
for you.

My opinion is that a IWebSocketConnection::createMessage() and some 
type-checking in sendMessage / sendFrame should be enough.

Original comment by ch...@devristo.com on 15 Dec 2011 at 6:05

GoogleCodeExporter commented 9 years ago

Yes, I use the sendString. But it was a little confusing that sendMessage was 
also there, and many user may prefer to use sendMessage in the first hand, and 
only after it gives the exception they will switch to sendString. If so, than 
why not to leave the sendString, and remove sendMessage from publics?

Original comment by mo...@tushino.ru on 16 Dec 2011 at 8:41

GoogleCodeExporter commented 9 years ago

I am not sure about that. I mean SendString can only create text frames, while 
in theory you could sent non-text data as well (HYBI only). In the end HIXIE 
will go away since nobody will use that in the future, then everything gets a 
lot easier.

Think what mostly lacks is good documentation, that is one of the reasons I 
took of downloads as well.

Original comment by ch...@devristo.com on 16 Dec 2011 at 4:42

GoogleCodeExporter commented 9 years ago

Original comment by ch...@devristo.com on 5 Jan 2012 at 9:53