Skip to content

An FTP resume spliced a changed remote when there was no cache entry#830

Open
xroche wants to merge 10 commits into
masterfrom
fix/ftp-nocache-splice-823
Open

An FTP resume spliced a changed remote when there was no cache entry#830
xroche wants to merge 10 commits into
masterfrom
fix/ftp-nocache-splice-823

Conversation

@xroche

@xroche xroche commented Jul 27, 2026

Copy link
Copy Markdown
Owner

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 an hts-cache someone deleted) nothing separates a genuine partial from a stale complete copy, so back_add() took the resume offset from the file size and FTP sent REST. 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 .ref marker the HTTP path relies on and the fexist branch 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 when SIZE reports a remote strictly longer than the local copy and MDTM reports a date equal to the local file's mtime. The mirror is stamped from MDTM, 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 any MDTM, and an ordering test lets all of them through.

Two costs and one hole worth naming. MDTM now 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 offset with a byte-identical file, while four shapes must not resume: a longer changed remote, a same-length one, a server that refuses MDTM, 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

xroche and others added 6 commits July 27, 2026 13:49
…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>
@xroche
xroche changed the base branch from fix/ftp-update-rest to master July 27, 2026 14:46
xroche and others added 4 commits July 27, 2026 16:48
#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An FTP resume can splice a changed remote when there is no cache entry

1 participant