Repository navigation
Conversation
A server that ignores the Range header replies with the whole file instead of the window that was asked for, and both the async and the sync HTTP cat_file handed that back as if it were the window, so a caller walking a file with cat_ranges got the same bytes at every offset. The streaming file interface already validates the response for the same request; share that check, and read the window out of the whole body when the window starts at the beginning of the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cat_fileon the HTTP filesystem does not check that the response is the window that was asked for, so against a server that ignoresRange—python -m http.server, many proxies — the whole file comes back instead:A consumer that walks a file with
cat_rangesgets the same bytes at every offset, so everything downstream of it is silently misaligned. Both the async and the sync filesystem do this:HTTPFileSystem._cat_filereturns the body as read, andHTTPFileSystem.cat_fileinhttp_sync.pyreturnsr.content.The streaming file interface already validates the response for the same request —
async_fetch_rangechecks for 206, a matchingContent-Rangestart, or aContent-Lengthno larger than the window (#1140 fixed that path). This PR shares that check and uses it in bothcat_fileimplementations, so the two paths can no longer disagree:ValueError, the same one the file interface raises, because a body longer than the window that starts at the requested offset cannot be told apart from one that starts at 0.test_cat_file_rangeon a range-honouring server still passes unchanged, which is the point: the contract there was already right, only the unhonoured case was missing.Test:
pytest fsspec/implementations/tests/test_http.py fsspec/implementations/tests/test_http_sync.py— the four new cases (two server configurations × async and sync) fail onmasterwith the whole body coming back and pass on this branch; the full suite is 2160 passed, 251 skipped, 2 xfailed.ruff checkandruff format(0.14.3, the pinned version) are clean on the five changed files.Not addressed here: a window whose two bounds have opposite signs, like
start=-8, end=1000, needs the file size to resolve, and is passed through as before.#2124also touchescat_filebut adds retries, a different root cause.