Re: bin/58281: ftp(1) should have a strict failure exit code option

[Luke Mewburn <luke%mewburn.net@localhost>]

On 24-05-25 12:10, martin%NetBSD.org@localhost wrote:
  | >Number:         58281
  | >Category:       bin
  | >Synopsis:       ftp(1) should have a strict failure exit code option
  | >Description:
  | 
  | It is possible to use ftp(1) in scripts to auto-download e.g. software
  | updates. But if the connection fails midway (due to network issues)
  | a sucess error code is returned to the shell. This makes auto-update
  | scripts prone to silent failure.

That's a sensible request.

If the file size is known, and the transferred bytes is short (taking
into account any restart point), then that could be considered a fetch
failure.


  | Example:
  | 
  | Requesting https://.....
  |   4% |*                                  |  6779 KiB    9.78 KiB/s  4:30:23 ETA
  | 6941845 bytes retrieved in 11:32 (9.78 KiB/s)
  | 
  | and exit status is zero. As you can see from the transfer rate the connection
  | was ... challenged, and something timed out and droped the whole connection
  | midway. The client does not report this in any way that I can find, and
  | the announced size of the file is not easily available to the script
  | unless the whole application transfers a manifest first.
  | 
  | >Fix:
  | Add a command line option to force the exit status to indicate failure
  | if less than the original announced number of bytes have been received?

I think that failing could be the default behaviour, instead of adding a
new option.

If you can (relatively) easily reproduce the issue, can you test if this
patch does what you want?

I've only fixed it for fetch_url(), because I /think/ that fetch_ftp
should already detect the error.


diff --git i/usr.bin/ftp/fetch.c w/usr.bin/ftp/fetch.c
index a2ccd8c3897a..83c9ec15df08 100644
--- i/usr.bin/ftp/fetch.c
+++ w/usr.bin/ftp/fetch.c
@@ -1856,6 +1856,11 @@ chunkerror:
 	}
 	if (bytes > 0)
 		ptransfer(0);
+
+		/* fail if short transfer when filesize is known */
+	if (filesize >= 0 && (bytes + restart_point < filesize))
+		goto cleanup_fetch_url;
+
 	bytes = 0;
 
 	rval = 0;