From d34aea63785d3efdd77b30b7ecd164a439bf3912 Mon Sep 17 00:00:00 2001 From: Konstantin Vukolov Date: Sat, 17 Jan 2026 01:32:59 +0300 Subject: [PATCH] Fix OPDS browser OOM --- lib/OpdsParser/OpdsParser.cpp | 44 ++++++++++++------- lib/OpdsParser/OpdsParser.h | 17 ++++--- lib/OpdsParser/OpdsStream.cpp | 30 +++++++++++++ lib/OpdsParser/OpdsStream.h | 23 ++++++++++ .../browser/OpdsBookBrowserActivity.cpp | 25 +++++++---- src/network/HttpDownloader.cpp | 18 ++++++-- src/network/HttpDownloader.h | 2 + 7 files changed, 121 insertions(+), 38 deletions(-) create mode 100644 lib/OpdsParser/OpdsStream.cpp create mode 100644 lib/OpdsParser/OpdsStream.h diff --git a/lib/OpdsParser/OpdsParser.cpp b/lib/OpdsParser/OpdsParser.cpp index da4042f..77a1c05 100644 --- a/lib/OpdsParser/OpdsParser.cpp +++ b/lib/OpdsParser/OpdsParser.cpp @@ -4,6 +4,14 @@ #include +OpdsParser::OpdsParser() { + parser = XML_ParserCreate(nullptr); + if (!parser) { + errorOccured = true; + Serial.printf("[%lu] [OPDS] Couldn't allocate memory for parser\n", millis()); + } +} + OpdsParser::~OpdsParser() { if (parser) { XML_StopParser(parser, XML_FALSE); @@ -14,14 +22,10 @@ OpdsParser::~OpdsParser() { } } -bool OpdsParser::parse(const char* xmlData, const size_t length) { - clear(); - - parser = XML_ParserCreate(nullptr); - if (!parser) { - Serial.printf("[%lu] [OPDS] Couldn't allocate memory for parser\n", millis()); - return false; - } +void OpdsParser::push(const char* xmlData, const size_t length) { + if (errorOccured) { + return; + } XML_SetUserData(parser, this); XML_SetElementHandler(parser, startElement, endElement); @@ -35,34 +39,40 @@ bool OpdsParser::parse(const char* xmlData, const size_t length) { while (remaining > 0) { void* const buf = XML_GetBuffer(parser, chunkSize); if (!buf) { + errorOccured = true; Serial.printf("[%lu] [OPDS] Couldn't allocate memory for buffer\n", millis()); XML_ParserFree(parser); parser = nullptr; - return false; + return; } const size_t toRead = remaining < chunkSize ? remaining : chunkSize; memcpy(buf, currentPos, toRead); - const bool isFinal = (remaining == toRead); - if (XML_ParseBuffer(parser, static_cast(toRead), isFinal) == XML_STATUS_ERROR) { + if (XML_ParseBuffer(parser, static_cast(toRead), 0) == XML_STATUS_ERROR) { + errorOccured = true; Serial.printf("[%lu] [OPDS] Parse error at line %lu: %s\n", millis(), XML_GetCurrentLineNumber(parser), XML_ErrorString(XML_GetErrorCode(parser))); XML_ParserFree(parser); parser = nullptr; - return false; + return; } currentPos += toRead; remaining -= toRead; } +} - // Clean up parser - XML_ParserFree(parser); - parser = nullptr; +void OpdsParser::finish() { + if (XML_Parse(parser, nullptr, 0, XML_TRUE) != XML_STATUS_OK) { + errorOccured = true; + XML_ParserFree(parser); + parser = nullptr; + } +} - Serial.printf("[%lu] [OPDS] Parsed %zu entries\n", millis(), entries.size()); - return true; +bool OpdsParser::error() const { + return errorOccured; } void OpdsParser::clear() { diff --git a/lib/OpdsParser/OpdsParser.h b/lib/OpdsParser/OpdsParser.h index acb4b69..93d310a 100644 --- a/lib/OpdsParser/OpdsParser.h +++ b/lib/OpdsParser/OpdsParser.h @@ -44,26 +44,23 @@ using OpdsBook = OpdsEntry; */ class OpdsParser { public: - OpdsParser() = default; + OpdsParser(); ~OpdsParser(); // Disable copy OpdsParser(const OpdsParser&) = delete; OpdsParser& operator=(const OpdsParser&) = delete; - /** - * Parse an OPDS XML feed. - * @param xmlData Pointer to the XML data - * @param length Length of the XML data - * @return true if parsing succeeded, false on error - */ - bool parse(const char* xmlData, size_t length); + void push(const char* xmlData, size_t length); + void finish(); + bool error() const; /** * Get the parsed entries (both navigation and book entries). * @return Vector of OpdsEntry entries */ - const std::vector& getEntries() const { return entries; } + const std::vector& getEntries() const & { return entries; } + std::vector getEntries() && { return std::move(entries); } /** * Get only book entries (legacy compatibility). @@ -96,4 +93,6 @@ class OpdsParser { bool inAuthor = false; bool inAuthorName = false; bool inId = false; + + bool errorOccured = false; }; diff --git a/lib/OpdsParser/OpdsStream.cpp b/lib/OpdsParser/OpdsStream.cpp new file mode 100644 index 0000000..d23405f --- /dev/null +++ b/lib/OpdsParser/OpdsStream.cpp @@ -0,0 +1,30 @@ +#include "OpdsStream.h" + +OpdsParserStream::OpdsParserStream(OpdsParser& parser) : parser(parser) {} + + +int OpdsParserStream::available() { + return 0; +} + +int OpdsParserStream::peek() { + abort(); +} + +int OpdsParserStream::read() { + abort(); +} + +size_t OpdsParserStream::write(uint8_t c) { + parser.push(reinterpret_cast(&c), 1); + return 1; +} + +size_t OpdsParserStream::write(const uint8_t *buffer, size_t size) { + parser.push(reinterpret_cast(buffer), size); + return size; +} + +OpdsParserStream::~OpdsParserStream() { + parser.finish(); +} diff --git a/lib/OpdsParser/OpdsStream.h b/lib/OpdsParser/OpdsStream.h new file mode 100644 index 0000000..184ae1f --- /dev/null +++ b/lib/OpdsParser/OpdsStream.h @@ -0,0 +1,23 @@ +#pragma once + +#include "OpdsParser.h" + +#include + +class OpdsParserStream : public Stream { +public: + OpdsParserStream(OpdsParser& parser); + + // That functions are not implimented for that stream + int available() override; + int peek() override; + int read() override; + + virtual size_t write(uint8_t c) override; + virtual size_t write(const uint8_t *buffer, size_t size) override; + + ~OpdsParserStream() override; + +private: + OpdsParser& parser; +}; diff --git a/src/activities/browser/OpdsBookBrowserActivity.cpp b/src/activities/browser/OpdsBookBrowserActivity.cpp index 4e0a08d..4a33baf 100644 --- a/src/activities/browser/OpdsBookBrowserActivity.cpp +++ b/src/activities/browser/OpdsBookBrowserActivity.cpp @@ -4,6 +4,8 @@ #include #include +#include + #include "CrossPointSettings.h" #include "MappedInputManager.h" #include "ScreenComponents.h" @@ -264,23 +266,28 @@ void OpdsBookBrowserActivity::fetchFeed(const std::string& path) { std::string url = UrlUtils::buildUrl(serverUrl, path); Serial.printf("[%lu] [OPDS] Fetching: %s\n", millis(), url.c_str()); - std::string content; - if (!HttpDownloader::fetchUrl(url, content)) { - state = BrowserState::ERROR; - errorMessage = "Failed to fetch feed"; - updateRequired = true; - return; + OpdsParser parser; + + { + OpdsParserStream stream{parser}; + if (!HttpDownloader::fetchUrl(url, stream)) { + state = BrowserState::ERROR; + errorMessage = "Failed to fetch feed"; + updateRequired = true; + return; + } } - OpdsParser parser; - if (!parser.parse(content.c_str(), content.size())) { + + if (parser.error()) { state = BrowserState::ERROR; errorMessage = "Failed to parse feed"; updateRequired = true; return; } - entries = parser.getEntries(); + entries = std::move(parser).getEntries(); + Serial.printf("[%lu] [OPDS] Found %d entries\n", millis(), entries.size()); selectorIndex = 0; if (entries.empty()) { diff --git a/src/network/HttpDownloader.cpp b/src/network/HttpDownloader.cpp index c4de3a0..9382697 100644 --- a/src/network/HttpDownloader.cpp +++ b/src/network/HttpDownloader.cpp @@ -5,11 +5,13 @@ #include #include +#include + #include #include "util/UrlUtils.h" -bool HttpDownloader::fetchUrl(const std::string& url, std::string& outContent) { +bool HttpDownloader::fetchUrl(const std::string& url, Stream& outContent) { // Use WiFiClientSecure for HTTPS, regular WiFiClient for HTTP std::unique_ptr client; if (UrlUtils::isHttpsUrl(url)) { @@ -34,10 +36,20 @@ bool HttpDownloader::fetchUrl(const std::string& url, std::string& outContent) { return false; } - outContent = http.getString().c_str(); + http.writeToStream(&outContent); + http.end(); - Serial.printf("[%lu] [HTTP] Fetched %zu bytes\n", millis(), outContent.size()); + Serial.printf("[%lu] [HTTP] Fetch success\n", millis()); + return true; +} + +bool HttpDownloader::fetchUrl(const std::string& url, std::string& outContent) { + StreamString stream; + if (!fetchUrl(url, stream)) { + return false; + } + outContent = stream.c_str(); return true; } diff --git a/src/network/HttpDownloader.h b/src/network/HttpDownloader.h index e6e0f16..ac520a4 100644 --- a/src/network/HttpDownloader.h +++ b/src/network/HttpDownloader.h @@ -27,6 +27,8 @@ class HttpDownloader { */ static bool fetchUrl(const std::string& url, std::string& outContent); + static bool fetchUrl(const std::string& url, Stream& stream); + /** * Download a file to the SD card. * @param url The URL to download