Skip to content

Commit 10dd71a

Browse files
committed
Improve HTTP upload handling and TFTP checks
Increase upload filename buffer and strengthen HTTP POST/upload handling: copy POST body segments into a dynamic buffer, enforce Content-Length limits (return 413 if too large), and accumulate request_data_length before calling HandlePost. Add includes guarded by CONFIG_HTTPD_ENABLE_UPLOAD, improve logging/debug messages, and replace direct pointers with explicit uint8_t* casts for dynamic_content_. Update upload flow to assert FlashCodeInstall, check filename/size, handle erase/write errors, and use the updated WriteChunk signature with written-byte reporting. Remove legacy progress symbol helper and tidy formatting. Also fix TFTP file-name comparison to use firmware::kFileName and firmware::kFileNameLength constants.
1 parent 496b9cb commit 10dd71a

3 files changed

Lines changed: 44 additions & 32 deletions

File tree

lib-remoteconfig/include/httpd/httpdhandlerequest.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class HttpDeamonHandleRequest
8686
char* firmwarefile_name_{nullptr};
8787
char* receive_buffer_{nullptr};
8888
const uint8_t* content_{nullptr};
89-
char upload_filename_[16];
89+
char upload_filename_[32];
9090

9191
http::Status status_{http::Status::kUnknownError};
9292
http::RequestMethod request_method_{http::RequestMethod::kUnknown};

lib-remoteconfig/src/httpd/httpdhandlerequest.cpp

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@
5151
#include "http/json_infos.h"
5252
#include "network_tcp.h"
5353
#include "network_iface.h"
54+
#if defined(CONFIG_HTTPD_ENABLE_UPLOAD)
55+
#include "firmware.h"
56+
#include "flashcodeinstall.h"
57+
#include "display.h" // IWYU pragma: keep
58+
#endif
5459
#include "firmware/debug/debug_dump.h"
5560
#include "firmware/debug/debug_debug.h"
5661

@@ -72,7 +77,7 @@ void HttpDeamonHandleRequest::HandleRequest(uint32_t bytes_received, char* recei
7277

7378
const char* status_msg = "OK";
7479

75-
DEBUG_PRINTF("%u: status_=%u", connection_handle_, static_cast<uint32_t>(status_));
80+
DEBUG_PRINTF("%u: status=%u, bytes received=%u", connection_handle_, static_cast<uint32_t>(status_), bytes_received);
7681

7782
// The HTTP handler keeps state across TCP segments (e.g. POST body arriving later).
7883
// status_ == UNKNOWN_ERROR means "we are not currently processing an in-progress request".
@@ -89,8 +94,13 @@ void HttpDeamonHandleRequest::HandleRequest(uint32_t bytes_received, char* recei
8994
} else if (request_method_ == http::RequestMethod::kPost) {
9095
// If POST has Content-Length but no data in this segment,
9196
// we must wait for next TCP segment(s).
97+
if (request_content_length_ > sizeof(dynamic_content_)) {
98+
status_ = http::Status::kRequestEntityTooLarge;
99+
DEBUG_PUTS("Content too large.");
100+
}
101+
92102
if ((request_content_length_ != 0U) && (request_data_length_ == 0U)) {
93-
DEBUG_PUTS("There is a POST header only -> no data");
103+
DEBUG_PRINTF("There is a POST header only -> no data, request_content_length_=%u", request_content_length_);
94104
DEBUG_EXIT();
95105
return;
96106
}
@@ -101,16 +111,19 @@ void HttpDeamonHandleRequest::HandleRequest(uint32_t bytes_received, char* recei
101111
} else if ((status_ == http::Status::kOk) && (request_method_ == http::RequestMethod::kPost)) {
102112
// Follow-up TCP segment containing POST body data.
103113
// We treat the new receive_buffer as body data.
104-
file_data_ = receive_buffer_;
105-
request_data_length_ = bytes_received_;
106114

107-
status_ = HandlePost();
115+
memcpy(&dynamic_content_[request_data_length_], receive_buffer_, bytes_received_);
116+
117+
request_data_length_ += bytes_received_;
108118

109119
// If we haven't received the full body yet, wait for more segments.
110120
if (request_data_length_ < request_content_length_) {
111121
DEBUG_EXIT();
112122
return;
113123
}
124+
125+
file_data_ = dynamic_content_;
126+
status_ = HandlePost();
114127
}
115128
#if defined(ENABLE_METHOD_DELETE)
116129
else if ((status_ == http::Status::kOk) && (request_method_ == http::RequestMethod::DELETE)) {
@@ -155,7 +168,10 @@ void HttpDeamonHandleRequest::HandleRequest(uint32_t bytes_received, char* recei
155168

156169
request_content_type_ = http::ContentTypes::kTextHtml;
157170
content_ = reinterpret_cast<uint8_t*>(dynamic_content_);
158-
content_size_ = static_cast<uint32_t>(snprintf(dynamic_content_, sizeof(dynamic_content_), "%u %s\n", static_cast<unsigned>(status_), status_msg));
171+
content_size_ = static_cast<uint32_t>(snprintf(dynamic_content_, sizeof(dynamic_content_),
172+
"%u %s\n",
173+
static_cast<unsigned>(status_),
174+
status_msg));
159175

160176
const auto kHeaderLength =
161177
static_cast<uint32_t>(snprintf(receive_buffer_, network::tcp::kTcpDataMss,
@@ -165,7 +181,11 @@ void HttpDeamonHandleRequest::HandleRequest(uint32_t bytes_received, char* recei
165181
"Content-Length: %u\r\n"
166182
"Connection: close\r\n"
167183
"\r\n",
168-
static_cast<unsigned int>(status_), status_msg, network::iface::HostName(), http::kContentType[static_cast<uint32_t>(request_content_type_)], static_cast<unsigned int>(content_size_)));
184+
static_cast<unsigned int>(status_),
185+
status_msg,
186+
network::iface::HostName(),
187+
http::kContentType[static_cast<uint32_t>(request_content_type_)],
188+
static_cast<unsigned int>(content_size_)));
169189

170190
network::tcp::Send(connection_handle_, reinterpret_cast<const uint8_t*>(receive_buffer_), kHeaderLength);
171191
} else {
@@ -184,12 +204,11 @@ void HttpDeamonHandleRequest::HandleRequest(uint32_t bytes_received, char* recei
184204
gzip_ ? "gzip" : "identity",
185205
http::kContentType[static_cast<uint32_t>(request_content_type_)],
186206
static_cast<unsigned int>(content_size_), (content_ == reinterpret_cast<uint8_t*>(dynamic_content_)) ? "no-cache" : "max-age=3600",
187-
(content_ == reinterpret_cast<uint8_t*>(dynamic_content_)) ? timing::Millis() : _TIME_STAMP_)
188-
);
207+
(content_ == reinterpret_cast<uint8_t*>(dynamic_content_)) ? timing::Millis() : _TIME_STAMP_));
189208

190209
network::tcp::Send(connection_handle_, reinterpret_cast<const uint8_t*>(receive_buffer_), kHeaderLength);
191210

192-
DEBUG_PRINTF("content_size_=%u, %s", content_size_, (content_ == dynamic_content_) ? "Dynamic" : "Static");
211+
DEBUG_PRINTF("content_size_=%u, %s", content_size_, (content_ == reinterpret_cast<uint8_t*>(dynamic_content_)) ? "Dynamic" : "Static");
193212
}
194213

195214
if (content_size_ != 0U) {
@@ -491,7 +510,7 @@ http::Status HttpDeamonHandleRequest::HandleGet() {
491510

492511
http::Status HttpDeamonHandleRequest::HandlePost() {
493512
DEBUG_ENTRY();
494-
DEBUG_PRINTF("bytes_received_=%d, request_data_length_=%u, request_content_length_=%u", bytes_received_, request_data_length_, request_content_length_);
513+
DEBUG_PRINTF("bytes_received_=%u, request_data_length_=%u, request_content_length_=%u", bytes_received_, request_data_length_, request_content_length_);
495514
DEBUG_PUTS(uri_);
496515

497516
if (request_content_type_ == http::ContentTypes::kApplicationJson) {
@@ -549,17 +568,6 @@ http::Status HttpDeamonHandleRequest::HandlePostJSON() {
549568
}
550569

551570
#if defined(CONFIG_HTTPD_ENABLE_UPLOAD)
552-
static void ShowProgressSymbol() {
553-
static constexpr char kProgressSymbols[] = {'/', '-', '\\', '|'};
554-
static uint32_t progress_symbols_index = 0;
555-
556-
printf("%c\r", kProgressSymbols[progress_symbols_index++]);
557-
558-
if (progress_symbols_index >= sizeof(kProgressSymbols)) {
559-
progress_symbols_index = 0;
560-
}
561-
}
562-
563571
http::Status HttpDeamonHandleRequest::HandlePostUpload() {
564572
DEBUG_ENTRY();
565573

@@ -569,21 +577,26 @@ http::Status HttpDeamonHandleRequest::HandlePostUpload() {
569577
printf("Firmware: %s -> %u bytes\n", upload_filename_, upload_size_);
570578

571579
if (strncmp(upload_filename_, firmware::kFileName, sizeof(upload_filename_)) != 0) {
580+
puts("Wrong firmware file name.");
581+
DEBUG_EXIT();
572582
return http::Status::kBadRequest;
573583
}
574584

575585
if ((upload_size_ >= 64) && (upload_size_ > (FIRMWARE_MAX_SIZE))) {
586+
puts("Wrong firmware file size.");
587+
DEBUG_EXIT();
576588
return http::Status::kRequestEntityTooLarge;
577589
}
578590

591+
assert(FlashCodeInstall::Get() != nullptr);
579592
if (!(FlashCodeInstall::Get()->Erase(upload_size_))) {
580593
puts("Erase failed.");
581594
DEBUG_EXIT();
582595
return http::Status::kInternalServerError;
583596
}
584597

585598
content_size_ = static_cast<uint32_t>(snprintf(dynamic_content_, sizeof(dynamic_content_), "{\"status\":\"ok\"}"));
586-
content_ = dynamic_content_;
599+
content_ = reinterpret_cast<uint8_t*>(dynamic_content_);
587600
request_content_type_ = http::ContentTypes::kApplicationJson;
588601

589602
DEBUG_EXIT();
@@ -592,11 +605,12 @@ http::Status HttpDeamonHandleRequest::HandlePostUpload() {
592605

593606
if (request_content_type_ == http::ContentTypes::kApplicationOctetStream) {
594607
if (part_uri[0] == 0) {
595-
ShowProgressSymbol();
596608
Display::Get()->Progress();
597609

598-
if (!(FlashCodeInstall::Get()->WriteChunk(reinterpret_cast<uint8_t*>(file_data_), request_data_length_))) {
599-
puts("WriteChunk failed.");
610+
uint32_t data_written;
611+
printf("%u\n", request_data_length_);
612+
if (!(FlashCodeInstall::Get()->WriteChunk(reinterpret_cast<uint8_t*>(file_data_), request_data_length_, data_written))) {
613+
DEBUG_PRINTF("WriteChunk failed. Data written:%u bytes", data_written);
600614
DEBUG_EXIT();
601615
return http::Status::kInternalServerError;
602616
}
@@ -609,19 +623,17 @@ http::Status HttpDeamonHandleRequest::HandlePostUpload() {
609623
}
610624

611625
if (memcmp(part_uri, "_complete", 10) == 0) {
612-
putchar('\n');
613-
614626
uint32_t write_count;
615627
if (!(FlashCodeInstall::Get()->WriteChunkComplete(write_count))) {
616-
puts("WriteChunkComplete failed.");
628+
DEBUG_PUTS("WriteChunkComplete failed.");
617629
DEBUG_EXIT();
618630
return http::Status::kInternalServerError;
619631
}
620632

621633
printf("Written bytes -> %u [%s]\n", write_count, write_count == upload_size_ ? "Ok" : "Wrong");
622634

623635
content_size_ = static_cast<uint32_t>(snprintf(dynamic_content_, sizeof(dynamic_content_), "{\"status\":\"ok\"}"));
624-
content_ = dynamic_content_;
636+
content_ = reinterpret_cast<uint8_t*>(dynamic_content_);
625637
request_content_type_ = http::ContentTypes::kApplicationJson;
626638
upload_size_ = 0;
627639
upload_filename_[0] = '\0';

lib-remoteconfig/src/tftp/tftpfileserver.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ bool TFTPFileServer::FileCreate(const char* file_name, tftp::Mode mode) {
7272
return false;
7373
}
7474

75-
if (strncmp(firmware::FILE_NAME, file_name, firmware::FILE_NAME_LENGTH) != 0) {
75+
if (strncmp(firmware::kFileName, file_name, firmware::kFileNameLength) != 0) {
7676
DEBUG_EXIT();
7777
return false;
7878
}

0 commit comments

Comments
 (0)