NetBSD Problem Report #54764

From elo@foobox.net  Sat Dec 14 20:16:52 2019
Return-Path: <elo@foobox.net>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-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 E88757A18A
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 14 Dec 2019 20:16:51 +0000 (UTC)
Message-Id: <20191214201645.E5786821A@marmite.localnet>
Date: Sat, 14 Dec 2019 20:16:45 +0000 (GMT)
From: elo@foobox.net
Reply-To: elo@foobox.net
To: gnats-bugs@NetBSD.org
Subject: Incorrect '+file' filename completion in mail(1)
X-Send-Pr-Version: 3.95

>Number:         54764
>Category:       bin
>Synopsis:       Incorrect '+file' filename completion in mail(1)
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    christos
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Dec 14 20:20:00 +0000 2019
>Last-Modified:  Tue Apr 21 17:20:40 +0000 2020
>Originator:     elo
>Release:        NetBSD 9.0_BETA
>Organization:
>Environment:
System: NetBSD marmite.localnet 9.0_BETA NetBSD 9.0_BETA (BLUEBELL) #3: Sun Nov 3 02:10:56 GMT 2019 elo@marmite.localnet:/usr/obj/sys/arch/amd64/compile/BLUEBELL amd64
Architecture: x86_64
Machine: amd64
>Description:
	If the 'folder' command of the mail(1) client program is
	provided the stem of a file argument employing the special
	'+file' naming convention, filename completion does not
	operate as expected. The initial '+' character indicates to
	the mail client that the filename to follow resides in the
	user's mail folder directory (if the user has defined such
	a directory), but the filename completion code treats the
	'+' as the first character of the filename, with no special
	significance.
>How-To-Repeat:
	Assume the current working directory has a file '+foo', and
	the user's mail folder directory contains an mbox file 'foo'.
	If the user enters the mail client command 'folder +fo', then
	requests filename completion, the file '+foo' in the working
	directory will be matched and expanded, not the file 'foo' in
	the folder directory. In my view, for the mail client to be
	self-consistent, it ought to be the other way round.
>Fix:
	Apply the following patch (or something like it). With this
	modification, if a mail folder directory has been defined and
	the filename stem provided to the 'folder' command begins
	with '+' and contains no '/' characters, filename completion
	will look for matches within the folder directory. To match a
	file elsewhere that has '+' as the first character, specify
	a relative or absolute path before the filename stem, e.g.,
	'folder ./+foo'.

--- a/usr/src/usr.bin/mail/complete.c	2019-12-14 13:12:21.684617910 +0000
+++ b/usr/src/usr.bin/mail/complete.c	2019-12-14 13:21:02.456628354 +0000
@@ -332,16 +332,27 @@
 {
 	StringList *words;
 	char dir[MAXPATHLEN];
-	char *fname;
+	char *fname, *mf;
 	DIR *dd;
 	struct dirent *dp;
 	unsigned char rv;
 	size_t len;

 	if ((fname = strrchr(word, '/')) == NULL) {
-		dir[0] = '.';
-		dir[1] = '\0';
-		fname = word;
+		if (word[0] == '+' && (mf = value(ENAME_FOLDER)) != NULL) {
+			if (mf[0] == '/') {
+				(void)estrlcpy(dir, mf, sizeof(dir));
+			} else {
+				dir[0] = '~';
+				dir[1] = '/';
+				(void)estrlcpy(dir + 2, mf, sizeof(dir) - 2);
+			}
+			fname = word + 1;
+		} else {
+			dir[0] = '.';
+			dir[1] = '\0';
+			fname = word;
+		}
 	} else {
 		if (fname == word) {
 			dir[0] = '/';

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54764 CVS commit: src/usr.bin/mail
Date: Sat, 14 Dec 2019 15:28:03 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sat Dec 14 20:28:03 UTC 2019

 Modified Files:
 	src/usr.bin/mail: complete.c

 Log Message:
 PR/54764: elo: Incorrect '+file' filename completion in mail(1)
 Add propel completion stem so that file completion works.
 pullup-9


 To generate a diff of this commit:
 cvs rdiff -u -r1.20 -r1.21 src/usr.bin/mail/complete.c

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

From: Steffen Nurpmeso <steffen@sdaoden.eu>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/54764 CVS commit: src/usr.bin/mail
Date: Sat, 14 Dec 2019 23:40:51 +0100

 Christos Zoulas wrote in <20191214203001.F26F07A1C8@mollari.NetBSD.org>:
  |The following reply was made to PR bin/54764; it has been noted by GNATS.
  |
  |From: "Christos Zoulas" <christos@netbsd.org>
  |To: gnats-bugs@gnats.NetBSD.org
  |Cc: 
  |Subject: PR/54764 CVS commit: src/usr.bin/mail
  |Date: Sat, 14 Dec 2019 15:28:03 -0500
  |
  | Module Name: src
  | Committed By:        christos
  | Date:                Sat Dec 14 20:28:03 UTC 2019
  | 
  | Modified Files:
  |  src/usr.bin/mail: complete.c
  | 
  | Log Message:
  | PR/54764: elo: Incorrect '+file' filename completion in mail(1)
  | Add propel completion stem so that file completion works.
  | pullup-9

 POSIX says "unset or set to null", so assuming that NetBSD Mail
 does not return NULL when doing "set folder=" (or "set folder")
 the above needs an additional "&& *mf != '\0')" for POSIX.

  | To generate a diff of this commit:
  | cvs rdiff -u -r1.20 -r1.21 src/usr.bin/mail/complete.c
  | 
  | Please note that diffs are not public domain; they are subject to the
  | copyright notices on the relevant files.
  |
  --End of <20191214203001.F26F07A1C8@mollari.NetBSD.org>

 --steffen
 |
 |Der Kragenbaer,                The moon bear,
 |der holt sich munter           he cheerfully and one by one
 |einen nach dem anderen runter  wa.ks himself off
 |(By Robert Gernhardt)

From: elo@foobox.net
To: gnats-bugs@NetBSD.org
Cc: christos@netbsd.org, steffen@sdaoden.eu
Subject: Re: bin/54764
Date: Mon, 16 Dec 2019 13:52:24 +0000 (GMT)

 Hello, Steffen.

 Thank you for taking the time to look at the patch I submitted.

 > From: Steffen Nurpmeso <steffen@sdaoden.eu>
 > Date: Sat, 14 Dec 2019 23:40:51 +0100
 >
 > POSIX says "unset or set to null", so assuming that NetBSD Mail
 > does not return NULL when doing "set folder=" (or "set folder")
 > the above needs an additional "&& *mf != '\0')" for POSIX.

 I had debated whether to qualify that conditional clause with the
 set-but-empty test, but ultimately decided not to do when I realised
 it would put the file-completion code's treatment of the 'folder'
 variable at odds with that of the mail client proper.

 Try this: invoke 'mail' whilst your working directory is other than
 your home directory. With 'folder' unset (and NULL), the 'folders'
 command will return 'No value set for "folder"'. But after any of
 'set folder', 'set folder=' or 'set folder=""' (all three of which
 set 'folder' to an empty string), the 'folders' command will list
 the contents of your _home_ directory.

 As per the mail man page, 'If [the folder] name begins with a `/',
 mail considers it to be an absolute pathname; otherwise, the folder
 directory is found relative to your home directory.' That's true
 even if the name is a zero-length string. The client itself is
 making a distinction between NULL and set-but-empty, and I wanted
 the file-completion behaviour to mimic it.

 If I've misled myself by this argument, I'd be very grateful to be
 put right. In any case, even if I'm not persuaded to change my mind,
 I'm happy to accept the consensus regarding the patch.

 Thanks again for your interest.

 elo

From: Steffen Nurpmeso <steffen@sdaoden.eu>
To: elo@foobox.net
Cc: gnats-bugs@NetBSD.org, christos@netbsd.org
Subject: Re: bin/54764
Date: Mon, 16 Dec 2019 22:48:39 +0100

 Hello elo.

 elo@foobox.net wrote in <20191216135224.74849821A@marmite.localnet>:
  |Thank you for taking the time to look at the patch I submitted.
  |
  |> From: Steffen Nurpmeso <steffen@sdaoden.eu>
  |> Date: Sat, 14 Dec 2019 23:40:51 +0100
  |>
  |> POSIX says "unset or set to null", so assuming that NetBSD Mail
  |> does not return NULL when doing "set folder=" (or "set folder")
  |> the above needs an additional "&& *mf != '\0')" for POSIX.
  |
  |I had debated whether to qualify that conditional clause with the
  |set-but-empty test, but ultimately decided not to do when I realised
  |it would put the file-completion code's treatment of the 'folder'
  |variable at odds with that of the mail client proper.
  |
  |Try this: invoke 'mail' whilst your working directory is other than
  |your home directory. With 'folder' unset (and NULL), the 'folders'
  |command will return 'No value set for "folder"'. But after any of
  |'set folder', 'set folder=' or 'set folder=""' (all three of which
  |set 'folder' to an empty string), the 'folders' command will list
  |the contents of your _home_ directory.

 That is not standardized behaviour.  But it seems Unix V10 does it
 like that, and BSD Mail introduced this behaviour in 1982 already!

   Author:     Kurt Shoens <kurt>
   AuthorDate: 1982-03-15 09:43:52 -0800
   Commit:     Kurt Shoens <kurt>
   CommitDate: 1982-03-15 09:43:52 -0800

       Included Eric's branch changes to remove vax and vmunix
       dependencies.

  |As per the mail man page, 'If [the folder] name begins with a `/',
  |mail considers it to be an absolute pathname; otherwise, the folder
  |directory is found relative to your home directory.' That's true
  |even if the name is a zero-length string. The client itself is
  |making a distinction between NULL and set-but-empty, and I wanted
  |the file-completion behaviour to mimic it.
  |
  |If I've misled myself by this argument, I'd be very grateful to be
  |put right. In any case, even if I'm not persuaded to change my mind,
  |I'm happy to accept the consensus regarding the patch.
  |
  |Thanks again for your interest.

 Since POSIX mostly standardizes existing behaviour i think the
 current POSIX wording is wrong, and you are right.
 In fact i had to fix an according bug in my Mail clone, thanks for
 that.  Be warned you have been credited as elo, please ring
 through if you want name/address/x changes, thank you.  (Will be
 included in next release in a few days already.)

 Thanks.

  --End of <20191216135224.74849821A@marmite.localnet>

 --steffen
 |
 |Der Kragenbaer,                The moon bear,
 |der holt sich munter           he cheerfully and one by one
 |einen nach dem anderen runter  wa.ks himself off
 |(By Robert Gernhardt)

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54764 CVS commit: [netbsd-9] src/usr.bin/mail
Date: Tue, 17 Dec 2019 12:30:36 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue Dec 17 12:30:36 UTC 2019

 Modified Files:
 	src/usr.bin/mail [netbsd-9]: complete.c mail.1 mime_child.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #555):

 	usr.bin/mail/mime_child.c: revision 1.10
 	usr.bin/mail/mail.1: revision 1.68
 	usr.bin/mail/complete.c: revision 1.21

 PR/54766: elo: Broken mime-hooks handling in mail(1)
 pullup-9.

 PR/54765: elo: Minor error in the mail(1) man page (-F description truncated)
 pullup-9

 PR/54764: elo: Incorrect '+file' filename completion in mail(1)
 Add propel completion stem so that file completion works.
 pullup-9


 To generate a diff of this commit:
 cvs rdiff -u -r1.20 -r1.20.48.1 src/usr.bin/mail/complete.c
 cvs rdiff -u -r1.65.2.1 -r1.65.2.2 src/usr.bin/mail/mail.1
 cvs rdiff -u -r1.9 -r1.9.6.1 src/usr.bin/mail/mime_child.c

 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->christos
Responsible-Changed-By: maya@NetBSD.org
Responsible-Changed-When: Tue, 21 Apr 2020 17:20:40 +0000
Responsible-Changed-Why:
Hi christos, it looks like you committed the patch suggested by elo. can you offer a final opinion on the following discussion? can this PR be closed? thanks!


>Unformatted:

Home	PR Database Search