grblHAL / Plugin_networking

grblHAL plugin for networking protocols (Telnet, WebSocket, FTP, HTTP) and related utilities on top of LwIP
Other
10 stars 6 forks source link

Websocket: no pong response to ping packet #10

Open kamplom opened 6 months ago

kamplom commented 6 months ago

What

grblHAL not respondig with pong opcodes to ping messages. Nothing arrives to the client side. No error message was triggered.

Client

Tested with python websocket-client library. It sends a packet with the ping OpCode and no data

Hack

I cannot understand the code easily so I just hardcoded a tcp_write with the OpCode for pong. websocket-client does not care about the payload in the pong response, but it should be the same as the one sent with the ping packed which triggered it.

case WsOpcode_Ping:
  if((frame_done = plen >= session->header.payload_rem)) {
      if(session->state != WsState_Closing) {
          plen -= session->header.payload_rem;
          if(collect_msg_frame(&session->header, payload, session->header.payload_rem))
              payload = session->header.frame;
          fs.opcode = WsOpcode_Pong;
          payload[0] = fs.token;

          uint8_t txbuf[5];
          if(tcp_sndbuf(session->pcb) > 4) {
              txbuf[0] = wshdr_pong.token;
              txbuf[1] = 2;
              strcpy((char *)&txbuf[2], "Hi");
              tcp_write(session->pcb, txbuf, 4, TCP_WRITE_FLAG_COPY);
              tcp_output(session->pcb);
          }
          // tcp_write(session->pcb, payload, session->header.payload_len, 1);
          // tcp_output(session->pcb);
      }
  } else {
      collect_msg_frame(&session->header, payload, plen);
      plen = 0;
  }
  break;

A proper solution should be implemented.

terjeio commented 6 months ago

Try this:

            case WsOpcode_Ping:
                if((frame_done = plen >= session->header.payload_rem)) {
                    if(session->state != WsState_Closing) {
                        plen -= session->header.payload_rem;
                        if(collect_msg_frame(&session->header, payload, session->header.payload_rem))
                            payload = session->header.frame;

                        uint8_t *pong;
                        uint_fast16_t hdr_len = session->header.payload_len < 126 ? 2 : 4;

                        if((pong = (uint8_t *)malloc(session->header.payload_len + hdr_len))) {

                            uint_fast16_t i = session->header.rx_index, j;
                            uint8_t *mask = (uint8_t *)&session->header.mask, *pm = payload, *buf = pong;

                            fs.opcode = WsOpcode_Pong;
                            *buf++ = fs.token;
                            *buf++ = session->header.payload_len < 126 ? session->header.payload_len : 126;
                            if(session->header.payload_len >= 126) {
                                *buf++ = (session->header.payload_len >> 8) & 0xFF;
                                *buf++ = session->header.payload_len & 0xFF;
                            }

                            for(j = 0; j < session->header.payload_len; j++)
                                *buf++ = *pm++ ^ mask[i++ % 4];

                            tcp_write(session->pcb, pong, session->header.payload_len + hdr_len, TCP_WRITE_FLAG_COPY);
                            tcp_output(session->pcb);
                            free(pong);
                        }
                    }
                } else {
                    collect_msg_frame(&session->header, payload, plen);
                    plen = 0;
                }
                break;
kamplom commented 6 months ago

It appears to be working. From the python client, the ping packet sent

DEBUG:websocket:++Sent raw: b'\x89\x807\nDs'
++Sent decoded: fin=1 opcode=9 data=b''

and the pong response

DEBUG:websocket:++Rcv raw: b'\x8a\x00'
++Rcv decoded: fin=1 opcode=10 data=b''

As you can see the data field is always empty. Before merging it, I think a test where the ping packet has a no empty data field should be conducted as well, and check if pong response contains the same data.