Address upstream review comments

2026-04-13 11:15:20 +02:00
parent ee2d608a49
commit 5c567b0a72
4 changed files with 32 additions and 14 deletions
@@ -77,11 +77,20 @@ bool ChapterXPathIndexer::tryExtractParagraphIndexFromXPath(const std::string& x
  }

  const std::string pKey = "/p[";
-  const size_t pos = normalized.find(pKey, secondBody != std::string::npos ? secondBody : 0);
+  const size_t searchStart = secondBody != std::string::npos ? secondBody : 0;
+  const size_t pos = normalized.find(pKey, searchStart);
  if (pos == std::string::npos) {
    return false;
  }

+  // Only accept p[...] that is a direct child of the /body segment — reject
+  // paths with intermediate ancestor segments (e.g. /body/.../div[4]/p[1])
+  // which would collapse structurally different locations to the same index.
+  const size_t bodyEnd = (secondBody != std::string::npos ? secondBody : 0) + bodyKey.size();
+  if (pos != bodyEnd) {
+    return false;
+  }
+
  const size_t start = pos + pKey.size();
  size_t end = start;
  while (end < normalized.size() && std::isdigit(static_cast<unsigned char>(normalized[end]))) {
@@ -342,9 +342,9 @@ size_t countTotalTextBytes(const std::string& tmpPath) {
  XML_SetElementHandler(parser, bcStart, bcEnd);
  XML_SetCharacterDataHandler(parser, bcChar);
  XML_SetDefaultHandlerExpand(parser, bcDefault);
-  runParse(parser, tmpPath);
+  const bool ok = runParse(parser, tmpPath);
  XML_ParserFree(parser);
-  return state.totalTextBytes;
+  return ok ? state.totalTextBytes : 0;
 }

 }  // namespace ChapterXPathIndexerInternal
@@ -576,7 +576,20 @@ KOReaderSyncClient::Error KOReaderSyncClient::updateProgress(const KOReaderProgr

  if (err != ESP_OK) return NETWORK_ERROR;
  if (httpCode >= 300 && httpCode < 400) return REDIRECT_ERROR;
-  if (httpCode == 200 || httpCode == 202) return OK;
+  if (httpCode == 200 || httpCode == 202) {
+    // Guard against a reverse proxy or captive portal returning HTTP 200 + HTML
+    // instead of the real API response, consistent with authenticate()'s check.
+    if (activeBuf->data) {
+      const char* p = activeBuf->data;
+      while (*p == ' ' || *p == '\t' || *p == '\r' || *p == '\n') {
+        p++;
+      }
+      if (*p != '\0' && *p != '{') {
+        return SERVER_ERROR;
+      }
+    }
+    return OK;
+  }
  if (httpCode == 401) return AUTH_FAILED;
  return SERVER_ERROR;
 }
@@ -58,16 +58,12 @@ KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr<Epub>& epub, c
  result.percentage = epub->calculateProgress(pos.spineIndex, intraSpineProgress);

  // Generate XPath for the current position.
-  // Prefer paragraph index from the section cache LUT (exact element mapping) over
-  // byte-offset estimation (which can drift in chapters with non-uniform content density).
-  if (pos.hasParagraphIndex && pos.paragraphIndex > 0) {
-    result.xpath = "/body/DocFragment[" + std::to_string(pos.spineIndex + 1) + "]/body/p[" +
-                   std::to_string(pos.paragraphIndex) + "]";
-  } else {
-    result.xpath = ChapterXPathIndexer::findXPathForProgress(epub, pos.spineIndex, intraSpineProgress);
-    if (result.xpath.empty()) {
-      result.xpath = generateXPath(pos.spineIndex);
-    }
+  // Always use the indexer which SAX-parses the actual XHTML to find the correct
+  // element path — a naive "/body/DocFragment[N]/body/p[M]" would assume paragraphs
+  // are direct children of <body>, which breaks for wrapped chapters (e.g. div/section).
+  result.xpath = ChapterXPathIndexer::findXPathForProgress(epub, pos.spineIndex, intraSpineProgress);
+  if (result.xpath.empty()) {
+    result.xpath = generateXPath(pos.spineIndex);
  }

  // Get chapter info for logging