Bug 35

Summary:	$MAIL contains extraneous slashes on Solaris, IRIX, others?
Product:	Portable OpenSSH	Reporter:	Dan Astoorian <djast>
Component:	sshd	Assignee:	Kevin Steves <stevesk>
Status:	CLOSED WONTFIX
Severity:	minor
Priority:	P4
Version:	-current
Hardware:	All
OS:	Solaris

Description Dan Astoorian 2001-12-11 09:40:21 AEDT

On some platforms, the $MAIL environment variable set by the daemon in session.c
(when UseLogin=no) contains an extraneous slash between the spool directory and
the userid; e.g., "/var/mail//djast" instead of "/var/mail/djast" .  Although
this is usually harmless, since the pathname is still valid with the null
pathname component, the extra slash reportedly confuses RMAIL under emacs in
such a way that it will not read the mailbox.

The cause of the bug is that the vendor's definition of MAILDIR under Solaris,
and of _PATH_MAILDIR undir IRIX, includes a trailing slash (i.e., "/var/mail/"
and "/usr/mail/" respectively).  Conversely, _PATH_MAILDIR under RedHat Linux
does not end with a slash.

Possible solutions:

[a] don't use _PATH_MAILDIR directly in session.c; use a different preprocessor
symbol, e.g., _PATH_SSH_MAILDIR.  Use configure.ac to find the vendor-supplied
symbol, determine the presence or absence of the trailing slash, and set the new
symbol to consistently omit (or include) the slash.

[b] in session.c, test at run time whether _PATH_MAILDIR ends in a slash, and
don't snprintf() the additional slash if it does.

[c] use realpath() on MAILDIR before appending the slash and pw->pw_name (this
seems excessive, and incurs unnecessary filesystem activity).

The first approach seems cleanest to me; let me know if I can contribute code.

(Reporting this into bugzilla for-the-record: I first reported this to
<openssh@openssh.com> on 21 Jun 2001.)

Comment 1 Damien Miller 2001-12-29 00:59:21 AEDT

/var/mail//djast is a valid path which should resolve to /var/mail/djast on any
Unix system. Isn't it RMAIL which is broken here?

Comment 2 Dan Astoorian 2001-12-29 03:54:56 AEDT

Yes, the bug is in RMAIL (and has likely been fixed in newer revisions
of RMAIL), but a fix/workaround in sshd would still be nice, even if
only for cosmetic reasons.

Comment 3 Kevin Steves 2002-01-06 12:02:02 AEDT

what is the decision with this one?  i don't think we should add code
that works around buggy apps that have been fixed.  it never goes away.
but if we want to change it, i might do something like this:

Index: session.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/session.c,v
retrieving revision 1.117
diff -u -r1.117 session.c
--- session.c	28 Dec 2001 14:50:54 -0000	1.117
+++ session.c	6 Jan 2002 00:55:00 -0000
@@ -855,6 +855,7 @@
 	env[0] = NULL;
 
 	if (!options.use_login) {
+		char *delim;
 		/* Set basic environment. */
 		child_set_env(&env, &envsize, "USER", pw->pw_name);
 		child_set_env(&env, &envsize, "LOGNAME", pw->pw_name);
@@ -866,8 +867,12 @@
 		child_set_env(&env, &envsize, "PATH", _PATH_STDPATH);
 #endif
 
-		snprintf(buf, sizeof buf, "%.200s/%.50s",
-			 _PATH_MAILDIR, pw->pw_name);
+		delim = "/";
+		if (sizeof(_PATH_MAILDIR) > 1 &&
+		    _PATH_MAILDIR[sizeof(_PATH_MAILDIR)-2] == '/')
+			delim = "";
+		snprintf(buf, sizeof buf, "%.200s%s%.50s",
+		    _PATH_MAILDIR, delim, pw->pw_name);
 		child_set_env(&env, &envsize, "MAIL", buf);
 
 		/* Normal systems set SHELL by default. */

Comment 4 Damien Miller 2002-01-14 20:00:41 AEDT

Can't we fix it in configure by stripping terminating slashes?

Comment 5 Kevin Steves 2002-01-24 10:28:37 AEDT

yes. however, i think this should probably be wontfix.

Comment 6 Tim Rice 2002-02-26 08:40:39 AEDT

I aggree with Kevin Steves this should probably be wontfix.
However, here is a workaround for you. This should work for Solaris
and any SVR4/SVR5. Copy /usr/linclude/maillock.h to your ssh build dir.
Edit the #define MAILDIR line and remove the trailing /
That should solve the problem with your broken apps.
Another option that might work is to remove the
#define HAVE_MAILLOCK_H 1 line from config.h before compiling.

Comment 7 Kevin Steves 2002-03-31 06:21:05 AEST

wontfix

Comment 8 Damien Miller 2004-04-14 12:24:17 AEST

Mass change of RESOLVED bugs to CLOSED