diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 38d9d2e8..a6946385 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,6 +45,25 @@ jobs: working-directory: app/frontend run: pnpm run check + test-uri-platforms: + name: URI paths (${{ matrix.os }}, py${{ matrix.python }}) + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + python: ["3.13", "3.14"] + steps: + - uses: actions/checkout@v4 + - uses: astral-sh/setup-uv@v9.0.0 + # --noconftest: tests/conftest.py imports the project's dependencies, + # which this job deliberately does not install. test_uri.py is stdlib-only. + - name: Test platform URI paths + env: + PYTHONPATH: haiku_rag_slim + run: > + uv run --no-project --python ${{ matrix.python }} --with pytest + pytest tests/test_uri.py -q --noconftest -o addopts= + test: needs: [lint, lint-frontend] runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cdbf5c0..1ef9df7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - A capability search that matches nothing returns `No results found.` instead of an empty string. - Repeating a search query within a question accumulates the results of both calls instead of replacing the earlier ones. +- `file://` URIs resolve to a Windows path through `url2pathname`: `file:///C:/docs/a.pdf` was read as `\C:\docs\a.pdf`, so ingestion reported `File does not exist` for every discovered file. A URI authority is kept as a UNC server/share (`file://server/share/a.pdf`) except `localhost`, which is dropped. +- A Windows drive letter is no longer read as a URI scheme, so `C:\docs\a.pdf` is accepted by `create_document_from_source`, `convert` and `resolve_adhoc_fetcher`. +- `convert` and `check_source_accessible` decode percent-escapes in `file://` URIs, so a path containing `[`, `]` or a space resolves. ## [0.76.0] - 2026-08-20 diff --git a/haiku_rag_slim/haiku/rag/client/documents.py b/haiku_rag_slim/haiku/rag/client/documents.py index 13fc65b0..d049a723 100644 --- a/haiku_rag_slim/haiku/rag/client/documents.py +++ b/haiku_rag_slim/haiku/rag/client/documents.py @@ -6,7 +6,7 @@ import mimetypes from dataclasses import dataclass, field from pathlib import Path from typing import TYPE_CHECKING -from urllib.parse import quote, unquote, urlparse +from urllib.parse import quote, urlparse from haiku.rag.client.exceptions import UnsupportedSourceError from haiku.rag.client.processing import ( @@ -20,6 +20,7 @@ from haiku.rag.store.models.chunk import Chunk from haiku.rag.store.models.document import Document from haiku.rag.store.models.document_item import DocumentItem, extract_items from haiku.rag.telemetry import logfire +from haiku.rag.uri import is_local_uri, uri_to_path if TYPE_CHECKING: from docling_core.types.doc.document import DoclingDocument @@ -654,14 +655,8 @@ async def create_document_from_source( # Directory case: recurse with the existing FS filter and produce one # document per file. Remote schemes (http/s3) never hit this branch. - if parsed_url.scheme in ("", "file"): - # file:// URIs URL-encode special characters ([, ], spaces, etc.); - # unquote to get the real filesystem path before any stat/rglob. - local_path = ( - Path(unquote(parsed_url.path)) - if parsed_url.scheme == "file" - else (Path(source) if isinstance(source, str) else source) - ) + if is_local_uri(source_str): + local_path = uri_to_path(source_str) if isinstance(source, str) else source if local_path.is_dir(): if uri is not None: raise UnsupportedSourceError( @@ -733,7 +728,7 @@ async def create_document_from_source( stored_uri = uri elif parsed_url.scheme == "file": stored_uri = source_str - elif parsed_url.scheme == "": + elif is_local_uri(source_str): stored_uri = Path(source_str).absolute().as_uri() else: stored_uri = source_str @@ -896,7 +891,7 @@ def check_source_accessible(uri: str) -> bool: try: parsed_url = urlparse(uri) if parsed_url.scheme == "file": - return Path(parsed_url.path).exists() + return uri_to_path(uri).exists() elif parsed_url.scheme in ("http", "https", "s3"): return True return False diff --git a/haiku_rag_slim/haiku/rag/client/processing.py b/haiku_rag_slim/haiku/rag/client/processing.py index 4ae7f50b..27e92d36 100644 --- a/haiku_rag_slim/haiku/rag/client/processing.py +++ b/haiku_rag_slim/haiku/rag/client/processing.py @@ -13,6 +13,7 @@ from haiku.rag.config import AppConfig from haiku.rag.converters import get_converter from haiku.rag.store.models.chunk import Chunk from haiku.rag.store.models.document_item import _picture_description_text +from haiku.rag.uri import is_local_uri, uri_to_path if TYPE_CHECKING: from docling_core.types.doc.document import DoclingDocument, PictureItem @@ -154,8 +155,10 @@ async def convert( finally: temp_path.unlink(missing_ok=True) - elif parsed.scheme == "file": - file_path = Path(parsed.path) + elif parsed.scheme and is_local_uri(source): + # A file:// URI, or a Windows path whose drive letter urlparse read as + # the scheme. A bare path with no scheme is raw text. + file_path = uri_to_path(source) if not file_path.exists(): raise UnsupportedSourceError(f"File does not exist: {file_path}") if file_path.suffix.lower() not in converter.supported_extensions: diff --git a/haiku_rag_slim/haiku/rag/sources/fs.py b/haiku_rag_slim/haiku/rag/sources/fs.py index ac0cab90..323002be 100644 --- a/haiku_rag_slim/haiku/rag/sources/fs.py +++ b/haiku_rag_slim/haiku/rag/sources/fs.py @@ -5,7 +5,6 @@ import os from collections.abc import AsyncIterator from datetime import UTC, datetime from pathlib import Path -from urllib.parse import unquote, urlparse from haiku.rag.client.exceptions import UnsupportedSourceError from haiku.rag.sources.base import ( @@ -19,14 +18,7 @@ from haiku.rag.sources.filter import ( FileFilter, _default_supported_extensions, ) - - -def _uri_to_path(uri: str) -> Path: - parsed = urlparse(uri) - if parsed.scheme in ("", "file"): - path = parsed.path if parsed.scheme == "file" else uri - return Path(unquote(path)) - raise ValueError(f"Unsupported URI scheme for FSSource: {uri}") +from haiku.rag.uri import is_local_uri, uri_to_path def walk_files(root: Path) -> list[Path]: @@ -92,7 +84,7 @@ class FSSource: returns None, `fetch()` raises ``UnsupportedSourceError``. """ try: - path = _uri_to_path(uri).resolve(strict=False) + path = uri_to_path(uri).resolve(strict=False) except (ValueError, OSError): return None if not path.is_relative_to(self.root): @@ -100,8 +92,7 @@ class FSSource: return path def supports(self, uri: str) -> bool: - scheme = urlparse(uri).scheme - if scheme not in ("", "file"): + if not is_local_uri(uri): return False return self._resolve_within_root(uri) is not None diff --git a/haiku_rag_slim/haiku/rag/sources/registry.py b/haiku_rag_slim/haiku/rag/sources/registry.py index 76b9620b..4ae01d9b 100644 --- a/haiku_rag_slim/haiku/rag/sources/registry.py +++ b/haiku_rag_slim/haiku/rag/sources/registry.py @@ -7,6 +7,7 @@ from haiku.rag.sources.base import Source from haiku.rag.sources.fs import FSSource from haiku.rag.sources.http import HTTPSource from haiku.rag.sources.s3 import S3Source +from haiku.rag.uri import is_local_uri, uri_to_path def resolve_configured_source( @@ -49,11 +50,13 @@ def resolve_adhoc_fetcher( if src.supports(uri): return src - scheme = urlparse(uri).scheme - if scheme in ("", "file"): + if is_local_uri(uri): # Root only matters for discover(); fetch() needs an absolute path - # that already encodes the location, so any root is correct. - return FSSource(root=Path("/")) + # that already encodes the location, so the path's own anchor is + # enough. On Windows "/" is only the current drive. + return FSSource(root=Path(uri_to_path(uri).anchor or "/")) + + scheme = urlparse(uri).scheme if scheme in ("http", "https"): return HTTPSource(source_id="http:adhoc") if scheme == "s3": diff --git a/haiku_rag_slim/haiku/rag/uri.py b/haiku_rag_slim/haiku/rag/uri.py new file mode 100644 index 00000000..66a55e7c --- /dev/null +++ b/haiku_rag_slim/haiku/rag/uri.py @@ -0,0 +1,41 @@ +import re +from pathlib import Path +from urllib.parse import urlparse +from urllib.request import url2pathname + +_WINDOWS_ABSOLUTE_PATH = re.compile(r"^[A-Za-z]:[\\/]") + + +def is_local_uri(uri: str) -> bool: + """True for a ``file://`` URI or a bare filesystem path. + + ``urlparse("C:/docs/a.pdf")`` reports the drive letter as scheme ``c``. + Require the separator after it so URI-like text such as ``x:content`` is + not mistaken for a path. + """ + scheme = urlparse(uri).scheme + return scheme in ("", "file") or bool(_WINDOWS_ABSOLUTE_PATH.match(uri)) + + +def uri_to_path(uri: str) -> Path: + """Filesystem path for a ``file://`` URI or a bare path. + + ``file://`` URIs percent-encode special characters, and on Windows carry a + leading slash before the drive (``file:///C:/docs``) that ``Path`` would + keep. ``url2pathname`` handles both, per platform. + """ + parsed = urlparse(uri) + if parsed.scheme == "file": + host, path = parsed.netloc, parsed.path + # file:////server/share spells a UNC path with an empty authority, the + # host being the first path segment (RFC 8089 appendix E.3.2). + if not host and path.startswith("//"): + host, _, path = path[2:].partition("/") + # url2pathname needs the host urlparse split off to build a UNC path, + # but Python 3.14 rejects one handed to it on a non-Windows platform. + # localhost denotes the current machine and is intentionally omitted. + authority = f"//{host}" if host.lower() not in ("", "localhost") else "" + return Path(f"{authority}{url2pathname('/' + path.lstrip('/'))}") + if is_local_uri(uri): + return Path(uri) + raise ValueError(f"Not a local URI: {uri}") diff --git a/tests/sources/test_resolve_fetcher.py b/tests/sources/test_resolve_fetcher.py index efb64558..848bb6b6 100644 --- a/tests/sources/test_resolve_fetcher.py +++ b/tests/sources/test_resolve_fetcher.py @@ -23,6 +23,12 @@ def test_adhoc_resolves_fs_for_bare_path(): assert isinstance(src, FSSource) +def test_adhoc_resolves_fs_for_windows_path(): + """urlparse reads the drive letter as a scheme; it is still a local path.""" + src = resolve_adhoc_fetcher("C:/docs/sample.md") + assert isinstance(src, FSSource) + + def test_adhoc_resolves_http(): src = resolve_adhoc_fetcher("https://example.com/x.pdf") assert isinstance(src, HTTPSource) diff --git a/tests/test_client.py b/tests/test_client.py index a5661750..f7be1022 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2493,6 +2493,13 @@ def test_check_source_accessible_file_uri(tmp_path): assert check_source_accessible((tmp_path / "gone.txt").as_uri()) is False +def test_check_source_accessible_percent_encoded_file_uri(tmp_path): + existing = tmp_path / "a[b] c.txt" + existing.write_text("x") + + assert check_source_accessible(existing.as_uri()) is True + + class _CountingSource: """A real Source over one in-memory document that counts its closes.""" diff --git a/tests/test_processing.py b/tests/test_processing.py index 29ec8fdc..f5feabe2 100644 --- a/tests/test_processing.py +++ b/tests/test_processing.py @@ -264,3 +264,14 @@ async def test_convert_rejects_bad_file_uris(tmp_path, make_source, match): with pytest.raises(UnsupportedSourceError, match=match): await convert(AppConfig(), make_source(tmp_path)) + + +@pytest.mark.asyncio +async def test_convert_percent_encoded_file_uri(tmp_path): + """`Path.as_uri()` encodes brackets and spaces; convert must decode them.""" + target = tmp_path / "a[b] c.md" + target.write_text("# Heading") + + doc = await convert(AppConfig(), target.as_uri()) + + assert "Heading" in doc.export_to_markdown() diff --git a/tests/test_uri.py b/tests/test_uri.py new file mode 100644 index 00000000..e96cc0b8 --- /dev/null +++ b/tests/test_uri.py @@ -0,0 +1,64 @@ +"""Tests for haiku.rag.uri.""" + +import os +from pathlib import Path, PureWindowsPath + +import pytest + +from haiku.rag.uri import is_local_uri, uri_to_path + + +@pytest.mark.parametrize( + "uri,expected", + [ + ("file:///tmp/a.md", True), + ("/tmp/a.md", True), + ("relative/a.md", True), + # urlparse reads the Windows drive letter as the scheme. + ("C:/docs/a.pdf", True), + ("c:\\docs\\a.pdf", True), + ("x:content", False), + ("http://example.com/a.pdf", False), + ("https://example.com/a.pdf", False), + ("s3://bucket/a.pdf", False), + ("webdav://host/a.pdf", False), + ], +) +def test_is_local_uri(uri, expected): + assert is_local_uri(uri) is expected + + +def test_uri_to_path_decodes_percent_escapes(): + assert uri_to_path("file:///tmp/a%5Bb%5D%20c.md") == Path("/tmp/a[b] c.md") + + +def test_uri_to_path_leaves_bare_paths_alone(): + """A bare path is not a URI, so ``a%20b.md`` is a filename, not ``a b.md``.""" + assert uri_to_path("/tmp/a%20b.md") == Path("/tmp/a%20b.md") + + +def test_uri_to_path_rejects_remote_schemes(): + with pytest.raises(ValueError, match="Not a local URI"): + uri_to_path("s3://bucket/key.pdf") + + +@pytest.mark.parametrize("authority", ["localhost", "LOCALHOST"]) +def test_uri_to_path_omits_localhost_authority(authority): + assert uri_to_path(f"file://{authority}/tmp/a.md") == Path("/tmp/a.md") + + +@pytest.mark.parametrize( + "uri", + ["file://server/share/a.md", "file:////server/share/a.md"], + ids=["authority", "empty_authority"], +) +def test_uri_to_path_reads_unc_host(uri): + """Both spellings name a UNC host, the second through an empty authority.""" + path = uri_to_path(uri) + assert path == Path("//server/share/a.md") + assert PureWindowsPath(path) == PureWindowsPath(r"\\server\share\a.md") + + +@pytest.mark.skipif(os.name != "nt", reason="Windows path semantics") +def test_uri_to_path_strips_windows_drive_prefix(): + assert uri_to_path("file:///C:/docs/a.pdf") == Path("C:\\docs\\a.pdf")