From: "H. Hartzer" <h@hartzer.sh>
Subject: [PATCH] Make incorrect ftp(1) usage more obvious
To: <tech@openbsd.org>
Date: Fri, 02 May 2025 15:38:54 +0000

Hi tech@,

I'm coming from a background of curl, wget, and fetch where I
frequently run my commands with the options after, like:
curl https://openbsd.org -o index.html
Unfortunately, this makes ftp(1)'s behavior a bit confusing.

For example:

ftp https://openbsd.org/index.html -o dir/openbsd-index.html
will silently put index.html into the current directory and exit 0.

ftp(1) only accepts arguments at the start of the command. None after
the URL(s).

This patch informs the user if they are making such a mistake. It also
adjusts the exit code for usage errors to 2, which makes testing a
little bit easier. It includes some basic regression testing.

Let me know if you have any questions or if this needs any adjustments.

Thanks!

-Henrich



diff --git a/regress/usr.bin/ftp/Makefile b/regress/usr.bin/ftp/Makefile
index e0c7f5994a1..8878b077492 100644
--- a/regress/usr.bin/ftp/Makefile
+++ b/regress/usr.bin/ftp/Makefile
@@ -2,7 +2,8 @@
 
 REGRESS_TARGETS= \
 		${DASHO_TARGETS:C/^/t-dasho-/} \
-		${REDIRECT_TARGETS:C/^/t-redirect-/}
+		${REDIRECT_TARGETS:C/^/t-redirect-/} \
+		${USAGE_TARGETS:C/^/t-usage-/} \
 
 CLEANFILES=	*.log
 
@@ -110,5 +111,18 @@ t-redirect-15:
 
 REDIRECT_TARGETS=1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
 
+usage=	@sh ${.CURDIR}/usage.sh
+
+#	${usage}	<args>							<$?>
+
+# Any invalid argument should show usage.
+t-usage-1:
+	${usage}	--some-random-argument					2
+
+# Invalid argument order.
+t-usage-2:
+	${usage}	"http://localhost/index.html -o /dev/null"		2
+
+USAGE_TARGETS=1 2
 
 .include <bsd.regress.mk>
diff --git a/usr.bin/ftp/main.c b/usr.bin/ftp/main.c
index 14758450aef..4d0162e3395 100644
--- a/usr.bin/ftp/main.c
+++ b/usr.bin/ftp/main.c
@@ -626,6 +626,17 @@ main(volatile int argc, char *argv[])
 					err(1, "pledge");
 			}
 
+			/*
+			* auto_fetch assumes that every remaining argument is a URL.
+			* Let's makes sure that's the case. Helps prevents mistakes like:
+			* ftp https://openbsd.org -o openbsd-index.html instead of
+			* ftp -o openbsd-index.html https://openbsd.org.
+			*/
+			for (int argpos = 0; argpos < argc; argpos++) {
+				if (!isurl(argv[argpos]))
+					usage();
+			}
+
 			rval = auto_fetch(argc, argv, outfile);
 			/* -1 == connected and cd-ed */
 			if (rval >= 0 || outfile != NULL)
@@ -1088,5 +1099,5 @@ usage(void)
 	    "       ftp [-N name] [-o output] host:/file[/] ...\n"
 #endif /* !SMALL */
 	    );
-	exit(1);
+	exit(2);
 }