Keep the good local copy when an update fetch returns an HTTP error (#176)

On --update, a URL that returned 200 on the first crawl but now answers a
transient error (403/404/5xx) had its good local copy destroyed: the error
body overwrote it (errpage is on by default) and/or the delete_old purge
removed it. The guard that masks such an error as a 304 to keep the cached
copy was gated on !opt->delete_old, but delete_old is on by default, so it
never ran for a normal update.

Drop that outer gate so the masking runs by default, but restrict it to the
complete-cached-copy case (range_req_size == 0): an error on a resume/range
fetch must still fall through, else a stale-partial 416 would be masked to
304 and the partial never re-fetched (regressing #206). The masked error
routes through the not-modified handler, which reloads and re-registers the
cached file, blocking both the overwrite and the purge. Unlinked-page purging
is unaffected: it targets pages not fetched this run, not a fetched page that
errored.

Test 44_local-update-errormask drives it over a local server whose keep.dat
200s on the first crawl and 403s on the conditional update fetch.

Closes #176

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

src/htsback.c

@@ -3540,22 +3540,20 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
                   if (back[i].r.statuscode == 406) {    // 'Not Acceptable'
                     back[i].r.statuscode = HTTP_OK;
                   }
-                  // 'do not erase already downloaded file'
-                  // on an updated file
-                  // with an error : consider a 304 error
-                  if (!opt->delete_old) {
-                    if (HTTP_IS_ERROR(back[i].r.statuscode) && back[i].is_update
-                        && !back[i].testmode) {
-                      if (back[i].url_sav[0] && fexist_utf8(back[i].url_sav)) {
-                        hts_log_print(opt, LOG_DEBUG,
-                                      "Error ignored %d (%s) because of 'no purge' option for %s%s",
-                                      back[i].r.statuscode, back[i].r.msg,
-                                      back[i].url_adr, back[i].url_fil);
-                        back[i].r.statuscode = HTTP_NOT_MODIFIED;
-                        deletehttp(&back[i].r);
-                        back[i].r.soc = INVALID_SOCKET;
-                      }
-                    }
+                  // On update, keep the good copy on error (mask as 304); skip
+                  // resume paths so a stale-partial 416 still re-fetches.
+                  if (HTTP_IS_ERROR(back[i].r.statuscode) &&
+                      back[i].is_update && !back[i].testmode &&
+                      back[i].range_req_size == 0 && back[i].url_sav[0] &&
+                      fexist_utf8(back[i].url_sav)) {
+                    hts_log_print(opt, LOG_DEBUG,
+                                  "Error %d (%s) ignored on update, keeping "
+                                  "existing copy: %s%s",
+                                  back[i].r.statuscode, back[i].r.msg,
+                                  back[i].url_adr, back[i].url_fil);
+                    back[i].r.statuscode = HTTP_NOT_MODIFIED;
+                    deletehttp(&back[i].r);
+                    back[i].r.soc = INVALID_SOCKET;
                   }
                   // Various hacks to limit re-transfers when updating a mirror
                   // Force update if same size detected

tests/44_local-update-errormask.test

@@ -0,0 +1,11 @@
+#!/bin/bash
+# Issue #176: on --update a page that 200'd on the first crawl but now 403s must
+# keep its good local copy, not be overwritten by the error body nor purged.
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+bash "$top_srcdir/tests/local-crawl.sh" --rerun \
+    --found 'errmask/keep.dat' \
+    --file-min-bytes 'errmask/keep.dat' 1024 \
+    httrack 'BASEURL/errmask/index.html'

tests/Makefile.am

@@ -112,6 +112,7 @@ TESTS = \
 	40_local-why.test \
 	41_local-utf8-link.test \
 	42_local-maxsize-slow.test \
-	43_local-update-truncate.test
+	43_local-update-truncate.test \
+	44_local-update-errormask.test
 
 CLEANFILES = check-network_sh.cache

tests/local-server.py

@@ -934,6 +934,33 @@ class Handler(SimpleHTTPRequestHandler):
     def route_mini304_page(self):
         self.big_send(b"<html><body>tiny cacheable page</body></html>\n", "text/html")
 
+    # --- /errmask/: issue #176 — a page that 200'd on the first crawl but 403s
+    # on the update fetch must keep its good copy, not be overwritten nor purged.
+    ERRMASK_GOOD = b"KEEP" + b"." * 1020  # 1024 B distinctive non-HTML body
+    ERRMASK_ERR = b"<html><body>error 403</body></html>\n"
+
+    def route_errmask_index(self):
+        self.send_html('\t<a href="keep.dat">keep</a>\n')
+
+    def route_errmask_keep(self):
+        # First crawl (no validator) gets the 1024 B body + Last-Modified; the
+        # update sends a conditional and gets a 403 error page.
+        if self.headers.get("If-Modified-Since") or self.headers.get("If-None-Match"):
+            self.send_response(403, "Forbidden")
+            self.send_header("Content-Type", "text/html")
+            self.send_header("Content-Length", str(len(self.ERRMASK_ERR)))
+            self.end_headers()
+            if self.command != "HEAD":
+                self.wfile.write(self.ERRMASK_ERR)
+            return
+        self.send_response(200)
+        self.send_header("Content-Type", "application/octet-stream")
+        self.send_header("Last-Modified", BIG_LASTMOD)
+        self.send_header("Content-Length", str(len(self.ERRMASK_GOOD)))
+        self.end_headers()
+        if self.command != "HEAD":
+            self.wfile.write(self.ERRMASK_GOOD)
+
     # --- delayed-type degenerate paths (issues #5/#107) --------------------
     def route_delayed_index(self):
         self.send_html(
@@ -1177,6 +1204,8 @@ class Handler(SimpleHTTPRequestHandler):
         "/redir/target.html": route_redir_target,
         "/mini304/index.html": route_mini304_index,
         "/mini304/page.html": route_mini304_page,
+        "/errmask/index.html": route_errmask_index,
+        "/errmask/keep.dat": route_errmask_keep,
     }
 
     # --- /big/ seeded pseudo-site ------------------------------------------