paragi / PHP-websocket-client

142 stars 61 forks source link

read method mistyped #24

Closed chwpi closed 1 year ago

chwpi commented 1 year ago

$payload_len += ord($header needs to be $payload_len += ord($ext in websocket_read

Trismegiste commented 1 year ago

I think you're referring to https://github.com/paragi/PHP-websocket-client/blob/a40a6bf0525a1e76950d19b41201ccccb80bf9cd/src/Client.php#L256 if I understand correctly ?

I agree, it seems a little odd but I don't understand the subtleties of Web Socket Protocol. We need more info from @paragi about this.

cp-helsinge commented 1 year ago

The job of this function, is to read a 7 bit, 2 or 8 byte integer, in a specific order (Little-endian), describing the length of the payload. If i understand your suggestion right? you suggest to use the length of that integer (not the value) as payload length. That doesn't give meaning to me, so i guess I don't really understand your point?

Du you have a concrete problem to solve? test case?

chwpi commented 1 year ago

The job of this function, is to read a 7 bit, 2 or 8 byte integer, in a specific order (Little-endian), describing the length of the payload. If i understand your suggestion right? you suggest to use the length of that integer (not the value) as payload length. That doesn't give meaning to me, so i guess I don't really understand your point?

Du you have a concrete problem to solve? test case?

it looks like the code referenced above instead changed the fread() to overwrite $header, which makes the block of code below it work without issue.

$header=fread($sp,$ext_len); That works OK

paragi commented 1 year ago

Your proposal: `// Get payload length extensions $ext_len = 0; if($payload_len >= 0x7E){ $ext_len = 2; if($payload_len == 0x7F) $ext_len = 8; $ext=fread($sp,$ext_len); if(!$ext) die("Reading header extension from websocket failed");

// Set extented paylod length $payload_len= 0; for($i=0;$i<$ext_len;$i++) $payload_len += ord($ext[$i]) << ($ext_len-$i-1)*8; }`

The only difference i can spot, is that you propose to introduce an extra string variable $ext to hold the headerbytes of the payload length, instead of reusing the spend $header variable. Is that correct?

'Yes and the function read the header which is totally ridiculous and wrong, Please read https://www.rfc-editor.org/rfc/rfc6455#section-5.2' Please clarify?