Merge pull request #99 from jgoguen/update-url-utils

feat(UrlUtils): Resolve more URL formats

src/util/UrlUtils.cpp

@@ -1,7 +1,179 @@
 #include "UrlUtils.h"
 
+#include <algorithm>
+#include <string_view>
+
 namespace UrlUtils {
 
+namespace {
+void popLastSegment(std::string& path) {
+  if (path.empty()) {
+    return;
+  }
+
+  size_t end = path.size();
+  if (end > 1 && path.back() == '/') {
+    end--;
+  }
+
+  while (end > 0 && path[end - 1] != '/') {
+    end--;
+  }
+
+  path.resize(end);
+}
+
+std::string normalizePath(const std::string_view path) {
+  if (path.empty()) {
+    return "/";
+  }
+
+  std::string normalized;
+  normalized.reserve(path.size());
+
+  size_t inputPos = 0;
+  while (inputPos < path.size()) {
+    if (path.compare(inputPos, 3, "../") == 0) {
+      inputPos += 3;
+      continue;
+    }
+    if (path.compare(inputPos, 2, "./") == 0) {
+      inputPos += 2;
+      continue;
+    }
+    if (path.compare(inputPos, 3, "/./") == 0) {
+      inputPos += 2;
+      continue;
+    }
+    if (path.compare(inputPos, 2, "/.") == 0 && inputPos + 2 == path.size()) {
+      if (normalized.empty()) {
+        normalized.push_back('/');
+      } else if (normalized.back() != '/') {
+        normalized.push_back('/');
+      }
+      break;
+    }
+    if (path.compare(inputPos, 4, "/../") == 0) {
+      inputPos += 4;
+      popLastSegment(normalized);
+      if (normalized.empty() || normalized.back() != '/') {
+        normalized.push_back('/');
+      }
+      continue;
+    }
+    if (path.compare(inputPos, 3, "/..") == 0 && inputPos + 3 == path.size()) {
+      popLastSegment(normalized);
+      if (normalized.empty() || normalized.back() != '/') {
+        normalized.push_back('/');
+      }
+      break;
+    }
+    if (path.compare(inputPos, 1, ".") == 0 && inputPos + 1 == path.size()) {
+      break;
+    }
+    if (path.compare(inputPos, 2, "..") == 0 && inputPos + 2 == path.size()) {
+      break;
+    }
+
+    const size_t nextSlash = path[inputPos] == '/' ? path.find('/', inputPos + 1) : path.find('/', inputPos);
+    if (nextSlash == std::string::npos) {
+      normalized.append(path, inputPos, path.size() - inputPos);
+      break;
+    }
+
+    normalized.append(path, inputPos, nextSlash - inputPos);
+    inputPos = nextSlash;
+  }
+
+  if (path.front() == '/' && normalized.empty()) {
+    return "/";
+  }
+
+  if (path.front() == '/' && normalized.size() == 1) {
+    return normalized;
+  }
+
+  if (path.front() != '/' && normalized.empty()) {
+    return ".";
+  }
+
+  return normalized;
+}
+
+std::string_view stripQueryAndFragment(const std::string_view urlPath) {
+  const size_t suffixStart = urlPath.find_first_of("?#");
+  return suffixStart == std::string::npos ? urlPath : urlPath.substr(0, suffixStart);
+}
+
+std::string_view stripFragment(const std::string_view urlPath) {
+  const size_t fragmentStart = urlPath.find('#');
+  return fragmentStart == std::string::npos ? urlPath : urlPath.substr(0, fragmentStart);
+}
+
+std::string_view extractScheme(const std::string_view url) {
+  const size_t protocolEnd = url.find("://");
+  return protocolEnd == std::string::npos ? "" : url.substr(0, protocolEnd);
+}
+
+struct UrlSuffixParts {
+  std::string_view path;
+  std::string_view query;
+  std::string_view fragment;
+};
+
+UrlSuffixParts splitReference(const std::string_view ref) {
+  UrlSuffixParts parts;
+  const size_t fragmentStart = ref.find('#');
+  const size_t queryStart = ref.find('?');
+  const size_t pathEnd = std::min(queryStart == std::string::npos ? ref.size() : queryStart,
+                                  fragmentStart == std::string::npos ? ref.size() : fragmentStart);
+
+  parts.path = ref.substr(0, pathEnd);
+  if (queryStart != std::string::npos && (fragmentStart == std::string::npos || queryStart < fragmentStart)) {
+    const size_t queryEnd = fragmentStart == std::string::npos ? ref.size() : fragmentStart;
+    parts.query = ref.substr(queryStart, queryEnd - queryStart);
+  }
+  if (fragmentStart != std::string::npos) {
+    parts.fragment = ref.substr(fragmentStart);
+  }
+
+  return parts;
+}
+
+std::string_view extractHostView(const std::string_view url) {
+  const size_t protocolEnd = url.find("://");
+  if (protocolEnd == std::string::npos) {
+    const size_t authorityEnd = url.find_first_of("/?#");
+    return authorityEnd == std::string::npos ? url : url.substr(0, authorityEnd);
+  }
+
+  const size_t authorityStart = protocolEnd + 3;
+  const size_t authorityEnd = url.find_first_of("/?#", authorityStart);
+  return authorityEnd == std::string::npos ? url : url.substr(0, authorityEnd);
+}
+
+void appendView(std::string& out, const std::string_view view) { out.append(view.data(), view.size()); }
+
+std::string buildResolvedUrl(const std::string_view host, const std::string_view path, const std::string_view query,
+                             const std::string_view fragment) {
+  std::string resolved;
+  resolved.reserve(host.size() + path.size() + query.size() + fragment.size());
+  appendView(resolved, host);
+  appendView(resolved, path);
+  appendView(resolved, query);
+  appendView(resolved, fragment);
+  return resolved;
+}
+
+std::string normalizeJoinedPath(const std::string_view baseDir, const std::string_view relativePath) {
+  std::string combined;
+  combined.reserve(baseDir.size() + relativePath.size());
+  appendView(combined, baseDir);
+  appendView(combined, relativePath);
+  return normalizePath(combined);
+}
+}  // namespace
+
 bool isHttpsUrl(const std::string& url) { return url.rfind("https://", 0) == 0; }
 
 std::string ensureProtocol(const std::string& url) {
@@ -11,17 +183,60 @@ std::string ensureProtocol(const std::string& url) {
   return url;
 }
 
-std::string extractHost(const std::string& url) {
+std::string extractHost(const std::string& url) { return std::string(extractHostView(url)); }
-  const size_t protocolEnd = url.find("://");
+
-  if (protocolEnd == std::string::npos) {
+std::string extractHostname(const std::string& url) {
-    // No protocol, find first slash
+  const std::string_view urlView(url);
-    const size_t firstSlash = url.find('/');
+  const size_t protocolEnd = urlView.find("://");
-    return firstSlash == std::string::npos ? url : url.substr(0, firstSlash);
+  size_t hostStart = protocolEnd == std::string::npos ? 0 : protocolEnd + 3;
+  if (hostStart >= urlView.size()) {
+    return "";
   }
-  // Find the first slash after the protocol
+
-  const size_t hostStart = protocolEnd + 3;
+  const size_t authorityEnd = urlView.find_first_of("/?#", hostStart);
-  const size_t pathStart = url.find('/', hostStart);
+  const size_t authorityLimit = authorityEnd == std::string::npos ? urlView.size() : authorityEnd;
-  return pathStart == std::string::npos ? url : url.substr(0, pathStart);
+  if (hostStart >= authorityLimit) {
+    return "";
+  }
+
+  if (protocolEnd == std::string::npos && urlView[hostStart] != '[') {
+    const size_t colonPos = urlView.find(':', hostStart);
+    if (colonPos != std::string::npos && colonPos < authorityLimit) {
+      bool digitsOnlyPort = colonPos + 1 < authorityLimit;
+      for (size_t i = colonPos + 1; i < authorityLimit; i++) {
+        if (urlView[i] < '0' || urlView[i] > '9') {
+          digitsOnlyPort = false;
+          break;
+        }
+      }
+      if (!digitsOnlyPort) {
+        return "";
+      }
+    }
+  }
+
+  const size_t atPos = urlView.find('@', hostStart);
+  if (atPos != std::string::npos && atPos < authorityLimit) {
+    hostStart = atPos + 1;
+  }
+  if (hostStart >= authorityLimit) {
+    return "";
+  }
+
+  if (urlView[hostStart] == '[') {
+    const size_t closingBracket = urlView.find(']', hostStart + 1);
+    if (closingBracket == std::string::npos || closingBracket >= authorityLimit || closingBracket == hostStart + 1) {
+      return "";
+    }
+    if (closingBracket + 1 < authorityLimit && urlView[closingBracket + 1] != ':') {
+      return "";
+    }
+    return std::string(urlView.substr(hostStart + 1, closingBracket - hostStart - 1));
+  }
+
+  const size_t portPos = urlView.find(':', hostStart);
+  const size_t hostEnd = (portPos == std::string::npos || portPos >= authorityLimit) ? authorityLimit : portPos;
+  return hostEnd > hostStart ? std::string(urlView.substr(hostStart, hostEnd - hostStart)) : "";
 }
 
 std::string buildUrl(const std::string& serverUrl, const std::string& path) {
@@ -30,18 +245,56 @@ std::string buildUrl(const std::string& serverUrl, const std::string& path) {
     return path;
   }
   const std::string urlWithProtocol = ensureProtocol(serverUrl);
+  const std::string_view strippedUrl = stripQueryAndFragment(urlWithProtocol);
   if (path.empty()) {
     return urlWithProtocol;
   }
+  const UrlSuffixParts refParts = splitReference(path);
+  if (path.rfind("//", 0) == 0) {
+    std::string resolved;
+    const std::string_view scheme = extractScheme(urlWithProtocol);
+    resolved.reserve(scheme.size() + 1 + path.size());
+    appendView(resolved, scheme);
+    resolved.push_back(':');
+    resolved += path;
+    return resolved;
+  }
   if (path[0] == '/') {
     // Absolute path - use just the host
-    return extractHost(urlWithProtocol) + path;
+    const std::string_view host = extractHostView(strippedUrl);
+    const std::string normalizedPath = normalizePath(refParts.path);
+    return buildResolvedUrl(host, normalizedPath, refParts.query, refParts.fragment);
   }
-  // Relative path - append to server URL
+  if (path[0] == '?') {
-  if (urlWithProtocol.back() == '/') {
+    std::string resolved;
-    return urlWithProtocol + path;
+    resolved.reserve(strippedUrl.size() + path.size());
+    appendView(resolved, strippedUrl);
+    resolved += path;
+    return resolved;
   }
-  return urlWithProtocol + "/" + path;
+  if (path[0] == '#') {
+    const std::string_view baseWithoutFragment = stripFragment(urlWithProtocol);
+    std::string resolved;
+    resolved.reserve(baseWithoutFragment.size() + path.size());
+    appendView(resolved, baseWithoutFragment);
+    resolved += path;
+    return resolved;
+  }
+
+  const std::string_view baseHost = extractHostView(strippedUrl);
+  std::string_view basePath = strippedUrl.substr(baseHost.size());
+  if (basePath.empty()) {
+    basePath = std::string_view("/");
+  }
+
+  const size_t lastSlash = basePath.find_last_of('/');
+  const std::string_view baseDir =
+      basePath.back() == '/'
+          ? basePath
+          : (lastSlash == std::string::npos ? std::string_view("/") : basePath.substr(0, lastSlash + 1));
+  const std::string resolvedPath =
+      refParts.path.empty() ? std::string(baseDir) : normalizeJoinedPath(baseDir, refParts.path);
+  return buildResolvedUrl(baseHost, resolvedPath, refParts.query, refParts.fragment);
 }
 
 }  // namespace UrlUtils

src/util/UrlUtils.h

@@ -18,6 +18,13 @@ std::string ensureProtocol(const std::string& url);
  */
 std::string extractHost(const std::string& url);
 
+/**
+ * Extract hostname only from a URL (e.g., "example.com" from
+ * "http://example.com:8080/path"). Returns an empty string if no hostname can
+ * be determined.
+ */
+std::string extractHostname(const std::string& url);
+
 /**
  * Build full URL from server URL and path.
  * If path starts with /, it's an absolute path from the host root.

test/run_url_utils_test.sh

@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+BUILD_DIR="$ROOT_DIR/build/url_utils"
+BINARY="$BUILD_DIR/UrlUtilsTest"
+PLATFORMIO_DIR="${PLATFORMIO_CORE_DIR:-$HOME/.platformio}"
+ARDUINO_FRAMEWORK_DIR="$PLATFORMIO_DIR/packages/framework-arduinoespressif32"
+
+mkdir -p "$BUILD_DIR"
+
+SOURCES=(
+  "$ROOT_DIR/test/url_utils/UrlUtilsTest.cpp"
+  "$ROOT_DIR/src/util/UrlUtils.cpp"
+)
+
+CXXFLAGS=(
+  -std=c++20
+  -O2
+  -Wall
+  -Wextra
+  -pedantic
+  -fno-exceptions
+  -DARDUINO_USB_MODE=1
+  -DARDUINO_USB_CDC_ON_BOOT=1
+  -DDESTRUCTOR_CLOSES_FILE=1
+  -I"$ROOT_DIR/test/shims"
+  -I"$ROOT_DIR"
+  -I"$ROOT_DIR/src"
+  -I"$ARDUINO_FRAMEWORK_DIR/cores/esp32"
+  -I"$ARDUINO_FRAMEWORK_DIR/variants/esp32c3"
+)
+
+c++ "${CXXFLAGS[@]}" "${SOURCES[@]}" -o "$BINARY"
+
+"$BINARY" "$@"

test/url_utils/UrlUtilsTest.cpp

@@ -0,0 +1,149 @@
+#include <cstdio>
+#include <string>
+
+#include "../../src/util/UrlUtils.h"
+
+static int testsPassed = 0;
+static int testsFailed = 0;
+
+#define ASSERT_EQ(a, b)                                                         \
+  do {                                                                          \
+    if ((a) != (b)) {                                                           \
+      fprintf(stderr, "  FAIL: %s:%d: %s != %s\n", __FILE__, __LINE__, #a, #b); \
+      testsFailed++;                                                            \
+      return;                                                                   \
+    }                                                                           \
+  } while (0)
+
+#define PASS() testsPassed++
+
+void testAbsoluteUrlRemainsUnchanged() {
+  printf("testAbsoluteUrlRemainsUnchanged...\n");
+  ASSERT_EQ(
+      UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "https://example.com/books/download/test.epub"),
+      "https://example.com/books/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "//cdn.example.com/books/test.epub"),
+            "https://cdn.example.com/books/test.epub");
+  PASS();
+}
+
+void testRootRelativeUrlUsesHostRoot() {
+  printf("testRootRelativeUrlUsesHostRoot...\n");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "/books/download/test.epub"),
+            "https://catalog.example.com/books/download/test.epub");
+  PASS();
+}
+
+void testRelativeUrlUsesFeedDirectory() {
+  printf("testRelativeUrlUsesFeedDirectory...\n");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "download/test.epub"),
+            "https://catalog.example.com/opds/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub", "download/test.epub"),
+            "https://catalog.example.com/opds/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub/", "download/test.epub"),
+            "https://catalog.example.com/opds/sub/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "images//cover.jpg"),
+            "https://catalog.example.com/opds/images//cover.jpg");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "subdir/"),
+            "https://catalog.example.com/opds/subdir/");
+  PASS();
+}
+
+void testParentRelativeUrlResolvesCorrectly() {
+  printf("testParentRelativeUrlResolvesCorrectly...\n");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub/feed.xml", "../download/test.epub"),
+            "https://catalog.example.com/opds/download/test.epub");
+  PASS();
+}
+
+void testUrlsWithQueryAndFragmentResolveCorrectly() {
+  printf("testUrlsWithQueryAndFragmentResolveCorrectly...\n");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub?auth=1", "download/test.epub"),
+            "https://catalog.example.com/opds/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub/#section", "download/test.epub?format=epub#start"),
+            "https://catalog.example.com/opds/sub/download/test.epub?format=epub#start");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml?auth=1#top", "download/test.epub"),
+            "https://catalog.example.com/opds/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "/books?id=/../cover"),
+            "https://catalog.example.com/books?id=/../cover");
+  ASSERT_EQ(
+      UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "download/test.epub?next=/../cover#frag/../x"),
+      "https://catalog.example.com/opds/download/test.epub?next=/../cover#frag/../x");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml", "download/test.epub#frag?x=1"),
+            "https://catalog.example.com/opds/download/test.epub#frag?x=1");
+  PASS();
+}
+
+void testRootRelativeUrlsWithQueryAndFragmentResolveCorrectly() {
+  printf("testRootRelativeUrlsWithQueryAndFragmentResolveCorrectly...\n");
+  ASSERT_EQ(
+      UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml?auth=1", "/books/download/test.epub?format=epub"),
+      "https://catalog.example.com/books/download/test.epub?format=epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub/#section", "/books/download/test.epub#start"),
+            "https://catalog.example.com/books/download/test.epub#start");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/sub?auth=1#top",
+                               "/books/download/test.epub?format=epub#start"),
+            "https://catalog.example.com/books/download/test.epub?format=epub#start");
+  PASS();
+}
+
+void testHostOnlyBaseUrlsWithQueryAndFragmentResolveCorrectly() {
+  printf("testHostOnlyBaseUrlsWithQueryAndFragmentResolveCorrectly...\n");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com?auth=1", "download/test.epub"),
+            "https://catalog.example.com/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com#top", "/books/download/test.epub"),
+            "https://catalog.example.com/books/download/test.epub");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com?auth=1", "?page=2"), "https://catalog.example.com?page=2");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com#top", "#latest"), "https://catalog.example.com#latest");
+  PASS();
+}
+
+void testQueryAndFragmentReferencesAreRfcCompliant() {
+  printf("testQueryAndFragmentReferencesAreRfcCompliant...\n");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml?auth=1", "#top"),
+            "https://catalog.example.com/opds/feed.xml?auth=1#top");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com/opds/feed.xml?auth=1#old", "?page=2"),
+            "https://catalog.example.com/opds/feed.xml?page=2");
+  ASSERT_EQ(UrlUtils::buildUrl("https://catalog.example.com?auth=1#old", "#latest"),
+            "https://catalog.example.com?auth=1#latest");
+  PASS();
+}
+
+void testExtractHostnameHandlesPortsAndIpv6() {
+  printf("testExtractHostnameHandlesPortsAndIpv6...\n");
+  ASSERT_EQ(UrlUtils::extractHostname("https://catalog.example.com:8080/path"), "catalog.example.com");
+  ASSERT_EQ(UrlUtils::extractHostname("catalog.example.com:8080/opds"), "catalog.example.com");
+  ASSERT_EQ(UrlUtils::extractHostname("https://user:pass@[2001:db8::1]:8080/path?x=1#frag"), "2001:db8::1");
+  ASSERT_EQ(UrlUtils::extractHostname("http://[fe80::1234]#top"), "fe80::1234");
+  PASS();
+}
+
+void testExtractHostnameRejectsMalformedInputs() {
+  printf("testExtractHostnameRejectsMalformedInputs...\n");
+  ASSERT_EQ(UrlUtils::extractHostname("https:///path"), "");
+  ASSERT_EQ(UrlUtils::extractHostname("http://[]"), "");
+  ASSERT_EQ(UrlUtils::extractHostname("http://@/x"), "");
+  ASSERT_EQ(UrlUtils::extractHostname("http://[::1"), "");
+  ASSERT_EQ(UrlUtils::extractHostname("http://[::1]x"), "");
+  ASSERT_EQ(UrlUtils::extractHostname("mailto:user@example.com"), "");
+  ASSERT_EQ(UrlUtils::extractHostname("urn:isbn:1234567890"), "");
+  PASS();
+}
+
+int main() {
+  printf("=== OPDS URL Utils Tests ===\n\n");
+
+  testAbsoluteUrlRemainsUnchanged();
+  testRootRelativeUrlUsesHostRoot();
+  testRelativeUrlUsesFeedDirectory();
+  testParentRelativeUrlResolvesCorrectly();
+  testUrlsWithQueryAndFragmentResolveCorrectly();
+  testRootRelativeUrlsWithQueryAndFragmentResolveCorrectly();
+  testHostOnlyBaseUrlsWithQueryAndFragmentResolveCorrectly();
+  testQueryAndFragmentReferencesAreRfcCompliant();
+  testExtractHostnameHandlesPortsAndIpv6();
+  testExtractHostnameRejectsMalformedInputs();
+
+  printf("\n=== Results: %d passed, %d failed ===\n", testsPassed, testsFailed);
+  return testsFailed > 0 ? 1 : 0;
+}