Follow HTTP redirects in the WebDAV source
This commit is contained in:
parent
84b778b4ea
commit
fac62cb347
4 changed files with 215 additions and 13 deletions
|
|
@ -12,6 +12,7 @@
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
- WebDAV source follows HTTP redirects (`PROPFIND` and `GET`); front-ended servers that 301/302 on trailing-slash or virtual-host rewrites no longer error.
|
||||||
- All in-process pdfium access (page slicing and embedded-attachment scanning) is serialized under a single shared lock. Concurrent ingester workers no longer corrupt libpdfium's global state, which previously failed valid PDFs with "Data format error".
|
- All in-process pdfium access (page slicing and embedded-attachment scanning) is serialized under a single shared lock. Concurrent ingester workers no longer corrupt libpdfium's global state, which previously failed valid PDFs with "Data format error".
|
||||||
- Embedded PDF attachment extension is derived from the attachment filename, not the parent's synthetic `...#attachment=<name>` URI; non-PDF attachments (e.g. `.joboptions`) are no longer misrouted to docling's PDF backend, and unsupported extensions are skipped.
|
- Embedded PDF attachment extension is derived from the attachment filename, not the parent's synthetic `...#attachment=<name>` URI; non-PDF attachments (e.g. `.joboptions`) are no longer misrouted to docling's PDF backend, and unsupported extensions are skipped.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -146,6 +146,15 @@ collection are emitted as DELETE.
|
||||||
|
|
||||||
Fetches are plain HTTP GETs — any WebDAV server already supports them.
|
Fetches are plain HTTP GETs — any WebDAV server already supports them.
|
||||||
|
|
||||||
|
Redirects are followed for both `PROPFIND` and `GET`, so front-ended
|
||||||
|
servers that 30x on trailing-slash normalisation or scheme upgrades (e.g.
|
||||||
|
Plone) work without extra configuration. Discovered URIs stay anchored to
|
||||||
|
`base_url` (the `GET` fetch follows redirects to the bytes). A same-host
|
||||||
|
scheme upgrade (`http`→`https`) is transparent; a redirect that moves the
|
||||||
|
collection to a different path or host makes discovery raise so you can
|
||||||
|
point `base_url` at the new location rather than silently dropping every
|
||||||
|
file. Credentials are never replayed to a different host on a redirect.
|
||||||
|
|
||||||
Bearer-token auth can replace HTTP Basic via the standard `headers` map:
|
Bearer-token auth can replace HTTP Basic via the standard `headers` map:
|
||||||
|
|
||||||
```yaml
|
```yaml
|
||||||
|
|
|
||||||
|
|
@ -34,6 +34,8 @@ _TAG_GETLASTMODIFIED = f"{{{_DAV_NS}}}getlastmodified"
|
||||||
_TAG_GETCONTENTTYPE = f"{{{_DAV_NS}}}getcontenttype"
|
_TAG_GETCONTENTTYPE = f"{{{_DAV_NS}}}getcontenttype"
|
||||||
|
|
||||||
|
|
||||||
|
_MAX_PROPFIND_REDIRECTS = 5
|
||||||
|
|
||||||
_PROPFIND_BODY = b"""<?xml version="1.0" encoding="utf-8"?>
|
_PROPFIND_BODY = b"""<?xml version="1.0" encoding="utf-8"?>
|
||||||
<propfind xmlns="DAV:">
|
<propfind xmlns="DAV:">
|
||||||
<prop>
|
<prop>
|
||||||
|
|
@ -197,8 +199,18 @@ class WebDAVSource:
|
||||||
if self.username is not None and self.password is not None
|
if self.username is not None and self.password is not None
|
||||||
else None
|
else None
|
||||||
)
|
)
|
||||||
|
# Follow redirects: Plone and other front-ended WebDAV servers 30x on
|
||||||
|
# trailing-slash normalisation and virtual-host rewrites; httpx defaults
|
||||||
|
# follow_redirects to False, which would surface those as errors. GET
|
||||||
|
# and HEAD (fetch) follow at the client level. PROPFIND (head/discover)
|
||||||
|
# is followed by hand in `_propfind` because httpx downgrades PROPFIND to
|
||||||
|
# GET on 302/303, which the final endpoint would reject or answer with a
|
||||||
|
# non-multistatus body.
|
||||||
self._http = httpx.AsyncClient(
|
self._http = httpx.AsyncClient(
|
||||||
auth=auth, headers=self.headers, transport=transport
|
auth=auth,
|
||||||
|
headers=self.headers,
|
||||||
|
transport=transport,
|
||||||
|
follow_redirects=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
def supports(self, uri: str) -> bool:
|
def supports(self, uri: str) -> bool:
|
||||||
|
|
@ -207,13 +219,43 @@ class WebDAVSource:
|
||||||
async def aclose(self) -> None:
|
async def aclose(self) -> None:
|
||||||
await self._http.aclose()
|
await self._http.aclose()
|
||||||
|
|
||||||
async def head(self, uri: str) -> str | None:
|
async def _propfind(self, url: str, *, depth: str) -> tuple[httpx.Response, str]:
|
||||||
response = await self._http.request(
|
"""Issue a PROPFIND, following redirects by hand so the method and body
|
||||||
"PROPFIND",
|
are preserved (httpx turns a redirected PROPFIND into a GET on 302/303).
|
||||||
uri,
|
Returns the final response and the URL it was served from.
|
||||||
headers={"Depth": "0", "Content-Type": "application/xml"},
|
|
||||||
content=_PROPFIND_BODY,
|
Each hop re-issues through the client, which re-applies `auth`, so a
|
||||||
|
cross-host redirect is refused *before* the request is sent — otherwise
|
||||||
|
we would leak Basic credentials to the redirect target (httpx only
|
||||||
|
strips auth cross-host for its own auto-followed redirects)."""
|
||||||
|
host = urlparse(url).netloc
|
||||||
|
current = url
|
||||||
|
for _ in range(_MAX_PROPFIND_REDIRECTS):
|
||||||
|
response = await self._http.request(
|
||||||
|
"PROPFIND",
|
||||||
|
current,
|
||||||
|
headers={"Depth": depth, "Content-Type": "application/xml"},
|
||||||
|
content=_PROPFIND_BODY,
|
||||||
|
follow_redirects=False,
|
||||||
|
)
|
||||||
|
location = response.headers.get("location")
|
||||||
|
if response.is_redirect and location:
|
||||||
|
current = urljoin(current, location)
|
||||||
|
if urlparse(current).netloc != host:
|
||||||
|
raise ValueError(
|
||||||
|
f"WebDAV PROPFIND {url!r} redirected to a different host "
|
||||||
|
f"{current!r}; refusing to send credentials. Update "
|
||||||
|
f"base_url to the new host."
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
return response, current
|
||||||
|
raise httpx.TooManyRedirects(
|
||||||
|
f"Exceeded {_MAX_PROPFIND_REDIRECTS} redirects for PROPFIND {url}",
|
||||||
|
request=response.request,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def head(self, uri: str) -> str | None:
|
||||||
|
response, _ = await self._propfind(uri, depth="0")
|
||||||
if response.is_error:
|
if response.is_error:
|
||||||
return None
|
return None
|
||||||
entries = _parse_multistatus(response.content)
|
entries = _parse_multistatus(response.content)
|
||||||
|
|
@ -267,13 +309,22 @@ class WebDAVSource:
|
||||||
now = datetime.now(UTC)
|
now = datetime.now(UTC)
|
||||||
seen: set[str] = set()
|
seen: set[str] = set()
|
||||||
|
|
||||||
response = await self._http.request(
|
response, final_url = await self._propfind(self.base_url, depth="infinity")
|
||||||
"PROPFIND",
|
|
||||||
self.base_url,
|
|
||||||
headers={"Depth": "infinity", "Content-Type": "application/xml"},
|
|
||||||
content=_PROPFIND_BODY,
|
|
||||||
)
|
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
|
# hrefs are resolved and filtered against base_url, and the worker
|
||||||
|
# resolves URIs back to this source by base_url too. A redirect that
|
||||||
|
# moves the collection to a different path would make every href fall
|
||||||
|
# outside base_url — silently emitting DELETEs for all known docs. Fail
|
||||||
|
# loudly instead so the operator points base_url at the new location.
|
||||||
|
# (_propfind already rejects cross-host redirects, so only the path can
|
||||||
|
# differ here; a same-path scheme upgrade stays transparent.)
|
||||||
|
if urlparse(final_url).path.rstrip("/") != urlparse(self.base_url).path.rstrip(
|
||||||
|
"/"
|
||||||
|
):
|
||||||
|
raise ValueError(
|
||||||
|
f"WebDAV collection {self.base_url!r} redirected to a different "
|
||||||
|
f"path {final_url!r}; update base_url to the new location."
|
||||||
|
)
|
||||||
entries = _parse_multistatus(response.content)
|
entries = _parse_multistatus(response.content)
|
||||||
|
|
||||||
for entry in entries:
|
for entry in entries:
|
||||||
|
|
|
||||||
|
|
@ -434,6 +434,147 @@ async def test_discover_emits_upsert_for_unknown_uri_without_revision():
|
||||||
assert non_delete[0].kind is SourceEventKind.UPSERT
|
assert non_delete[0].kind is SourceEventKind.UPSERT
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_fetch_follows_redirect():
|
||||||
|
"""Plone commonly 301s (trailing-slash normalisation, VHM rewrites); the
|
||||||
|
client must follow to fetch the real bytes instead of returning the 3xx."""
|
||||||
|
body = b"redirected body"
|
||||||
|
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
if request.url.path == "/dav/a.md":
|
||||||
|
return httpx.Response(
|
||||||
|
301, headers={"location": "https://nc.example.com/dav/final.md"}
|
||||||
|
)
|
||||||
|
assert request.url.path == "/dav/final.md"
|
||||||
|
return httpx.Response(
|
||||||
|
200,
|
||||||
|
content=body,
|
||||||
|
headers={"content-type": "text/markdown", "etag": '"rev-1"'},
|
||||||
|
)
|
||||||
|
|
||||||
|
src = WebDAVSource(
|
||||||
|
source_id="nc",
|
||||||
|
base_url="https://nc.example.com/dav/",
|
||||||
|
transport=_transport(handler),
|
||||||
|
)
|
||||||
|
result = await src.fetch("https://nc.example.com/dav/a.md")
|
||||||
|
assert result.body == body
|
||||||
|
assert result.revision == "rev-1"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_discover_follows_redirect_preserving_propfind():
|
||||||
|
"""A 302 on PROPFIND is followed with the method preserved (httpx would
|
||||||
|
downgrade it to GET). A same-path scheme upgrade is transparent, and hrefs
|
||||||
|
stay anchored to base_url so stored URIs remain stable for the worker."""
|
||||||
|
methods: list[str] = []
|
||||||
|
multistatus = _multistatus(
|
||||||
|
{"href": "/dav/a.md", "etag": '"rev-a"', "content_type": "text/markdown"},
|
||||||
|
)
|
||||||
|
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
methods.append(request.method)
|
||||||
|
if request.url.scheme == "http":
|
||||||
|
return httpx.Response(
|
||||||
|
302, headers={"location": "https://nc.example.com/dav/"}
|
||||||
|
)
|
||||||
|
return httpx.Response(207, content=multistatus)
|
||||||
|
|
||||||
|
src = WebDAVSource(
|
||||||
|
source_id="nc",
|
||||||
|
base_url="http://nc.example.com/dav/",
|
||||||
|
transport=_transport(handler),
|
||||||
|
)
|
||||||
|
events = [event async for event in src.discover()]
|
||||||
|
assert methods == ["PROPFIND", "PROPFIND"]
|
||||||
|
assert [e.uri for e in events] == ["http://nc.example.com/dav/a.md"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_discover_raises_when_collection_relocates():
|
||||||
|
"""If the collection root redirects to a different path, the multistatus
|
||||||
|
hrefs fall outside base_url. Resolving them against base_url would skip
|
||||||
|
every file and DELETE all known docs — fail loudly instead."""
|
||||||
|
multistatus = _multistatus(
|
||||||
|
{"href": "/dav2/a.md", "etag": '"rev-a"', "content_type": "text/markdown"},
|
||||||
|
)
|
||||||
|
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
if request.url.path == "/dav/":
|
||||||
|
return httpx.Response(
|
||||||
|
301, headers={"location": "https://nc.example.com/dav2/"}
|
||||||
|
)
|
||||||
|
return httpx.Response(207, content=multistatus)
|
||||||
|
|
||||||
|
src = WebDAVSource(
|
||||||
|
source_id="nc",
|
||||||
|
base_url="https://nc.example.com/dav/",
|
||||||
|
transport=_transport(handler),
|
||||||
|
)
|
||||||
|
with pytest.raises(ValueError, match="redirected to a different path"):
|
||||||
|
[event async for event in src.discover()]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_discover_refuses_cross_host_redirect_without_sending_credentials():
|
||||||
|
"""A PROPFIND redirected to another host must not have the configured
|
||||||
|
credentials replayed to that host (httpx only strips auth cross-host for
|
||||||
|
its own auto-followed redirects, not our manual loop)."""
|
||||||
|
seen_hosts: list[str | None] = []
|
||||||
|
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
seen_hosts.append(request.url.host)
|
||||||
|
return httpx.Response(
|
||||||
|
302, headers={"location": "https://evil.example.com/dav/"}
|
||||||
|
)
|
||||||
|
|
||||||
|
src = WebDAVSource(
|
||||||
|
source_id="nc",
|
||||||
|
base_url="https://nc.example.com/dav/",
|
||||||
|
username="alice",
|
||||||
|
password="hunter2",
|
||||||
|
transport=_transport(handler),
|
||||||
|
)
|
||||||
|
with pytest.raises(ValueError, match="different host"):
|
||||||
|
[event async for event in src.discover()]
|
||||||
|
assert "evil.example.com" not in seen_hosts
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_head_follows_redirect_preserving_propfind():
|
||||||
|
methods: list[str] = []
|
||||||
|
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
methods.append(request.method)
|
||||||
|
if request.url.path == "/dav/a.md":
|
||||||
|
return httpx.Response(
|
||||||
|
302, headers={"location": "https://nc.example.com/dav/final.md"}
|
||||||
|
)
|
||||||
|
return httpx.Response(207, content=_multistatus({"href": "/x", "etag": '"r"'}))
|
||||||
|
|
||||||
|
src = WebDAVSource(
|
||||||
|
source_id="nc",
|
||||||
|
base_url="https://nc.example.com/dav/",
|
||||||
|
transport=_transport(handler),
|
||||||
|
)
|
||||||
|
assert await src.head("https://nc.example.com/dav/a.md") == "r"
|
||||||
|
assert methods == ["PROPFIND", "PROPFIND"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_propfind_redirect_loop_is_bounded():
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
return httpx.Response(302, headers={"location": "https://nc.example.com/loop/"})
|
||||||
|
|
||||||
|
src = WebDAVSource(
|
||||||
|
source_id="nc",
|
||||||
|
base_url="https://nc.example.com/dav/",
|
||||||
|
transport=_transport(handler),
|
||||||
|
)
|
||||||
|
with pytest.raises(httpx.TooManyRedirects):
|
||||||
|
[event async for event in src.discover()]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_fetch_rejects_file_exceeding_max_size():
|
async def test_fetch_rejects_file_exceeding_max_size():
|
||||||
def handler(request: httpx.Request) -> httpx.Response:
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue