Bug 169748

Summary:

[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

Product:

Base System

Reporter:

Enji Cooper <ngie>

Component:

bin

Assignee:

Alfonso S. Siciliano <asiciliano>

Status:

Open ---

Severity:

Affects Only Me

CC:

asiciliano, grahamperrin, tom

Priority:

Normal

Keywords:

install, patch

Version:

9.0-RELEASE

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
file.diff	none
bin-169748.patch	none

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

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

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.

Comment 5 Alfonso S. Siciliano freebsd_committer

2022-01-21 03:26:05 UTC

Same problem some day ago with CURRENT 14, it is an easily reproducible error, review D33978

Comment 6 commit-hook freebsd_committer

2022-02-23 00:58:27 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=55af0f96d0fb92e49fad0c63e7b062c419197459

commit 55af0f96d0fb92e49fad0c63e7b062c419197459
Author:     Alfonso S. Siciliano <asiciliano@FreeBSD.org>
AuthorDate: 2022-02-23 00:54:51 +0000
Commit:     Alfonso S. Siciliano <asiciliano@FreeBSD.org>
CommitDate: 2022-02-23 00:54:51 +0000

    bsdinstall/distfetch: fix main bar percentage with errors

    UI fix not related to the real fetching process, use 'nfiles'
    (instead of 'total files size') to compute main bar percentage
    if an error occurs:

     - fix: main bar greater than 100%, if an error occurs before fetching
     - fix: main bar less than 100%, if an error occurs during fetching
     - add: last mixedgauge, at least one dialog if a total failure occurs

    PR:             164094, 169748
    Approved by:    bapt (mentor)
    Review:         https://reviews.freebsd.org/D33978

 usr.sbin/bsdinstall/distfetch/distfetch.c | 33 ++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

Comment 7 Graham Perrin freebsd_committer

2022-10-17 12:39:16 UTC

Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>