Bug 169748 - [patch] bsdinstall(8): when distfile fetch is complete on some distfiles, but not entire set, progress can go over 100%; installer gets confused with distfiles and halts
Summary: [patch] bsdinstall(8): when distfile fetch is complete on some distfiles, but...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 9.0-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-09 20:30 UTC by Enji Cooper
Modified: 2020-07-12 05:09 UTC (History)
1 user (show)

See Also:


Attachments
file.diff (498 bytes, patch)
2012-07-09 20:30 UTC, Enji Cooper
no flags Details | Diff
bin-169748.patch (3.84 KB, patch)
2012-12-02 11:30 UTC, Enji Cooper
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enji Cooper freebsd_committer 2012-07-09 20:30:17 UTC
The FTP download choked on base.txz (corp. firewall issue), but continued downloading the rest of the distfiles, then asked if I wanted to retry downloading the files.

2nd try crashed and burned (the progress bar went over 100% and the installer halted).

Fix: The attached patch (truncate the file) isn't optimal, but it's simple and will address multiple potential issues dealing with distfile fetch failures without introducing having to introduce the whacky state machine that was present in sysinstall (and in many cases got things wrong). An alternate, less failure prone method should be provided which can operate outside of the installer if download retrying is needed.

That and there's no guarantee that the upstream FTP/HTTP server supports download resume until the fetch is attempted.

Patch attached with submission follows:
How-To-Repeat: 1. Download the network installer.
2. Nuke or taint one of the tarballs such that it fetches but fails to extract, or corrupt one of the bytes in the file.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2012-07-10 02:08:15 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-sysinstall

Over to maintainer(s).
Comment 2 Enji Cooper freebsd_committer 2012-12-02 11:30:10 UTC
The attached patch goes beyond what the previous patch did in the 
following ways:

1. Catch errors and terminate whenever a failure occurs.
2. Catch more user errors with undefined environment variables.
3. Catch memory allocation errors.
4. Convert fprintf+return calls to err*(3)/warnx(3) calls.
5. Reorder error handling to avoid potentially confusing mangling via 
dialog(3).

Thanks,
-Garrett
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:01:38 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 4 Thomas Hurst 2020-04-25 03:36:48 UTC
+		if (fetchStatURL(urls[i], &ustat, "") == -1) {
+			snprintf(errormsg, sizeof(errormsg),
+			    "Error while determining file size for %s: %s\n",
+			    urls[i], fetchLastErrString);
+			items[i*2 + 1] = "Failed";
+			dialog_msgbox("Fetch Error", errormsg, 0, 0,
+			    TRUE);
+			return (-1);
+		}

I'd suggest it best here just to quietly reset total_bytes = 0 and abort trying to get the sizes up-front.  HEAD requests failing isn't fatal, the code can fall back to just calculating percentage from the number of files.

I've seen this sort of thing more than once: https://www.reddit.com/r/freebsd/comments/g7gdc9/is_that_normal_overall_progress_208/

i.e. *only* the first fetchStatURL() failed.  Resetting total_bytes to 0 would have left them with a functional but coarse-grained total progress bar, while this patch would have errored out the entire operation.