JianyuWang0623 commented on PR #3595: URL: https://github.com/apache/nuttx-apps/pull/3595#issuecomment-4900902305
@Old-Ding Thanks for the fix — the analysis of the desktop-name desync looks correct to me. One concern about the **Testing** section: the listed checks (`git diff --check`, `git show --check`) only validate formatting/whitespace, not the actual behavior this PR fixes. Since the bug is a runtime protocol-desync issue, could you share how it was reproduced and verified? Specifically it would help to have: 1. **Repro setup** — which VNC server was used, and how a desktop name longer than `sizeof(conn->name) - 1` was produced (server name, or how you forced an oversized `desktop-name-length`). 2. **Before-fix evidence** — logs showing the desync, e.g. the `vncviewer: unknown message type ...` / `-EPROTO` path or a garbled framebuffer update caused by the leftover name bytes being parsed as the next message. 3. **After-fix evidence** — the same session working, including the `vncviewer: server desktop: "..." WxH` line and a stable framebuffer stream afterwards. Also, since this touches parsing of an attacker-controllable length field, it would be good to confirm the behavior when the server advertises a very large `name-length` (the code now drains `name_len - copy_len` bytes unconditionally). A short note on whether that path was exercised would be reassuring. Finally, please run `./tools/checkpatch.sh -f system/vncviewer/rfb_protocol.c` once — the Testing note says it was skipped due to no bash/WSL, but the new `rfb_discard_exact` block should be verified against the coding style before merge. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
