client.send() does not mask the messages

[adelin-mcbsoft]

Hi Gil,

Me again.
I want to bring to your attention the following situation:

Messages that go through client.send() are sent in plain-text, unmasked (see more about websocket masking).
However, it seems that the library understands and successfully decodes the received masked messages.

How I tested:

1. When you use the HTML5 send() method to send a message, the messages are automatically masked by the browser and sent to the server.
2. At the server, the messages come masked, and the server decodes it. In PHP, I'm using the following function to unmask:

/**
     * @method  unmask
     * @param   $text   string
     * @return  string
     * @desc    Unmask incoming framed message
     */
    private function unmaskReceivedData($text) {
        $length = ord($text[1]) & 127;

        if($length == 126) {
            $masks = substr($text, 4, 4);
            $data = substr($text, 8);
        } elseif($length == 127) {
            $masks = substr($text, 10, 4);
            $data = substr($text, 14);
        } else {
            $masks = substr($text, 2, 4);
            $data = substr($text, 6);
        }

        $text = '';
        for ($i = 0; $i < strlen($data); ++$i) {
            $text .= $data[$i] ^ $masks[$i%4];
        }
        return $text;
    }

AFAIK, this method is being written according to the RFC standards.
From there, on my server I decode (unmask) the message, process it, then mask it again to broadcast to all the connected clients.

For masking I'm using the following code:

    private function maskSentData($text)  {
        $b1 = 0x80 | (0x1 & 0x0f);
        $length = strlen($text);

        $header = '';

        if($length <= 125)
            $header = pack('CC', $b1, $length);
        elseif($length > 125 && $length < 65536)
            $header = pack('CCn', $b1, 126, $length);
        elseif($length >= 65536)
            $header = pack('CCNN', $b1, 127, $length);
        return $header . $text;
    }

When I broadcast it, the browser and the ESP32 receive it successfully.

However, when I try to send a message through libraries client.send(text) method, all the messages received on the server are plain, not masked in any way (and of course, the servers unmask method fails).

Just a note to this: Something might had happen recently (through an update or so) because few weeks ago (when we had issue #14 ), this bug didn't happen (though I cannot identify where the change could come from...).

You know the library as better as anyone, so I'm pretty sure you can shed some light here.

If you need any more info from me, just let me know.
Hope it helps,

Thanks,
Best,
A.

[gilmaimon]

Hi,

I am currently answering from mobile, and I will only be able to fully address your issues this weekend.

However, regarding masking. There is a little trick I use which is to mask the messages with the key "/x00/x00/x00/x00" so message are indeed masked (the mask flag is on) but the masking process is a no-op (so I'm saving space and time, which I felt are especially relevant for embedded). Masking should not fail on your end, does your code handles null masks well?

If I am missing a point I'm sorry, as I've said I will have time to read and answer more thoroughly this weekend.

Best regards,
Gil.

[adelin-mcbsoft]

I'm not sure I got what you said... adding "/x00/x00/x00/x00" in front of the string turns on the mask flag?
...
Later edit: Tested, doesn't work.

[gilmaimon]

No, that's not what I meant.
All upstream messages are masked, but they are masked using a null key so the masked result is exactly the same as the unmasked, but it is still considered masked.

Try using Wireshark, you will see the messages are masked (should be).

Best regards,
Gil.

[adelin-mcbsoft]

I see now what you're saying... basically it flags it as masked but is not really masked.
My code does handle this situation, however the behavior is weird (see image):

In the same loop (and almost same text), 90% of the time the messages are sent incorrectly:
For a single send() call, the library sends firstly a request with the null bytes (without the text), and afterwards sends the text in a second call (marked as red in the screenshot).

In 10% of the times, it sends it correctly, null bytes + text in a single call (marked as green in the screenshot).

I don't really understand why... I'll try to investigate further, though my test loop is so simple:

void loop() {
  if (client.available()) {
    client.poll();
    if(i++ % 10000 == 0) {
      Serial.println("Client still connected.");
      String  str_i  = String(i, DEC);
      String  text = "{\"type\":\"user-msg\",\"name\":\"Jess D_Arduino\",\"message\":\"Here we are again, sequence [";
              text = text + str_i;
              text = text + "]...\"}";
      client.send(text);
    }
  } else {
    Serial.println("Client Disconnected!");
    delay(5000);
  }
}

Hit me if you have any ideas... (and once again, I appreciate your time) !

[adelin-mcbsoft]

Tried with a simple plain text in the loop... client.send("This is a nice text.");, same result: only one correct sent from many (see image).

[gilmaimon]

Oh, well the library always sends 2 packets: one for the header and one for the content. The ESP sometimes choose to optimize and send those together, but it is not guaranteed neither is it necessary.

Servers should always first read the header, see how much content there is (according to the header) and only then read that many bytes from the socket.

I haven't looked, but it is possible that your code tries to read everything at once?

[adelin-mcbsoft]

Well, somehow - yes - on the server-side, I am reading a single message in a single iteration of the loop (and to be truly honest, I find it normal to be this way).

Is kind'a surprising to hear that mostly it sends two separate packets and sometimes "for optimization purposes" sends it all at once....

Is there any way to set the ESP/Libraries to send the package/message "all-at-once" ? I mean, would be great to have some consistency in the workflow of the code... not alternating all the time from a method to another.

[adelin-mcbsoft]

Even the browsers implementation send it in a single package, so the RFC must state a single package design (header + body) on the same 'shot', so there should't be any reason to have two or more...

[gilmaimon]

It shouldn't matter to you. As a socket user you should call read with the required length and the code will block until you get that many bytes. Internally the TCP stream can be fragmented to any numbers of frames but you should know how many bytes you expect to read, and only continue execution when you have read that many bytes.

Can you share the code that reads from a socket in your implementation? I think it might be easier to explain with it.

[adelin-mcbsoft]

I have the feeling that we might be missing the point around...
TCP fragmentation & co. is part of another layer, way before we come to the socket implementation, isn't it? That is handled & solved by the network interface.

In my implementation, i use the socket_recv function in PHP (exaclty while(socket_recv($changed_socket, $socket_data, 1024, 0) >= 1) { ) which is being triggered at every newly received data in the socket (I'm reading a maximum of 1024 bytes in length, because usually I don't need longer lines -- and usually, I expect text only).

Given the current situation, to get both the header mask & the body, the loop must go through 2 iteratons most of the time and compute them afterwards, compared with the case where it is sent in a single line. And here's where all this issue's coming from...

So it all resumes to a single-question: is it a way to force ESP to send it in a single-line, like all the browsers do? Would solve the entire problem and wouldn't require additional workarounds.

(I mean, browsers mask the message differently - like in the real way... so there isn't any header required - therefore, is a single line with the body).

[gilmaimon]

Your issue is the receiving code, you should not read an arbitrary number of 1024 bytes. You should read 2 bytes which are the first and common part of the header, then read extra bytes if necessary (for long messages). At this point you should know the body length using all the header data you received, then read that many bytes from the socket. All of your calls should be using the MSG_WAITALL flag. For example, your code will fail to read a whole message in case it's longer than 1024. Websockets is very well defined, you should follow the RFC and receive from a socket accordingly (the header is there for a reason).

[adelin-mcbsoft]

Yes... according to RFC, you seem to be right. However, the only header I receive are those null-bytes from time to time... the implementation I have (which is a forked one) might be wrong at some point.
I'll investigate further.

Once again, big big big thanks for your support and your time... really important for me and my project.
Wish you all the best Gil,
Many many thanks,
A.

[gilmaimon]

Thank you, your contribution to this project is really big for me and I appreciate and enjoy your issues.

Let me know if you need any help with the php implementation, I know some basic PHP and the syntax is C-like enough for me. If you have any questions on implementing WS, I would love to help.

My email is mail.gilmaimon@gmail.com.

Again thank you.

Best wishes,
Gil.