From f23d840cee0aeaeeafc183fd80249983cb3602e4 Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:42:22 +0200 Subject: [PATCH] httpd: make the /cmd answer usable when a command fails or floods Three ways the answer could mislead the caller, all in the paths that matter most when something is wrong. The capture sat in write_char(), so anything printed through print_string_no_syslog() escaped it. That is exactly the replies of the syslog commands, which deliberately avoid the syslog path: "syslog on" answered with the bare address and no sentence around it, "syslog off" answered with nothing at all. Moving the capture one level down, into write_char_no_syslog(), catches both paths and costs nothing, since write_char() reaches it anyway. On a parse error send_bad_request() rewrote the buffer from the start, throwing away the very text that explains the error and replacing it with the 400 HTML page. Keeping the captured text and restating only the status keeps the parser's own message; "400 NO" is as long as "200 OK", so the header does not have to be rebuilt. An overlong listing was cut at the buffer end with nothing to show for it, so a clipped answer read as a short one. It now ends in a marker, and the reserved room is accounted for in the guard. While here: a command that configures something prints nothing, and an empty body reads the same as "nothing ran", so a successful silent command now answers "OK". No internal RAM and no xdata; 174 bytes of BANK1, which has 7356 left. --- httpd/httpd.c | 27 ++++++++++++++++++++++----- rtl837x_common.h | 4 ++++ rtlplayground.c | 14 +++++++++++--- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index efba40c0..50de43ed 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -16,12 +16,17 @@ #define CMARK_S 6 +/* Answer to POST /cmd. Kept as a macro so that its length is known at compile + * time: an unchanged slen afterwards means the command printed nothing. */ +#define CMD_RESPONSE_HEADER "HTTP/1.1 200 OK\r\nConnection: close\r\n" \ + "Content-Type: text/plain\r\n\r\n" + #pragma codeseg BANK1 #pragma constseg BANK1 extern volatile __xdata uint8_t sfr_data[4]; extern volatile __xdata uint32_t ticks; -extern __xdata uint8_t cmd_capture; /* owned by rtlplayground.c, see write_char() */ +extern __xdata uint8_t cmd_capture; /* owned by rtlplayground.c, see write_char_no_syslog() */ extern __code uint8_t * __code hex; extern __code struct f_data f_data[]; extern __code char * __code mime_strings[]; @@ -626,14 +631,26 @@ static void handle_firmware_fragment(__xdata uint8_t *p) static void run_cmd_body(__xdata uint8_t *body) { - slen = strtox(outbuf, "HTTP/1.1 200 OK\r\nConnection: close\r\n" \ - "Content-Type: text/plain\r\n\r\n"); + slen = strtox(outbuf, CMD_RESPONSE_HEADER); cmd_capture = 1; execute_commands(body); + if (cmd_capture == 2) + slen += strtox(outbuf + slen, CMD_TRUNCATED); cmd_capture = 0; + /* Commands that configure something print nothing at all. Saying + * so beats an empty body, which reads the same as "nothing ran". + * Only on success: a silent failure must not answer with "OK". */ + if (err_status == ERR_OK && slen == sizeof(CMD_RESPONSE_HEADER) - 1) + slen += strtox(outbuf + slen, "OK\n"); + /* On a parse error keep what the parser printed, because that text + * is the explanation, and only restate the status. "400 NO" is as + * long as "200 OK", so the header does not have to be rebuilt. */ if (err_status != ERR_OK) { - send_bad_request(); - return; + outbuf[9] = '4'; + outbuf[10] = '0'; + outbuf[11] = '0'; + outbuf[13] = 'N'; + outbuf[14] = 'O'; } } diff --git a/rtl837x_common.h b/rtl837x_common.h index 77a31b13..c15eced8 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -37,6 +37,10 @@ extern __xdata uint8_t sbuf[SBUF_SIZE]; // Size of the TCP Output buffer #define TCP_OUTBUF_SIZE 2500 +// Appended when a captured command wrote more than the output buffer holds, +// so a clipped listing is visibly clipped instead of silently short. +#define CMD_TRUNCATED "\n[output truncated]\n" + // Size of the port name, including the terminating null byte #define PORT_NAME_SIZE 32 diff --git a/rtlplayground.c b/rtlplayground.c index aa41d9e8..b78cac97 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -248,6 +248,17 @@ extern __xdata uint16_t slen; void write_char_no_syslog(char c) { + /* Capturing sits here rather than in write_char() so that the messages + * printed through print_string_no_syslog() are captured too: those are + * the replies of the syslog commands, which must not generate a syslog + * packet but do belong in the answer to a command sent over HTTP. */ + if (cmd_capture) { + if (slen < TCP_OUTBUF_SIZE - sizeof(CMD_TRUNCATED)) + outbuf[slen++] = c; + else + cmd_capture = 2; /* out of room, httpd says so */ + } + do { } while (tx_buf_empty == 0); if (c =='\n') { @@ -262,9 +273,6 @@ void write_char_no_syslog(char c) void write_char(char c) { - if (cmd_capture && slen < TCP_OUTBUF_SIZE - 1) - outbuf[slen++] = c; - write_char_no_syslog(c); if (syslog_state.enabled) {