An FTP resume spliced a changed remote when there was no cache entry#830
Open
xroche wants to merge 10 commits into
Open
An FTP resume spliced a changed remote when there was no cache entry#830xroche wants to merge 10 commits into
xroche wants to merge 10 commits into
Conversation
…ld file into the new body FTP sent REST whenever the mirrored file merely existed, and on an --update pass every previously mirrored file exists, so a complete copy was treated as an interrupted download. The server resumed at its length and the mirror ended up part old body, part new tail, at exactly the remote size, so nothing downstream noticed. Resuming now follows the decision back_add() already makes for HTTP, which only marks a copy partial when the cache does not hold it, and r.size is seeded from the resume offset so a genuine resume is no longer reported "FTP file incomplete". Closes #798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
git-clang-format only sees the diff present when it runs; the comments were translated after it, so those lines never went through it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
With no cache entry to tell a partial from a stale copy, back_add() took range_req_size from the file size and the transfer resumed with REST, so a remote that had changed since had its tail appended to the old head at exactly the remote length. FTP has no conditional retrieval, so the client has to decide. Resuming now needs SIZE to report a remote strictly longer than the local copy and MDTM to report it no newer than that copy. The mirror is stamped from MDTM as the HTTP path is stamped from Last-Modified, so later passes compare two server-clock times; a server that does not answer MDTM proves nothing and is re-fetched whole. Closes #823 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
run_launch_ftp() already resets msg, statuscode and size for a retry; without lastmodified in that list a retry whose MDTM fails stamps the mirror with the previous attempt's date. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
"Remote no newer than the local copy" is one-sided, and it lets through exactly the mirrors #823 is about: nothing stamped an FTP file before this branch, so every existing one carries a client-clock mtime ahead of any MDTM, as does a copy restored from an archive or moved without preserving times. The first pass over such a tree resumed and spliced. The date now has to match, which is what the HTTP path gets from the server via If-Unmodified-Since. A partial written here is stamped from the same MDTM, so the resume it exists for still compares equal. Pass 6 plants a stale copy dated by the local clock over a larger changed remote; it splices under the old comparison. Pass 5 now dates its copy as the remote so the missing MDTM is the only thing refusing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
#810 landed, so its branch is gone and this stacks on master now. The REST condition keeps the MDTM guard on top of the #798 gate, and the TESTS list takes 110 and 111 from master beside 121. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Register 121_local-ftp-nocache-splice.test in tests-list.mk after the #845 TESTS-list restructure moved it out of tests/Makefile.am. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
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.
Stacks on #810 and must not land before it: that PR rewrites the same decision, and this one covers the case it leaves open.
With no cache entry (
--cache=0, or anhts-cachesomeone deleted) nothing separates a genuine partial from a stale complete copy, soback_add()took the resume offset from the file size and FTP sentREST. A remote that had changed since then got its tail appended to the old head, landing at exactly the remote length, so no size check downstream noticed.FTP has no conditional retrieval, so the decision falls to the client. Refusing every cacheless resume would disable FTP resume altogether, since FTP never writes the
.refmarker the HTTP path relies on and thefexistbranch is the only resume it has. Comparing bytes across the overlap works on any server but costs an extra data connection every time. So resume only whenSIZEreports a remote strictly longer than the local copy andMDTMreports a date equal to the local file's mtime. The mirror is stamped fromMDTM, which nothing did for FTP before, so a partial written here compares equal on the next pass and still resumes. The comparison has to be equality rather than "no newer", because every mirror made before this change carries a client-clock mtime ahead of anyMDTM, and an ordering test lets all of them through.Two costs and one hole worth naming.
MDTMnow goes out for every FTP file, including fresh fetches with no local copy: one extra round trip, and what makes the stamping possible. A server too old to answer it proves nothing and gets a full re-fetch. And a remote that grows while keeping its mtime is still indistinguishable from a partial, so that shape can still splice.Test 121 asserts on the server's command log rather than the mirror alone. A genuine partial must still come back as
REST offsetwith a byte-identical file, while four shapes must not resume: a longer changed remote, a same-length one, a server that refusesMDTM, and a stale copy dated by the local clock. All four fail on #810's head, on the bytes as well as the log, and the last one also fails on the first draft of this PR.Closes #823