From 772e9dc52608a22eb8cad94abf8205724356b404 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Tue, 18 Aug 2026 14:14:14 -0500 Subject: [PATCH] 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