Skip to content
This repository has been archived by the owner on Aug 25, 2022. It is now read-only.

Fix for read function not reading the correct number of bytes #106

Merged
merged 1 commit into from
May 27, 2015

Conversation

jrdiaz
Copy link
Contributor

@jrdiaz jrdiaz commented May 23, 2015

Hi again. After upgrading to new Elephant.IO version from legacy one, I realised that when reading from Socket.IO, the messages were incorrect. After a bit of study I found that the calculation of the number of bytes to read were wrong because it was treated as an integer number when it was an hex and the mold wasn't converting the value correctly from hex to integer.

Here comes a quick pull request with the change to make the read function usable.

Hope that it helps!

@Taluu
Copy link
Contributor

Taluu commented May 27, 2015

So, it works with this fix ? this could be a huge improvement !

@jrdiaz
Copy link
Contributor Author

jrdiaz commented May 27, 2015

Yes, it works with this fix, at least for short reponses. All the short, simple responses I've tried works as expected just by applying the changes.

I still have to test if library manages correctly large ones (>16Kb), but this is a different issue that would come in a new pull request when properly tested.

Besides this, I just needed a dumb utility function to convert the websocket string response like "5::{}" to a proper PHP object discarding the "5::" and json encoding the rest of string. Maybe it could be an addition to client utility functions, buy I just wanted to pull the fix since as you say, it could be an improvement.

The longer operation I tried was:

  • connect to Socket.IO (0x version) server
  • wait for "Ok" response (1:: message using fixed read function)
  • emit from PHP a complex object
  • process complex object on websocket server
  • read response on PHP
  • emit simple message from PHP
  • process message in websocket server
  • emit event from websocket server with simple value (boolean in this case)
  • read response in PHP of the emitted event and the passed value

Everything worked as expected

Using the read function, the need for callback function is reduced to zero since every websocket function can emit an event at the end and PHP can read the emitted event with the arguments used.

@Taluu
Copy link
Contributor

Taluu commented May 27, 2015

The PR seems legit, so I guess I can merge it. Thanks ! :)

But if I recall correctly, isn't the 5::{} meaning a binary event sent through the socket, and {} the data typically ? hence your need to "discard" the 5:: part (maybe find something else though... correctly parsing the message would help I guess)

Taluu added a commit that referenced this pull request May 27, 2015
Fix for read function not reading the correct number of bytes
@Taluu Taluu merged commit 2452d2d into Wisembly:master May 27, 2015
@jrdiaz
Copy link
Contributor Author

jrdiaz commented May 27, 2015

You are right 5:: wasn't the heading, I was typing by memory :P

This was referenced May 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants