Abnormal after receiving Continuation Frame

[Ambert1108]

Hello author, I am using oatpp-ws 1.3.0 version, and it seems that the warehouse has not been updated for a long time.

I recently encountered a problem using oatpp-ws: when the other end sends a text frame, a continuous frame, and a text frame, the ws listening will be disconnected and an error occurs, what happen?

[oatpp::web::protocol::websocket::WebSocket::listen()]:Unhandled error occurred. Message='[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state. OPCODE_CONTINUATION expected'

Analyze

To solve this problem, I checked the source code, as shown below：

bool WebSocket::checkForContinuation(const Frame::Header& frameHeader) {
  if(m_lastOpcode == Frame::OPCODE_TEXT || m_lastOpcode == Frame::OPCODE_BINARY) {
    return false;
  }
  if(frameHeader.fin) {
    m_lastOpcode = -1;
  } else {
    m_lastOpcode = frameHeader.opcode;
  }
  return true;
}

void WebSocket::handleFrame(const Frame::Header& frameHeader) {
  
  switch (frameHeader.opcode) {
    case Frame::OPCODE_CONTINUATION:
      if(m_lastOpcode < 0) {
        throw std::runtime_error("[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state.");
      }
      readPayload(frameHeader, nullptr);
      break;
      
    case Frame::OPCODE_TEXT:
      if(checkForContinuation(frameHeader)) {
        readPayload(frameHeader, nullptr);
      } else {
        throw std::runtime_error("[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state. OPCODE_CONTINUATION expected");
      }
      break;
      
    case Frame::OPCODE_BINARY:
      if(checkForContinuation(frameHeader)) {
        readPayload(frameHeader, nullptr);
      } else {
        throw std::runtime_error("[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state. OPCODE_CONTINUATION expected");
      }
      break;
          
   //part of code
}
    
void WebSocket::listen() {
  m_listening = true;
  try {
    Frame::Header frameHeader;
    do {
      readFrameHeader(frameHeader);
      handleFrame(frameHeader);
    } while(frameHeader.opcode != Frame::OPCODE_CLOSE && m_listening);
  } catch(const std::runtime_error& error) {
    OATPP_LOGD("[oatpp::web::protocol::websocket::WebSocket::listen()]", "Unhandled error occurred. Message='%s'", error.what());
  } catch(...) {
    OATPP_LOGD("[oatpp::web::protocol::websocket::WebSocket::listen()]", "Unhandled error occurred");
  }
}

As you can see, after we call the listen method of Websocket, it will continuously try to read and process frames, and enter handleFrame when a frame is received.

In handleFrame, the frame type is determined. When the received frame is a text frame or a binary frame, oatpp will determine m_lastOpcode. If m_lastOpcode is a text frame or a binary frame, an exception will be thrown. Otherwise, if this frame is the last frame, m_lastOpcode is reset to -1; if this frame is not the last frame, m_lastOpcode is set to the type of this frame.

1. The program listens to a text frame (fin=false). Set m_lastOpcode to OPCODE_TEXT, which is the text frame type. Read the payload and save it.

2. The program listens to a continuation frame (fin=true). Take out the payload, combine it with the previous payload, and pass it to the user.

3. The program listens to a text frame (fin=true). Judge that m_lastOpcode=OPCODE_TEXT and throw an exception.

My fixed

At this point, we know that this problem occurs because: oatpp does not reset m_lastOpcode to -1 after processing the continuation frame and determining that the continuation frame is the last frame (fin=true).

Therefore, after processing the continuation frame, reset m_lastOpcode to -1, and the problem is solved.

void WebSocket::handleFrame(const Frame::Header& frameHeader) {
  
  switch (frameHeader.opcode) {
    case Frame::OPCODE_CONTINUATION:
      if(m_lastOpcode < 0) {
        throw std::runtime_error("[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state.");
      }
      readPayload(frameHeader, nullptr);
      m_lastOpcode = -1; //here we are
      break;
      
    case Frame::OPCODE_TEXT:
      if(checkForContinuation(frameHeader)) {
        readPayload(frameHeader, nullptr);
      } else {
        throw std::runtime_error("[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state. OPCODE_CONTINUATION expected");
      }
      break;
      
    case Frame::OPCODE_BINARY:
      if(checkForContinuation(frameHeader)) {
        readPayload(frameHeader, nullptr);
      } else {
        throw std::runtime_error("[oatpp::web::protocol::websocket::WebSocket::handleFrame()]: Invalid communication state. OPCODE_CONTINUATION expected");
      }
      break;
          
   //part of code
}

However, my approach probably does not fully consider the overall logic of the program design. If the author sees this, please answer two questions:

1. Does the above problem exist?
2. If so, is there a plan to fix this problem?

THANKS!