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

Wrong length header sent by httpd_ws_send_frame_async() for more than 65534 bytes (IDFGH-4360) #6196

Closed
adeuring opened this issue Dec 2, 2020 · 1 comment

Comments

@adeuring
Copy link

adeuring commented Dec 2, 2020

I noticed that httpd_ws_send_frame_async() does not set the length bytes of the header correctly when the frame buffer is longer than 65534 bytes, i.e., when the length header is 8 bytes long.

The following patch fixes the problem:

--- a/components/esp_http_server/src/httpd_ws.c
+++ b/components/esp_http_server/src/httpd_ws.c
@@ -295,9 +295,10 @@ esp_err_t httpd_ws_send_frame_async(httpd_handle_t hd, int fd, httpd_ws_frame_t
     } else {
         header_buf[1] = 127;                /* Length for 64 bits */
         uint8_t shift_idx = sizeof(uint64_t) - 1; /* Shift index starts at 7 */
-        for (int8_t idx = 2; idx > 9; idx--) {
+        for (int8_t idx = 2; idx <= 9; idx++) {
             /* Now do shifting (be careful of endianess, i.e. when buffer index is 2, frame length shift index is 7) */
-            header_buf[idx] = (frame->len >> (uint8_t)(shift_idx * 8)) & 0xffU;
+          uint64_t size_long = frame->len;
+          header_buf[idx] = (size_long >> (shift_idx * 8)) & 0xffU;
             shift_idx--;
         }
         tx_len = 10;

The change of the "for" statement is obvious, I think. But I needed some time to figure out that a shift operation on a 32 bit value by more than 32 bits is effectively a rotate op, hence the introduction of the uint64_t variabe.

@github-actions github-actions bot changed the title Wrong length header sent by httpd_ws_send_frame_async() for more than 65534 bytes Wrong length header sent by httpd_ws_send_frame_async() for more than 65534 bytes (IDFGH-4360) Dec 2, 2020
@Alvin1Zhang
Copy link
Collaborator

Thanks for reporting and sharing the fix, we will look into.

@igrr igrr closed this as completed in c415c6f Feb 1, 2021
loganfin pushed a commit to Lumenaries/esp_http_server that referenced this issue Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants