NetBSD Problem Report #58281

From martin@aprisoft.de  Sat May 25 12:06:14 2024
Return-Path: <martin@aprisoft.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 498FC1A923C
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 25 May 2024 12:06:14 +0000 (UTC)
Message-Id: <20240525120604.DEB4A5CC79B@emmas.aprisoft.de>
Date: Sat, 25 May 2024 14:06:04 +0200 (CEST)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: ftp(1) should have a strict failure exit code option
X-Send-Pr-Version: 3.95

>Number:         58281
>Category:       bin
>Synopsis:       ftp(1) should have a strict failure exit code option
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    lukem
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat May 25 12:10:01 +0000 2024
>Closed-Date:    Thu Jan 22 22:17:02 +0000 2026
>Last-Modified:  Thu Jan 22 22:17:02 +0000 2026
>Originator:     Martin Husemann
>Release:        NetBSD 10.0_STABLE
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD emmas.aprisoft.de 10.0_STABLE NetBSD 10.0_STABLE (EMMAS) #7: Sun Apr 28 16:05:18 CEST 2024 martin@seven-days-to-the-wolves.aprisoft.de:/work/src-10/sys/arch/amd64/compile/EMMAS amd64
Architecture: x86_64
Machine: amd64
>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.

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.

>How-To-Repeat:
s/a

>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?

>Release-Note:

>Audit-Trail:
From: Luke Mewburn <luke@mewburn.net>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: bin/58281: ftp(1) should have a strict failure exit code option
Date: Fri, 19 Jul 2024 12:39:02 +1000

 On 24-05-25 12:10, martin@NetBSD.org 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;

From: "Luke Mewburn" <lukem@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58281 CVS commit: src/usr.bin/ftp
Date: Fri, 29 Nov 2024 04:31:57 +0000

 Module Name:	src
 Committed By:	lukem
 Date:		Fri Nov 29 04:31:57 UTC 2024

 Modified Files:
 	src/usr.bin/ftp: fetch.c version.h

 Log Message:
 ftp: exit non-zero if short http transfer when filesize is known

 If a http file size is known and the fetch finishes with less bytes
 transferred, exit non-zero.

 Bump version to 20241129.

 PR bin/54713
 PR bin/58281


 To generate a diff of this commit:
 cvs rdiff -u -r1.241 -r1.242 src/usr.bin/ftp/fetch.c
 cvs rdiff -u -r1.98 -r1.99 src/usr.bin/ftp/version.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: bin-bug-people->lukem
Responsible-Changed-By: lukem@NetBSD.org
Responsible-Changed-When: Fri, 29 Nov 2024 04:42:16 +0000
Responsible-Changed-Why:


State-Changed-From-To: open->feedback
State-Changed-By: lukem@NetBSD.org
State-Changed-When: Fri, 29 Nov 2024 04:42:16 +0000
State-Changed-Why:
I decided to fix this to always exit non-zero in this situation.
This should be fixed trunk in src/usr.bin/ftp/fetch.c 1.242.
If so, candidate for pullup, although I need to consider the version
for the branch because there are other changes in trunk not in the       
netbsd-9 or netbsd-10 branches (yet).


State-Changed-From-To: feedback->pending-pullups
State-Changed-By: lukem@NetBSD.org
State-Changed-When: Sun, 01 Dec 2024 09:28:04 +0000
State-Changed-Why:
Part of netbsd-10 pullup request #1021 (which synced all commits from trunk).                 

Still considering whether netbsd-9 pullup is required, as there are other
features and fixes not pulled up either, and the version presented by
ftp on that branch will need to be considered (it shouldn't be "20241129").


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58281 CVS commit: [netbsd-10] src/usr.bin/ftp
Date: Mon, 2 Dec 2024 10:19:39 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Dec  2 10:19:39 UTC 2024

 Modified Files:
 	src/usr.bin/ftp [netbsd-10]: cmds.c extern.h fetch.c ftp.1 ftp.c main.c
 	    progressbar.c ruserpass.c util.c version.h

 Log Message:
 Pull up following revision(s) (requested by lukem in ticket #1021):

 	usr.bin/ftp/ftp.c: revision 1.178
 	usr.bin/ftp/version.h: revision 1.99
 	usr.bin/ftp/ruserpass.c: revision 1.35
 	usr.bin/ftp/main.c: revision 1.134
 	usr.bin/ftp/main.c: revision 1.135
 	usr.bin/ftp/progressbar.c: revision 1.27
 	usr.bin/ftp/util.c: revision 1.165
 	usr.bin/ftp/cmds.c: revision 1.144
 	usr.bin/ftp/extern.h: revision 1.84
 	usr.bin/ftp/fetch.c: revision 1.242
 	usr.bin/ftp/ftp.1: revision 1.160

 s/bninary/binary/ in comment.

 extract duplicate code into a function.

 Check bounds when copying to destination.

 Remove const where the const string ended up being overwritten.

 use unsigned when doing shifts.

 remove const when string gets overwritten.

 ftp: exit non-zero if short http transfer when filesize is known
 If a http file size is known and the fetch finishes with less bytes
 transferred, exit non-zero.
 Bump version to 20241129.
 PR bin/54713
 PR bin/58281

 ftp: help improvements
 Document -? as a separate mode.
 Document -H HEADER in the usage.
 Clarify units for -b and -x.
 Consistent argument names between ftp -? and ftp(1).

 ftp: order getopt Upper before lower
 Consistently order options in getopt and the switch
 with the upper case option before the lower case option.
 This makes it easier to cross-reference with -? and ftp(1).
 No functional change.


 To generate a diff of this commit:
 cvs rdiff -u -r1.141.6.1 -r1.141.6.2 src/usr.bin/ftp/cmds.c
 cvs rdiff -u -r1.82.10.1 -r1.82.10.2 src/usr.bin/ftp/extern.h
 cvs rdiff -u -r1.235.2.3 -r1.235.2.4 src/usr.bin/ftp/fetch.c
 cvs rdiff -u -r1.147.2.6 -r1.147.2.7 src/usr.bin/ftp/ftp.1
 cvs rdiff -u -r1.174.2.2 -r1.174.2.3 src/usr.bin/ftp/ftp.c
 cvs rdiff -u -r1.128.2.2 -r1.128.2.3 src/usr.bin/ftp/main.c
 cvs rdiff -u -r1.24.6.1 -r1.24.6.2 src/usr.bin/ftp/progressbar.c
 cvs rdiff -u -r1.33.86.1 -r1.33.86.2 src/usr.bin/ftp/ruserpass.c
 cvs rdiff -u -r1.164.2.3 -r1.164.2.4 src/usr.bin/ftp/util.c
 cvs rdiff -u -r1.95.2.2 -r1.95.2.3 src/usr.bin/ftp/version.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: lukem@NetBSD.org
State-Changed-When: Thu, 22 Jan 2026 22:17:02 +0000
State-Changed-Why:
Pulled up to netbsd-10. Exists in netbsd-11


>Unformatted:

Home	PR Database Search