From 772e9dc52608a22eb8cad94abf8205724356b404 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Tue, 18 Aug 2026 14:14:14 -0500 Subject: [PATCH 1/3] httpd: parse a configuration upload once the whole request has arrived Saving the configuration works in Chrome and fails in Firefox, and the difference is only how each browser splits the request. The handler erased the config sector as soon as the request started and then parsed the multipart body one TCP segment at a time, which requires every boundary and every part header to fall inside a single segment. Firefox splits inside a part header, so the parser lost its place and the erased sector was left holding a truncated body or nothing at all. A single-burst scripted post lost the whole body the same way. The configuration is limited to two kilobytes, so the whole request body now accumulates in xdata and is parsed only after the closing boundary has arrived. The parts are walked in one pass, the part carrying a filename is written to a freshly erased sector, and the client receives a 200 instead of the previous silent close. No segmentation can confuse this, since the parser only ever sees a complete body. Locating the closing boundary first also bounds the walk over the parts, since none can lie beyond it, so the length of the buffer is no longer the bound and the test for the two trailing dashes is unnecessary. The walk matches at offsets inside a buffer that is not terminated, so neither existing helper fits: strcmp() goes on to compare the byte after the match and is_word_x() demands a separator there. Add strstart() and strstart_x() for that case, and use strlen_x() for the boundary length. The firmware upload path still streams, since a megabyte cannot be buffered, and is untouched. --- httpd/httpd.c | 100 ++++++++++++++++++++++++++++++++++++++++++++--- rtl837x_common.h | 2 + rtlplayground.c | 26 ++++++++++++ 3 files changed, 123 insertions(+), 5 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index d856ccd..3a54aca 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -41,6 +41,14 @@ __xdata uint32_t cont_addr; // HTTP header properties __xdata uint8_t boundary[72]; + +// a client may split the request anywhere, including inside a boundary or a +// part header, so a configuration upload is parsed only once it is complete +#define CONFIG_UPLOAD_BUF 2560 +__xdata uint8_t config_upload; +__xdata uint8_t config_buf[CONFIG_UPLOAD_BUF]; +__xdata uint16_t cfg_pos, cfg_hdr, cfg_body, cfg_end, cfg_last; +__xdata uint8_t cfg_bl; __xdata uint8_t *content_type = 0; __xdata uint8_t *session = 0; @@ -77,6 +85,7 @@ inline uint8_t is_separator(uint8_t c) void httpd_init(void) __banked { + config_upload = 0; // xdata is not zeroed by the startup code __xdata struct httpd_state * __xdata s = &(uip_conn->appstate); // Start listening to port 80 uip_listen(HTONS(80)); @@ -315,6 +324,62 @@ void gen_random_bytes(__xdata uint8_t *b, uint8_t bytes) } +/* 0: body incomplete, 1: configuration stored, 2: malformed */ +static uint8_t config_take(void) +{ + cfg_bl = strlen_x(boundary); + + // the body is complete once the closing boundary has arrived + cfg_last = 0; + while (1) { + if (cfg_last + cfg_bl + 1 >= write_len) + return 0; + if (strstart_x(&config_buf[cfg_last], boundary) + && strstart(&config_buf[cfg_last + cfg_bl], "--")) + break; + cfg_last++; + } + + // every part lies ahead of the closing boundary, so it bounds the walk + cfg_pos = 0; + while (cfg_pos < cfg_last) { + if (!strstart_x(&config_buf[cfg_pos], boundary)) { + cfg_pos++; + continue; + } + cfg_hdr = cfg_pos + cfg_bl; + cfg_body = cfg_hdr; + while (1) { + if (cfg_body + 3 >= cfg_last) + return 2; + if (strstart(&config_buf[cfg_body], "\r\n\r\n")) + break; + cfg_body++; + } + cfg_end = cfg_body; + cfg_body += 4; + // reaching cfg_last is a match: the last part ends at the closing boundary + while (cfg_end < cfg_last && !strstart_x(&config_buf[cfg_end], boundary)) + cfg_end++; + while (cfg_hdr + 8 < cfg_body) { + // the part carrying a filename holds the configuration + if (strstart(&config_buf[cfg_hdr], "filename")) { + config_buf[cfg_end] = 0; + flash_region.addr = CONFIG_START; + flash_sector_erase(); + flash_region.addr = CONFIG_START; + flash_region.len = cfg_end - cfg_body + 1; + flash_write_bytes(config_buf + cfg_body); + return 1; + } + cfg_hdr++; + } + cfg_pos = cfg_end; + } + return 2; +} + + /* * Reads post data from the http stream and writes it into flash memory * Input: the current position in the TCP buffer (uip_appdata) @@ -437,6 +502,7 @@ void handle_post(void) return; } print_string("Firmware upload started."); + config_upload = 0; uptr = FIRMWARE_UPLOAD_START; verify_crc = 1; max_upload = 1024576; @@ -445,12 +511,10 @@ void handle_post(void) send_unauthorized(); return; } - dbg_string("Configuration upload, erasing config mem!\n"); - uptr = CONFIG_START; + dbg_string("Configuration upload\n"); verify_crc = 0; - max_upload = 2048; - flash_region.addr = CONFIG_START; - flash_sector_erase(); + config_upload = 1; + write_len = 0; } // Check for other POST requests, which are not multipart, below } else { @@ -504,6 +568,32 @@ void handle_post(void) send_bad_request(); return; } + if (config_upload) { + cfg_pos = uip_len - (p - uip_appdata); + if (write_len + cfg_pos >= CONFIG_UPLOAD_BUF) { + print_string("Configuration too large, aborting.\n"); + config_upload = 0; + s->tstate = TSTATE_NONE; + send_bad_request(); + return; + } + memcpy(config_buf + write_len, p, cfg_pos); + write_len += cfg_pos; + uint8_t taken = config_take(); + + if (!taken) { + s->tstate = TSTATE_MULTIPART; + return; + } + config_upload = 0; + s->tstate = TSTATE_NONE; + if (taken == 2) { + send_bad_request(); + return; + } + slen = strtox(outbuf, "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n"); + return; + } // We skip the intial parts as part of the header do { p = skip_boundary(p); diff --git a/rtl837x_common.h b/rtl837x_common.h index 66df88a..44e06d0 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -164,6 +164,8 @@ uint16_t strlen_x(register __xdata const char *s); uint16_t strtox(register __xdata uint8_t *dst, register __code const char *s); uint16_t strcpy(register __xdata uint8_t *dst, register const char *s); char strcmp(register __xdata const uint8_t *a, register __code const uint8_t *b); +bool strstart(__xdata const uint8_t *a, __code const uint8_t *b); +bool strstart_x(__xdata const uint8_t *a, __xdata const uint8_t *b); void tcpip_output(void); uint8_t read_flash(uint8_t bank, __code uint8_t *addr); void get_random_32(void); diff --git a/rtlplayground.c b/rtlplayground.c index b9cc05d..e08c09a 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -360,6 +360,32 @@ char strcmp(register __xdata const uint8_t *a, register __code const uint8_t *b) } +/* + * True when b is a prefix of a. Unlike strcmp() the byte after the match is not + * compared, and unlike is_word_x() it need not be a separator. + */ +bool strstart(__xdata const uint8_t *a, __code const uint8_t *b) +{ + uint8_t i = 0; + + while (b[i] && (b[i] == a[i])) + i++; + + return !b[i]; +} + + +bool strstart_x(__xdata const uint8_t *a, __xdata const uint8_t *b) +{ + uint8_t i = 0; + + while (b[i] && (b[i] == a[i])) + i++; + + return !b[i]; +} + + void print_short(uint16_t a) { // allocating the registers first improves the sdcc code here From e8e7fcbe7fb2a46a5d8dc445a657b8d035e40789 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Mon, 24 Aug 2026 15:45:25 -0500 Subject: [PATCH 2/3] httpd: send the config tail from the config sector, not a bare offset send_config() streams a configuration larger than the TCP output buffer through the cont_addr/cont_len continuation, but set cont_addr to the offset within the config instead of a flash address, so the tail was read from code space. The file server sets cont_addr absolute; do the same by adding CONFIG_START. The bug was unreachable while CONFIG_UPLOAD_BUF capped uploads below TCP_OUTBUF_SIZE, but a stored config near the full sector exposes it. --- httpd/page_impl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpd/page_impl.c b/httpd/page_impl.c index bf3e268..3e2e696 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -822,7 +822,7 @@ found_end: if (valid_len > (TCP_OUTBUF_SIZE - slen)) { cont_len = valid_len - (TCP_OUTBUF_SIZE - slen); valid_len = TCP_OUTBUF_SIZE - slen; - cont_addr = valid_len; + cont_addr = CONFIG_START + valid_len; } flash_region.addr = CONFIG_START; From cefe48fe5125fca1c1712af8c5dd541084fe3231 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Mon, 24 Aug 2026 15:45:25 -0500 Subject: [PATCH 3/3] httpd: size the config upload buffer for a full config sector CONFIG_UPLOAD_BUF at 2560 capped a configuration upload at about 2.2K while the config sector holds 4K. Size the buffer as CONFIG_LEN plus room for the multipart framing so the whole sector is usable. config_take() wrote the payload without checking it against the sector: safe before only because the buffer could not hold an oversized one. Reject a payload that does not fit CONFIG_LEN, terminator included, instead of writing past the erased sector. --- httpd/httpd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index 3a54aca..ff4eff9 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -43,8 +43,9 @@ __xdata uint32_t cont_addr; __xdata uint8_t boundary[72]; // a client may split the request anywhere, including inside a boundary or a -// part header, so a configuration upload is parsed only once it is complete -#define CONFIG_UPLOAD_BUF 2560 +// part header, so a configuration upload is parsed only once it is complete; +// sized for a full config sector plus the multipart framing around it +#define CONFIG_UPLOAD_BUF (CONFIG_LEN + 384) __xdata uint8_t config_upload; __xdata uint8_t config_buf[CONFIG_UPLOAD_BUF]; __xdata uint16_t cfg_pos, cfg_hdr, cfg_body, cfg_end, cfg_last; @@ -364,6 +365,9 @@ static uint8_t config_take(void) while (cfg_hdr + 8 < cfg_body) { // the part carrying a filename holds the configuration if (strstart(&config_buf[cfg_hdr], "filename")) { + // the payload plus its terminator must fit the sector + if (cfg_end - cfg_body + 1 > CONFIG_LEN) + return 2; config_buf[cfg_end] = 0; flash_region.addr = CONFIG_START; flash_sector_erase();