diff --git a/src/activities/settings/FontDownloadActivity.h b/src/activities/settings/FontDownloadActivity.h index 90d488a8..7686b24b 100644 --- a/src/activities/settings/FontDownloadActivity.h +++ b/src/activities/settings/FontDownloadActivity.h @@ -36,9 +36,9 @@ class FontDownloadActivity : public Activity { void render(RenderLock&&) override; bool preventAutoSleep() override { return state_ == LOADING_MANIFEST || state_ == DOWNLOADING || - // This is added because HTTPClient is a synchronous/blocking function, - // and blocks the main loop until the download is complete. - // So `activityManager.preventAutoSleep()` is never called during downloading + // The download is synchronous and blocks the main loop until it + // completes, so activityManager.preventAutoSleep() is never polled + // during downloading. state_ == COMPLETE || state_ == ERROR; } bool skipLoopDelay() override { return true; } diff --git a/src/network/HttpDownloader.cpp b/src/network/HttpDownloader.cpp index bd26a5ac..31a791cd 100644 --- a/src/network/HttpDownloader.cpp +++ b/src/network/HttpDownloader.cpp @@ -1,204 +1,201 @@ #include "HttpDownloader.h" -#include +#include #include -#include -#include -#include +#include #include +#include +#include #include -#include -#include - -#include "util/UrlUtils.h" +#include +#include namespace { -class FileWriteStream final : public Stream { - public: - FileWriteStream(FsFile& file, size_t total, HttpDownloader::ProgressCallback progress, bool* cancelFlag) - : file_(file), total_(total), progress_(std::move(progress)), cancelFlag_(cancelFlag) {} +// RX holds the response headers. 4096 fits real OPDS servers; GitHub's release +// CDN sends more and logs HTTP_HEADER "Buffer length is small", but that's +// non-fatal: the headers we read (Location, Content-Length) come first and +// survive. Smaller keeps contiguous heap free while WiFi and TLS are up. TX +// only carries our GET; the body streams in READ_CHUNK pieces. +constexpr int HTTP_RX_BUF = 4096; +constexpr int HTTP_TX_BUF = 1024; +// Per-socket-op timeout. Some OPDS download endpoints are slow to send headers +// (>15s) and chunked catalogs stall mid-body, so 15s killed them. 60s gives +// slow servers room. esp_http_client's timeout_ms is uint32, so unlike Arduino +// HTTPClient's uint16 setTimeout it doesn't silently truncate. +constexpr int HTTP_TIMEOUT_MS = 60000; +constexpr size_t READ_CHUNK = 2048; - size_t write(uint8_t byte) override { return write(&byte, 1); } +struct Sink { + std::function write; // returns false to abort the transfer + HttpDownloader::ProgressCallback progress; + bool* cancelFlag = nullptr; + size_t total = 0; + size_t downloaded = 0; +}; - size_t write(const uint8_t* buffer, size_t size) override { - // Write-through stream for HTTPClient::writeToStream with progress tracking. - if (cancelFlag_ && *cancelFlag_) { - writeOk_ = false; - return 0; - } - const size_t written = file_.write(buffer, size); - if (written != size) { - writeOk_ = false; - } - downloaded_ += written; - if (progress_ && total_ > 0) { - progress_(downloaded_, total_); - } - return written; +bool isRedirect(int status) { + return status == 301 || status == 302 || status == 303 || status == 307 || status == 308; +} + +// Streams a GET body through sink.write in READ_CHUNK pieces. Uses the manual +// open/fetch_headers/read path rather than esp_http_client_perform(): perform() +// pushes the whole body through an event callback and reports a chunked body +// that ends early as ESP_ERR_HTTP_INCOMPLETE_DATA, whereas the read loop streams +// large/slow files and surfaces a short read directly. +HttpDownloader::DownloadError runGet(const std::string& url, const std::string& username, const std::string& password, + Sink& sink) { + esp_http_client_config_t config = {}; + config.url = url.c_str(); + config.buffer_size = HTTP_RX_BUF; + config.buffer_size_tx = HTTP_TX_BUF; + config.timeout_ms = HTTP_TIMEOUT_MS; + // Verify HTTPS against the bundled CA roots. This build has esp-tls + // CONFIG_ESP_TLS_INSECURE off, so an unverified TLS handshake can't be set + // up at all; the model is public servers over verified https and local + // servers over plain http (esp_http_client picks the transport from the URL + // scheme, so http:// needs no cert config). The prior setInsecure() worked + // only because Arduino's ssl_client drives mbedtls directly. + config.crt_bundle_attach = esp_crt_bundle_attach; + config.keep_alive_enable = true; + + esp_http_client_handle_t client = esp_http_client_init(&config); + if (!client) { + LOG_ERR("HTTP", "client init failed"); + return HttpDownloader::HTTP_ERROR; } - int available() override { return 0; } - int read() override { return -1; } - int peek() override { return -1; } - void flush() override { file_.flush(); } + esp_http_client_set_header(client, "User-Agent", "CrossPoint-ESP32-" CROSSPOINT_VERSION); + if (!username.empty() && !password.empty()) { + // Preemptive Basic auth, like the prior addHeader; don't wait for a 401. + const std::string credentials = username + ":" + password; + const String header = "Basic " + base64::encode(credentials.c_str()); + esp_http_client_set_header(client, "Authorization", header.c_str()); + } - size_t downloaded() const { return downloaded_; } - bool ok() const { return writeOk_; } + // open()/read() does not auto-follow redirects (only perform() does), so step + // 30x responses manually. OPDS download endpoints and the GitHub release CDN + // both redirect. + esp_err_t err = esp_http_client_open(client, 0); + if (err != ESP_OK) { + LOG_ERR("HTTP", "open failed: %s", esp_err_to_name(err)); + esp_http_client_cleanup(client); + return HttpDownloader::HTTP_ERROR; + } + int64_t contentLength = esp_http_client_fetch_headers(client); + int status = esp_http_client_get_status_code(client); + for (int hop = 0; isRedirect(status) && hop < 5; ++hop) { + if (esp_http_client_set_redirection(client) != ESP_OK) break; + err = esp_http_client_open(client, 0); + if (err != ESP_OK) { + LOG_ERR("HTTP", "redirect open failed: %s", esp_err_to_name(err)); + esp_http_client_cleanup(client); + return HttpDownloader::HTTP_ERROR; + } + contentLength = esp_http_client_fetch_headers(client); + status = esp_http_client_get_status_code(client); + } - private: - FsFile& file_; - size_t total_; - size_t downloaded_ = 0; - bool writeOk_ = true; - HttpDownloader::ProgressCallback progress_; - bool* cancelFlag_; -}; + if (status != 200) { + LOG_ERR("HTTP", "unexpected status: %d", status); + esp_http_client_cleanup(client); + return HttpDownloader::HTTP_ERROR; + } + + // fetch_headers returns 0 for a chunked response (no Content-Length); leave + // total at 0 so progress stays silent and the size check is skipped. + sink.total = contentLength > 0 ? static_cast(contentLength) : 0; + + auto buf = makeUniqueNoThrow(READ_CHUNK); + if (!buf) { + LOG_ERR("HTTP", "OOM: %u byte read buffer", (unsigned)READ_CHUNK); + esp_http_client_cleanup(client); + return HttpDownloader::HTTP_ERROR; + } + + while (true) { + if (sink.cancelFlag && *sink.cancelFlag) { + esp_http_client_cleanup(client); + return HttpDownloader::ABORTED; + } + const int read = esp_http_client_read(client, buf.get(), READ_CHUNK); + if (read < 0) { + LOG_ERR("HTTP", "read error after %zu bytes", sink.downloaded); + esp_http_client_cleanup(client); + return HttpDownloader::HTTP_ERROR; + } + if (read == 0) break; // all data received + if (!sink.write(reinterpret_cast(buf.get()), read)) { + esp_http_client_cleanup(client); + return HttpDownloader::FILE_ERROR; + } + sink.downloaded += read; + if (sink.progress && sink.total > 0) sink.progress(sink.downloaded, sink.total); + } + + const bool complete = esp_http_client_is_complete_data_received(client); + esp_http_client_cleanup(client); + if (!complete) { + LOG_ERR("HTTP", "incomplete: got %zu of %zu bytes", sink.downloaded, sink.total); + return HttpDownloader::HTTP_ERROR; + } + return HttpDownloader::OK; +} } // namespace bool HttpDownloader::fetchUrl(const std::string& url, Stream& outContent, const std::string& username, const std::string& password) { - std::unique_ptr client; - if (UrlUtils::isHttpsUrl(url)) { - auto* secureClient = new NetworkClientSecure(); - secureClient->setInsecure(); - client.reset(secureClient); - } else { - client.reset(new NetworkClient()); - } - HTTPClient http; - LOG_DBG("HTTP", "Fetching: %s", url.c_str()); - - http.begin(*client, url.c_str()); - http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); - http.addHeader("User-Agent", "CrossPoint-ESP32-" CROSSPOINT_VERSION); - - if (!username.empty() && !password.empty()) { - std::string credentials = username + ":" + password; - String encoded = base64::encode(credentials.c_str()); - http.addHeader("Authorization", "Basic " + encoded); - } - - const int httpCode = http.GET(); - if (httpCode != HTTP_CODE_OK) { - LOG_ERR("HTTP", "Fetch failed: %d", httpCode); - http.end(); - return false; - } - - http.writeToStream(&outContent); - - http.end(); - - LOG_DBG("HTTP", "Fetch success"); - return true; + Sink sink; + sink.write = [&outContent](const uint8_t* data, size_t len) { return outContent.write(data, len) == len; }; + return runGet(url, username, password, sink) == OK; } bool HttpDownloader::fetchUrl(const std::string& url, std::string& outContent, const std::string& username, const std::string& password) { - StreamString stream; - if (!fetchUrl(url, stream, username, password)) { - return false; - } - outContent = stream.c_str(); - return true; + LOG_DBG("HTTP", "Fetching: %s", url.c_str()); + outContent.clear(); // start clean; the sink appends, so don't carry prior content + Sink sink; + sink.write = [&outContent](const uint8_t* data, size_t len) { + outContent.append(reinterpret_cast(data), len); + return true; + }; + return runGet(url, username, password, sink) == OK; } HttpDownloader::DownloadError HttpDownloader::downloadToFile(const std::string& url, const std::string& destPath, ProgressCallback progress, bool* cancelFlag, const std::string& username, const std::string& password) { - std::unique_ptr client; - if (UrlUtils::isHttpsUrl(url)) { - auto* secureClient = new NetworkClientSecure(); - secureClient->setInsecure(); - client.reset(secureClient); - } else { - client.reset(new NetworkClient()); - } - HTTPClient http; + LOG_DBG("HTTP", "Downloading: %s -> %s", url.c_str(), destPath.c_str()); - LOG_DBG("HTTP", "Downloading: %s", url.c_str()); - LOG_DBG("HTTP", "Destination: %s", destPath.c_str()); - - http.begin(*client, url.c_str()); - http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); - http.addHeader("User-Agent", "CrossPoint-ESP32-" CROSSPOINT_VERSION); - - if (!username.empty() && !password.empty()) { - std::string credentials = username + ":" + password; - String encoded = base64::encode(credentials.c_str()); - http.addHeader("Authorization", "Basic " + encoded); - } - - const int httpCode = http.GET(); - if (httpCode != HTTP_CODE_OK) { - LOG_ERR("HTTP", "Download failed: %d", httpCode); - http.end(); - return HTTP_ERROR; - } - - const int64_t reportedLength = http.getSize(); - const size_t contentLength = reportedLength > 0 ? static_cast(reportedLength) : 0; - if (contentLength > 0) { - LOG_DBG("HTTP", "Content-Length: %zu", contentLength); - } else { - LOG_DBG("HTTP", "Content-Length: unknown"); - } - - // Remove existing file if present if (Storage.exists(destPath.c_str())) { Storage.remove(destPath.c_str()); } - - // Open file for writing FsFile file; if (!Storage.openFileForWrite("HTTP", destPath.c_str(), file)) { LOG_ERR("HTTP", "Failed to open file for writing"); - http.end(); return FILE_ERROR; } - // Let HTTPClient handle chunked decoding and stream body bytes into the file. - FileWriteStream fileStream(file, contentLength, progress, cancelFlag); - const int writeResult = http.writeToStream(&fileStream); + Sink sink; + sink.progress = std::move(progress); + sink.cancelFlag = cancelFlag; + sink.write = [&file](const uint8_t* data, size_t len) { return file.write(data, len) == len; }; + const DownloadError result = runGet(url, username, password, sink); + // Close before any remove() on the same path; DESTRUCTOR_CLOSES_FILE would + // otherwise close only after the remove. file.close(); - http.end(); - if (cancelFlag && *cancelFlag) { + if (result != OK) { Storage.remove(destPath.c_str()); - return ABORTED; + return result; } - - if (writeResult < 0) { - LOG_ERR("HTTP", "writeToStream error: %d", writeResult); + if (sink.downloaded == 0) { + LOG_ERR("HTTP", "no data received"); Storage.remove(destPath.c_str()); return HTTP_ERROR; } - - const size_t downloaded = fileStream.downloaded(); - LOG_DBG("HTTP", "Downloaded %zu bytes", downloaded); - - // Guard against partial writes even if HTTPClient completes. - if (!fileStream.ok()) { - LOG_ERR("HTTP", "Write failed during download"); - Storage.remove(destPath.c_str()); - return FILE_ERROR; - } - - if (contentLength == 0 && downloaded == 0) { - LOG_ERR("HTTP", "Download failed: no data received"); - Storage.remove(destPath.c_str()); - return HTTP_ERROR; - } - - // Verify download size if known - if (contentLength > 0 && downloaded != contentLength) { - LOG_ERR("HTTP", "Size mismatch: got %zu, expected %zu", downloaded, contentLength); - Storage.remove(destPath.c_str()); - return HTTP_ERROR; - } - + LOG_DBG("HTTP", "Downloaded %zu bytes", sink.downloaded); return OK; } diff --git a/src/network/HttpDownloader.h b/src/network/HttpDownloader.h index 5913c895..1c0a243f 100644 --- a/src/network/HttpDownloader.h +++ b/src/network/HttpDownloader.h @@ -5,8 +5,9 @@ #include /** - * HTTP client utility for fetching content and downloading files. - * Wraps NetworkClientSecure and HTTPClient for HTTPS requests. + * HTTP client utility for fetching content and downloading files. Built on + * esp_http_client: https is verified against the CA bundle, plain http is + * used for local servers (transport is chosen from the URL scheme). */ class HttpDownloader { public: diff --git a/src/util/UrlUtils.cpp b/src/util/UrlUtils.cpp index 34e24914..790fe6a3 100644 --- a/src/util/UrlUtils.cpp +++ b/src/util/UrlUtils.cpp @@ -2,8 +2,6 @@ namespace UrlUtils { -bool isHttpsUrl(const std::string& url) { return url.rfind("https://", 0) == 0; } - std::string ensureProtocol(const std::string& url) { if (url.find("://") == std::string::npos) { return "http://" + url; diff --git a/src/util/UrlUtils.h b/src/util/UrlUtils.h index 6428161b..d88ca13c 100644 --- a/src/util/UrlUtils.h +++ b/src/util/UrlUtils.h @@ -3,11 +3,6 @@ namespace UrlUtils { -/** - * Check if URL uses HTTPS protocol - */ -bool isHttpsUrl(const std::string& url); - /** * Prepend http:// if no protocol specified (server will redirect to https if needed) */