Merge pull request #359 from bloqaudio/fix/upload-preamble-split

httpd: buffer a firmware upload's part header before streaming

httpd/httpd.c

@@ -49,6 +49,8 @@ __xdata uint8_t boundary[72];
 __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;
+// part-header bytes buffered so far; accumulates across TCP segments
+__xdata uint16_t pre_acc;
 __xdata uint8_t cfg_bl;
 __xdata uint8_t * __xdata content_type = 0;
 __xdata uint8_t * __xdata session = 0;
@@ -60,6 +62,9 @@ __xdata uint32_t max_upload;
 __xdata uint16_t short_parsed;
 
 __xdata char passwd[21];
+// Set when a verified firmware upload awaits its response ACK, after
+// which the chip resets to apply the staged image
+__xdata uint8_t fw_reset_pending;
 __xdata char session_id[SESSION_ID_LENGTH + 1];
 __xdata uint8_t authenticated;
 __xdata uint32_t now;
@@ -91,6 +96,7 @@ void httpd_init(void) __banked
 	// Start listening to port 80
 	uip_listen(HTONS(80));
 	s->tstate = TSTATE_CLOSED;
+	fw_reset_pending = 0; // xdata is not zeroed by the startup code
 }
 
 
@@ -240,17 +246,6 @@ void send_unauthorized(void)
 }
 
 
-__xdata uint8_t *skip_boundary(__xdata uint8_t *p)
-{
-	while (*p) {
-		if (is_word_x(p, boundary))
-			return p + strlen_x(boundary);
-		p++;
-	}
-	return p;
-}
-
-
 __xdata uint8_t *scan_header(__xdata uint8_t * __xdata p)
 {
 	content_type = 0;
@@ -386,21 +381,47 @@ static uint8_t config_take(void)
 }
 
 
+// unlike scan_header(), keeps no auth state, so it may run on every buffered segment
+static uint16_t preamble_payload_start(uint16_t n)
+{
+	uint16_t pos;
+
+	for (pos = 0; pos + 24 <= n; pos++) {
+		if (strstart(&config_buf[pos], "application/octet-stream"))
+			break;
+	}
+	if (pos + 24 > n)
+		return 0;
+	pos += 24;
+	while (pos + 3 < n && !strstart(&config_buf[pos], "\r\n\r\n"))
+		pos++;
+	if (pos + 3 >= n)
+		return 0;
+	return pos + 4;
+}
+
+
+// Source window for stream_upload(); filled by the caller before the call
+__xdata struct {
+	__xdata uint8_t *p;
+	uint16_t bptr;
+	uint16_t plen;
+} upload_settings;
+
 /*
  * Reads post data from the http stream and writes it into flash memory
- * Input: the current position in the TCP buffer (uip_appdata)
+ * Input: upload_settings, set by the caller
  * Returns 1: More data to read, 0: Upload complete, all parts reads
  */
-uint8_t stream_upload(uint16_t bptr)
+uint8_t stream_upload(void)
 {
-	__xdata uint8_t *p = uip_appdata;
 	__xdata struct httpd_state * __xdata s = &(uip_conn->appstate);
 
 	dbg_string("Stream_upload called: ");
-	dbg_short(bptr); dbg_char('\n');
+	dbg_short(upload_settings.bptr); dbg_char('\n');
 
 	do {
-		if (bptr >= uip_len) {
+		if (upload_settings.bptr >= upload_settings.plen) {
 			s->tstate = TSTATE_POST;
 			return 1;
 		}
@@ -413,17 +434,23 @@ uint8_t stream_upload(uint16_t bptr)
 			flash_write_bytes(flash_buf);
 			uptr += write_len;
 			write_len = 0;
-			// TODO: This is a bit premature, what about a nice web-page saying the device will reset???
 			if (verify_crc) {
 				dbg_string("CRC16: "); dbg_short(crc_final); dbg_char('\n');
+				// Both bodies are 33 bytes; Content-Length lets the
+				// browser complete the response without waiting for
+				// the connection close (which a reset would swallow)
 				if (crc_final == 0xb001) {
 					print_string("Checksum OK.\nUpload to flash done, will reset!\n");
-					// close connection to avoid retries by browser
+					slen = strtox(outbuf, "HTTP/1.1 200 OK\r\nContent-Length: 33\r\n"
-					uip_close();
+						"Content-Type: text/plain\r\n\r\n"
-					reset_chip();
+						"OK: checksum verified, rebooting\n");
+					// Reset once the response is fully ACKed
+					fw_reset_pending = 1;
 				} else {
 					print_string("Checksum incorrect! Aborting.\n");
-					uip_close();
+					slen = strtox(outbuf, "HTTP/1.1 400 Bad Request\r\nContent-Length: 33\r\n"
+						"Content-Type: text/plain\r\n\r\n"
+						"NO: checksum failed, not applied\n");
 				}
 			}
 			// Make sure there is a 0 at the end of the uploaded data
@@ -431,18 +458,15 @@ uint8_t stream_upload(uint16_t bptr)
 			flash_region.addr = uptr;
 			flash_region.len = 1;
 			flash_write_bytes(flash_buf);
-			if (bptr >= uip_len)
+			if (upload_settings.bptr >= upload_settings.plen)
 				return 0;
-			if(!verify_crc)
-				//ugly hack to signal connection finished after config upload.
-				uip_close();
 			return 1;
 		}
-		if (p[bptr] == boundary[bindex]) {
+		if (upload_settings.p[upload_settings.bptr] == boundary[bindex]) {
 			if (!bindex)
 				crc_final = crc_value;
-			crc16(p + bptr);
+			crc16(upload_settings.p + upload_settings.bptr);
-			bptr++;
+			upload_settings.bptr++;
 			bindex++;
 		} else {
 			if (bindex) {
@@ -450,8 +474,8 @@ uint8_t stream_upload(uint16_t bptr)
 				write_len += bindex;
 				bindex = 0;
 			}
-			crc16(p + bptr);
+			crc16(upload_settings.p + upload_settings.bptr);
-			flash_buf[write_len++] = p[bptr++];
+			flash_buf[write_len++] = upload_settings.p[upload_settings.bptr++];
 			if (write_len >= FLASH_PAGE_SIZE) {
 				dbg_string("len: "); dbg_short(write_len); dbg_char(' ');
 				dbg_string("CRC16: "); dbg_short(crc_value); dbg_char('\n');
@@ -512,6 +536,7 @@ void handle_post(void)
 			uptr = FIRMWARE_UPLOAD_START;
 			verify_crc = 1;
 			max_upload = 1024576;
+			pre_acc = 0;
 		} else if (is_word(request_path, "config")) {
 			if (!authenticated) {
 				send_unauthorized();
@@ -600,21 +625,21 @@ void handle_post(void)
 			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
+		cfg_pos = uip_len - (p - uip_appdata);
-		do {
+		if (pre_acc + cfg_pos >= CONFIG_UPLOAD_BUF) {
-			p = skip_boundary(p);
+			print_string("Firmware upload header too large, aborting.\n");
-			if (!*p) {
+			s->tstate = TSTATE_NONE;
-				s->tstate = TSTATE_MULTIPART;
+			send_bad_request();
-				return;
+			return;
-			}
+		}
-			p = scan_header(p);
+		memcpy(config_buf + pre_acc, p, cfg_pos);
-			if (!*p)
+		pre_acc += cfg_pos;
-				goto bad_request;
+		cfg_end = preamble_payload_start(pre_acc);
-			if (!content_type) // We are waiting for the part with the octet stream
+		if (!cfg_end) {
-				continue;
+			s->tstate = TSTATE_MULTIPART;
-		} while (!is_word(content_type, "application/octet-stream"));
+			return;
+		}
 		dbg_string("Have content octets\n");
-		p += 4; // Skip \r\n\r\n sequence at end of preamble of part
 
 		flash_init(0); // Re-initialize flash for non-DIO operation, otherwise flashing fails
 		set_sys_led_state(SYS_LED_FAST);
@@ -622,7 +647,14 @@ void handle_post(void)
 		crc_value = 0;
 		bindex = 0;
 		write_len = 0;
-		stream_upload(p - uip_appdata);
+		// A verdict is only built once the upload part completes;
+		// clear any stale response so the completion check in the
+		// appcall POST branch cannot send leftovers
+		slen = 0;
+		upload_settings.p = config_buf;
+		upload_settings.bptr = cfg_end;
+		upload_settings.plen = pre_acc;
+		stream_upload();
 
 		dbg_string("Done reading first fragment\n");
 		return;
@@ -633,9 +665,6 @@ void handle_post(void)
 	}
 	slen = strtox(outbuf, "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n");
 	return;
-bad_request:
-	send_bad_request();
-	return;
 }
 
 
@@ -699,11 +728,23 @@ void httpd_appcall(void)
 			cont_len -= slen;
 			cont_addr += slen;
 			s->tstate = TSTATE_TX;
+		} else if (fw_reset_pending) {
+			// The upload verdict has been fully ACKed by the client;
+			// now it is safe to reset and apply the staged image
+			print_string("Resetting to apply update\n");
+			reset_chip();
 		}
 	} else if (uip_newdata() && s->tstate == TSTATE_POST) {
 		// Check here maxupload by subtracting uip_len and close socekt if fails!
 		if (max_upload - uip_len > 0) {
-			stream_upload(0);
+			upload_settings.p = uip_appdata;
+			upload_settings.bptr = 0;
+			upload_settings.plen = uip_len;
+			stream_upload();
+			// A completed part with a built verdict must go out
+			// through the normal TX path
+			if (s->tstate == TSTATE_NONE && slen)
+				goto do_send;
 			write_char('.');
 		} else {
 			send_bad_request();