mirror of
https://github.com/daveallie/crosspoint-reader.git
synced 2026-02-04 06:37:38 +03:00
fix: OPDS browser OOM (#403)
## Summary - Rewrite OpdsParser to stream parsing instead of full content - Fix OOM due to big http xml response Closes #385 --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**NO**_
This commit is contained in:
parent
e3d6e32609
commit
47ef92e8fd
@ -4,6 +4,14 @@
|
||||
|
||||
#include <cstring>
|
||||
|
||||
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,13 +22,11 @@ OpdsParser::~OpdsParser() {
|
||||
}
|
||||
}
|
||||
|
||||
bool OpdsParser::parse(const char* xmlData, const size_t length) {
|
||||
clear();
|
||||
size_t OpdsParser::write(uint8_t c) { return write(&c, 1); }
|
||||
|
||||
parser = XML_ParserCreate(nullptr);
|
||||
if (!parser) {
|
||||
Serial.printf("[%lu] [OPDS] Couldn't allocate memory for parser\n", millis());
|
||||
return false;
|
||||
size_t OpdsParser::write(const uint8_t* xmlData, const size_t length) {
|
||||
if (errorOccured) {
|
||||
return length;
|
||||
}
|
||||
|
||||
XML_SetUserData(parser, this);
|
||||
@ -28,43 +34,48 @@ bool OpdsParser::parse(const char* xmlData, const size_t length) {
|
||||
XML_SetCharacterDataHandler(parser, characterData);
|
||||
|
||||
// Parse in chunks to avoid large buffer allocations
|
||||
const char* currentPos = xmlData;
|
||||
const char* currentPos = reinterpret_cast<const char*>(xmlData);
|
||||
size_t remaining = length;
|
||||
constexpr size_t chunkSize = 1024;
|
||||
|
||||
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 length;
|
||||
}
|
||||
|
||||
const size_t toRead = remaining < chunkSize ? remaining : chunkSize;
|
||||
memcpy(buf, currentPos, toRead);
|
||||
|
||||
const bool isFinal = (remaining == toRead);
|
||||
if (XML_ParseBuffer(parser, static_cast<int>(toRead), isFinal) == XML_STATUS_ERROR) {
|
||||
if (XML_ParseBuffer(parser, static_cast<int>(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 length;
|
||||
}
|
||||
|
||||
currentPos += toRead;
|
||||
remaining -= toRead;
|
||||
}
|
||||
return length;
|
||||
}
|
||||
|
||||
// Clean up parser
|
||||
void OpdsParser::flush() {
|
||||
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() {
|
||||
entries.clear();
|
||||
currentEntry = OpdsEntry{};
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
#pragma once
|
||||
#include <Print.h>
|
||||
#include <expat.h>
|
||||
|
||||
#include <string>
|
||||
@ -42,28 +43,30 @@ using OpdsBook = OpdsEntry;
|
||||
* }
|
||||
* }
|
||||
*/
|
||||
class OpdsParser {
|
||||
class OpdsParser final : public Print {
|
||||
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);
|
||||
size_t write(uint8_t) override;
|
||||
size_t write(const uint8_t*, size_t) override;
|
||||
|
||||
void flush() override;
|
||||
|
||||
bool error() const;
|
||||
|
||||
operator bool() { return !error(); }
|
||||
|
||||
/**
|
||||
* Get the parsed entries (both navigation and book entries).
|
||||
* @return Vector of OpdsEntry entries
|
||||
*/
|
||||
const std::vector<OpdsEntry>& getEntries() const { return entries; }
|
||||
const std::vector<OpdsEntry>& getEntries() const& { return entries; }
|
||||
std::vector<OpdsEntry> getEntries() && { return std::move(entries); }
|
||||
|
||||
/**
|
||||
* Get only book entries (legacy compatibility).
|
||||
@ -96,4 +99,6 @@ class OpdsParser {
|
||||
bool inAuthor = false;
|
||||
bool inAuthorName = false;
|
||||
bool inId = false;
|
||||
|
||||
bool errorOccured = false;
|
||||
};
|
||||
|
||||
15
lib/OpdsParser/OpdsStream.cpp
Normal file
15
lib/OpdsParser/OpdsStream.cpp
Normal file
@ -0,0 +1,15 @@
|
||||
#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) { return parser.write(c); }
|
||||
|
||||
size_t OpdsParserStream::write(const uint8_t* buffer, size_t size) { return parser.write(buffer, size); }
|
||||
|
||||
OpdsParserStream::~OpdsParserStream() { parser.flush(); }
|
||||
23
lib/OpdsParser/OpdsStream.h
Normal file
23
lib/OpdsParser/OpdsStream.h
Normal file
@ -0,0 +1,23 @@
|
||||
#pragma once
|
||||
|
||||
#include <Stream.h>
|
||||
|
||||
#include "OpdsParser.h"
|
||||
|
||||
class OpdsParserStream : public Stream {
|
||||
public:
|
||||
explicit 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;
|
||||
};
|
||||
@ -3,6 +3,7 @@
|
||||
#include <Epub.h>
|
||||
#include <GfxRenderer.h>
|
||||
#include <HardwareSerial.h>
|
||||
#include <OpdsStream.h>
|
||||
#include <WiFi.h>
|
||||
|
||||
#include "CrossPointSettings.h"
|
||||
@ -265,23 +266,27 @@ 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)) {
|
||||
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) {
|
||||
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()) {
|
||||
|
||||
@ -2,6 +2,7 @@
|
||||
|
||||
#include <HTTPClient.h>
|
||||
#include <HardwareSerial.h>
|
||||
#include <StreamString.h>
|
||||
#include <WiFiClient.h>
|
||||
#include <WiFiClientSecure.h>
|
||||
|
||||
@ -9,7 +10,7 @@
|
||||
|
||||
#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<WiFiClient> client;
|
||||
if (UrlUtils::isHttpsUrl(url)) {
|
||||
@ -34,10 +35,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;
|
||||
}
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user