Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Websockets: send multi frames #10413

Closed
mikeduglas opened this issue Feb 4, 2023 · 4 comments
Closed

Websockets: send multi frames #10413

mikeduglas opened this issue Feb 4, 2023 · 4 comments
Assignees
@bagder
Labels
WebSocket

Comments

@mikeduglas
Copy link
Contributor

I did this

I want to send multiple frames (not fragments).
AFAIK 1st frame must not have CURLWS_CONT bit set and FIN bit aslo must be 0. All other frames must have CURLWS_CONT bit set, also last frame must have FIN bit set.

Now, in ws_packethead function, FIN bit is always set to 1 if CURLWS_CONT bit is not set. So the server gets only first frame and ignores all other frames.

I expected the following

curl/libcurl version

7.87.1-DEV

operating system

Windows 11
@mikeduglas
Copy link
Contributor Author

Seems I found a bug in ws_packethead, see the comment inside the final "else" clause, now I'm able to send multi frame packets.

  if(!(flags & CURLWS_CONT)) {
    /* if not marked as continuing, assume this is the final fragment */
    firstbyte |= WSBIT_FIN | opcode;
    ws->ws.contfragment = FALSE;
    infof(data, "WS: if not marked as continuing, assume this is the final fragment");
  }
  else if(ws->ws.contfragment) {
    /* the previous fragment was not a final one and this isn't either, keep a
       CONT opcode and no FIN bit */
    firstbyte |= WSBIT_OPCODE_CONT;
    infof(data, "WS: the previous fragment was not a final one and this isn't either, keep a CONT opcode and no FIN bit");
  }
  else {
      firstbyte = opcode;   // <--- I added this line, otherwise 1st frame loses the opcode (TEXT or BINARY) and is always sent as continuation frame (firstbyte is 0 that means WSBIT_OPCODE_CONT )
      ws->ws.contfragment = TRUE;
    infof(data, "WS: ws->ws.contfragment = TRUE;");
  }

@mikeduglas
Copy link
Contributor Author

mikeduglas commented Feb 5, 2023 •
edited

Above solution is incomplete, last frame must be sent with CONT opcode, so here is the code that seems to work properly:

  if(!(flags & CURLWS_CONT)) {
    if (!ws->ws.contfragment)
      /* if not marked as continuing, assume this is the final fragment */
      firstbyte |= WSBIT_FIN | opcode;
    else
      /* if marked as continuing, assume this is the final fragment; keep a CONT opcode and set FIN bit */
      firstbyte |= WSBIT_FIN | WSBIT_OPCODE_CONT;
    ws->ws.contfragment = FALSE;
  }
  else if(ws->ws.contfragment) {
    /* the previous fragment was not a final one and this isn't either, keep a
       CONT opcode and no FIN bit */
    firstbyte |= WSBIT_OPCODE_CONT;
  }
  else {
      firstbyte = opcode;   // first frame: preserve opcode
      ws->ws.contfragment = TRUE;
  }

I am sending multi frames in this way:

send(frame1, CURLWS_BINARY | CURLWS_CONT) // sending as BINARY with FIN==0
send(frame2, CURLWS_BINARY | CURLWS_CONT) // sending as CONT with FIN==0
send(frame2, CURLWS_BINARY | CURLWS_CONT) // sending as CONT with FIN==0
...
send(lastframe, CURLWS_BINARY) // sending as CONT with FIN==1

@bagder
Copy link
Member

bagder commented Feb 6, 2023

Thanks for the patch @mikeduglas! Can you turn that into a proper PR or would you prefer I do it?

@bagder bagder self-assigned this Feb 6, 2023
@mikeduglas
Copy link
Contributor Author

@bagder Please do it yourself, I can't.

bagder pushed a commit that referenced this issue Feb 6, 2023
@mikeduglas @bagder
ws: fix multiframe handling
434408a 
Fixes #10413
@bagder bagder mentioned this issue Feb 6, 2023
Closed
bagder pushed a commit that referenced this issue Feb 6, 2023
@mikeduglas @bagder
ws: fix multiframe send handling
ad55b23 
Fixes #10413
Closes #10420
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
@mikeduglas @bch
ws: fix multiframe send handling
87d370a 
Fixes curl#10413
Closes curl#10420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
WebSocket
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ws: fix multiframe send handling curl/curl
2 participants
@bagder @mikeduglas