Change "servers" mts.conf entry to only support a single SMTP server.

As part of this change, fix SMTP code so the chosen SMTP server is
configured correctly for the netsec code.

diff --git a/INSTALL b/INSTALL
index ca1c1df..246cf0e 100644 (file)
--- a/INSTALL
+++ b/INSTALL
@@ -260,7 +260,7 @@ Options for configure
      is not given, then the configuration process will search for the
      command `more' and use it as the default.
 
      is not given, then the configuration process will search for the
      command `more' and use it as the default.
 

---with-smtpservers='SMTPSERVER1[ SMTPSERVER2...]'    (DEFAULT is localhost)
+--with-smtpserver='SMTPSERVER'    (DEFAULT is localhost)

      If this option is not specified, the mts.conf file will contain
      the line "servers: localhost", which may be manually edited later.
      You may find it convenient to specify a value at configure-time,
      If this option is not specified, the mts.conf file will contain
      the line "servers: localhost", which may be manually edited later.
      You may find it convenient to specify a value at configure-time,

diff --git a/Makefile.am b/Makefile.am
index c79adeb..bbda834 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -526,7 +526,7 @@ etc/mts.conf: $(srcdir)/etc/mts.conf.in Makefile
        @rm -f $@
        $(SED) -e 's,%mts%,$(MTS),' \
               -e 's,%mailspool%,$(mailspool),' \
        @rm -f $@
        $(SED) -e 's,%mts%,$(MTS),' \
               -e 's,%mailspool%,$(mailspool),' \

-              -e 's,%smtpservers%,$(smtpservers),' \
+              -e 's,%smtpserver%,$(smtpserver),' \

               -e 's,%default_locking%,$(default_locking),' \
               -e 's,%supported_locks%,$(supported_locks),' \
                                < $(srcdir)/etc/mts.conf.in > $@
               -e 's,%default_locking%,$(default_locking),' \
               -e 's,%supported_locks%,$(supported_locks),' \
                                < $(srcdir)/etc/mts.conf.in > $@

diff --git a/build_nmh b/build_nmh
index 6e96756..6958faa 100755 (executable)
--- a/build_nmh
+++ b/build_nmh
@@ -222,7 +222,7 @@ fi
 config_prefix=/usr/local/nmh
 config_locking=
 config_mts=smtp
 config_prefix=/usr/local/nmh
 config_locking=
 config_mts=smtp

-config_smtpservers=localhost
+config_smtpserver=localhost

 config_sasl='determined by configure'
 config_tls='determined by configure'
 config_debug=n
 config_sasl='determined by configure'
 config_tls='determined by configure'
 config_debug=n


@@ -252,9 +252,8 @@ if install-mh -check >/dev/null 2>&1; then
 
     mtsconfservers=`grep '^servers:' "$mtsconf"`
     if [ "$mtsconfservers" ]; then
 
     mtsconfservers=`grep '^servers:' "$mtsconf"`
     if [ "$mtsconfservers" ]; then

-      servers=`echo "$mtsconfservers" | \
-               sed -e 's/^servers: *//' -e 's/ /\\\ /g'`
-      [ "$servers" ]  &&  config_smtpservers="$servers"
+      servers=`echo "$mtsconfservers" | sed -e 's/^servers: *//'`
+      [ "$servers" ]  &&  config_smtpserver="$servers"

     fi
   fi
 
     fi
   fi
 


@@ -294,9 +293,9 @@ if [ $yes -eq 0 ]; then
   [ "$mts" ]  &&  config_mts="$mts"
 
   if [ "$config_mts" = smtp ]; then
   [ "$mts" ]  &&  config_mts="$mts"
 
   if [ "$config_mts" = smtp ]; then

-    printf 'SMTP server(s), space separated [%s]: ' $config_smtpservers >&3
-    read response
-    [ "$response" ]  &&  config_smtpservers="$response"
+    printf 'SMTP server [%s]: ' $config_smtpserver >&3
+    read server
+    [ "$server" ]  &&  config_smtpserver="$servers"

   fi
 
   printf 'Cyrus SASL support (y|n) [%s]: ' "$config_sasl" >&3
   fi
 
   printf 'Cyrus SASL support (y|n) [%s]: ' "$config_sasl" >&3


@@ -314,8 +313,8 @@ config_opts="--prefix=$config_prefix"
   config_opts="$config_opts --with-locking=$config_locking"
 [ "$config_mts"  -a  "$config_mts" != smtp ]  &&
   config_opts="$config_opts --with-mts=$config_mts"
   config_opts="$config_opts --with-locking=$config_locking"
 [ "$config_mts"  -a  "$config_mts" != smtp ]  &&
   config_opts="$config_opts --with-mts=$config_mts"

-[ "$config_smtpservers"  -a  "$config_smtpservers" != localhost ]  &&
-  config_opts="$config_opts --with-smtpservers=\"$config_smtpservers\""
+[ "$config_smtpserver"  -a  "$config_smtpserver" != localhost ]  &&
+  config_opts="$config_opts --with-smtpserver=$config_smtpserver"

 [ "$config_sasl" = y ]  &&  config_opts="$config_opts --with-cyrus-sasl"
 [ "$config_tls" = y ]  &&  config_opts="$config_opts --with-tls"
 [ $config_debug = y ]  &&  config_opts="$config_opts --enable-assert"
 [ "$config_sasl" = y ]  &&  config_opts="$config_opts --with-cyrus-sasl"
 [ "$config_tls" = y ]  &&  config_opts="$config_opts --with-tls"
 [ $config_debug = y ]  &&  config_opts="$config_opts --enable-assert"


@@ -374,9 +373,8 @@ if [ -z "$CFLAGS" ]; then
   fi
 fi
 
   fi
 fi
 

-printf "\n./configure CFLAGS=\"${CFLAGS}\" $config_opts\n"
-#### Use eval to handle space-separated smtpservers.
-eval ./configure CFLAGS=\""${CFLAGS}"\" $config_opts
+printf '\n./configure %s\n' "$config_opts"
+./configure CFLAGS="${CFLAGS}" $config_opts

 status=$?
 
 if [ $status -eq 0 ]; then
 status=$?
 
 if [ $status -eq 0 ]; then

diff --git a/configure.ac b/configure.ac
index ae98df8..d1a755c 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -59,13 +59,13 @@ AS_IF([test x"$with_mts" = x"smtp"], [MTS="smtp"],
       [MTS="smtp"])
 AC_SUBST([MTS])dnl
 
       [MTS="smtp"])
 AC_SUBST([MTS])dnl
 

-dnl What should be the default mail server(s)?
-AC_ARG_WITH([smtpservers],
-  [AS_HELP_STRING([--with-smtpservers='SMTPSERVER1@<:@ SMTPSERVER2...@:>@'],
-  [specify the default SMTP server(s) @<:@localhost@:>@])])
-AS_IF([test -n "$with_smtpservers"], [smtpservers="$with_smtpservers"],
-      [smtpservers="localhost"])
-AC_SUBST([smtpservers])dnl
+dnl What should be the default mail server?
+AC_ARG_WITH([smtpserver],
+  [AS_HELP_STRING([--with-smtpserver='SMTPSERVER'],
+  [specify the default SMTP server @<:@localhost@:>@])])
+AS_IF([test -n "$with_smtpserver"], [smtpserver="$with_smtpserver"],
+      [smtpserver="localhost"])
+AC_SUBST([smtpserver])dnl

 
 dnl -------------------------------------------------------------------
 dnl Default location is /usr/local/nmh/{bin,etc,libexec,man}, unless we
 
 dnl -------------------------------------------------------------------
 dnl Default location is /usr/local/nmh/{bin,etc,libexec,man}, unless we


@@ -645,7 +645,7 @@ RPM build root             : ${nmhrpm}
 backup prefix              : ${backup_prefix}
 transport system           : ${MTS}
 spool default locking type : ${with_locking}
 backup prefix              : ${backup_prefix}
 transport system           : ${MTS}
 spool default locking type : ${with_locking}

-default smtp servers       : ${smtpservers}
+default smtp server        : ${smtpserver}

 SASL support               : ${sasl_support}
 TLS support                : ${tls_support}
 OAuth support              : ${oauth_support}
 SASL support               : ${sasl_support}
 TLS support                : ${tls_support}
 OAuth support              : ${oauth_support}

diff --git a/docs/pending-release-notes b/docs/pending-release-notes
index 7de2628..23cc77c 100644 (file)
--- a/docs/pending-release-notes
+++ b/docs/pending-release-notes
@@ -68,6 +68,7 @@ NEW FEATURES
 - configure now defaults to enabling each of TLS and Cyrus SASL if the
   necessary headers and libraries are found.
 - Moved build_nmh to top-level directory.
 - configure now defaults to enabling each of TLS and Cyrus SASL if the
   necessary headers and libraries are found.
 - Moved build_nmh to top-level directory.

+- Better error reporting for connections to network services

 
 -----------------
 OBSOLETE FEATURES
 
 -----------------
 OBSOLETE FEATURES


@@ -80,6 +81,8 @@ OBSOLETE FEATURES
 - msh(1) was deprecated in nmh 1.6, and was removed in this release.
 - Support in alias files for the the "*" address-group (everyone) was
   deprecated in nmh 1.6, and was removed in this release.
 - msh(1) was deprecated in nmh 1.6, and was removed in this release.
 - Support in alias files for the the "*" address-group (everyone) was
   deprecated in nmh 1.6, and was removed in this release.

+- Support for multiple hostnames in the "servers" entry of mts.conf has
+  been removed.

 
 -------------------
 DEPRECATED FEATURES
 
 -------------------
 DEPRECATED FEATURES

diff --git a/etc/mts.conf.in b/etc/mts.conf.in
index bcf7b2f..0e20f28 100644 (file)
--- a/etc/mts.conf.in
+++ b/etc/mts.conf.in
@@ -46,5 +46,5 @@ mmdflfil:
 # Hardcoded POP server name (prevents inc'ing from local mail spool).
 #pophost: localhost
 
 # Hardcoded POP server name (prevents inc'ing from local mail spool).
 #pophost: localhost
 

-# List of smtp servers to try if using smtp support
-servers: %smtpservers%
+# A SINGLE SMTP server to use when using SMTP support
+servers: %smtpserver%

diff --git a/man/mh-tailor.man b/man/mh-tailor.man
index 226109f..0a3f524 100644 (file)
--- a/man/mh-tailor.man
+++ b/man/mh-tailor.man
@@ -212,12 +212,25 @@ to
 .BR servers :
 localhost
 .RS 5
 .BR servers :
 localhost
 .RS 5

-A lists of hosts and networks which to look for SMTP servers when
-posting non\-local mail.  It turns out this is a major win for hosts
-which don't run an message transport system.  The value of
+A SINGLE hostname to be used when performing mail submission via SMTP.  Previous
+versions of
+.B nmh
+supported multiple hostnames for
+.BR servers ,
+but the current version of
+.B nmh
+only supports a single entry (the name is kept for backwards compatibility
+reasons).  This can be overridden via the
+.B \-server
+option to
+.IR send (1).
+It is not possible to change the mail submission port number in the

 .B servers
 .B servers

-should be one or more items.  Each item is the name of a host which
-is (hopefully) running a SMTP server.
+entry; see the
+.B \-port
+option to
+.BR send (1)
+for this functionality.

 .SS "SendMail"
 This option is only available if you set
 .B mts
 .SS "SendMail"
 This option is only available if you set
 .B mts

diff --git a/mts/smtp/smtp.c b/mts/smtp/smtp.c
index 0e6e511..77bcb6e 100644 (file)
--- a/mts/smtp/smtp.c
+++ b/mts/smtp/smtp.c
@@ -81,7 +81,7 @@ static int smtp_init (char *, char *, char *, int, int, int, int, const char *,
 static int sendmail_init (char *, int, int, int, int, const char *,
                          const char *);
 
 static int sendmail_init (char *, int, int, int, int, const char *,
                          const char *);
 

-static int rclient (char *, char *);
+static int rclient (char *, char *, char **);

 static int sm_ierror (const char *fmt, ...);
 static int sm_nerror (char *);
 static int smtalk (int time, char *fmt, ...);
 static int sm_ierror (const char *fmt, ...);
 static int sm_nerror (char *);
 static int smtalk (int time, char *fmt, ...);


@@ -111,7 +111,7 @@ smtp_init (char *client, char *server, char *port, int watch, int verbose,
            const char *oauth_svc, int tls)
 {
     int result, sd1;
            const char *oauth_svc, int tls)
 {
     int result, sd1;

-    char *errstr;
+    char *errstr, *chosen_server;

 
     if (watch)
        verbose = TRUE;
 
     if (watch)
        verbose = TRUE;


@@ -141,7 +141,14 @@ smtp_init (char *client, char *server, char *port, int watch, int verbose,
     if (user)
        netsec_set_userid(nsc, user);
 
     if (user)
        netsec_set_userid(nsc, user);
 

-    netsec_set_hostname(nsc, server);
+    if ((sd1 = rclient (server, port, &chosen_server)) == NOTOK)
+       return RP_BHST;
+
+    SIGNAL (SIGPIPE, SIG_IGN);
+
+    netsec_set_fd(nsc, sd1, sd1);
+
+    netsec_set_hostname(nsc, chosen_server);

 
     if (sm_debug)
        netsec_set_snoop(nsc, 1);
 
     if (sm_debug)
        netsec_set_snoop(nsc, 1);


@@ -157,13 +164,6 @@ smtp_init (char *client, char *server, char *port, int watch, int verbose,
            return sm_ierror("OAuth2 not supported");
     }
 
            return sm_ierror("OAuth2 not supported");
     }
 

-    if ((sd1 = rclient (server, port)) == NOTOK)
-       return RP_BHST;
-
-    SIGNAL (SIGPIPE, SIG_IGN);
-
-    netsec_set_fd(nsc, sd1, sd1);
-

     if (tls & S_TLSENABLEMASK) {
        if (netsec_set_tls(nsc, 1, tls & S_NOVERIFY, &errstr) != OK)
            return sm_nerror(errstr);
     if (tls & S_TLSENABLEMASK) {
        if (netsec_set_tls(nsc, 1, tls & S_NOVERIFY, &errstr) != OK)
            return sm_nerror(errstr);


@@ -426,11 +426,16 @@ sendmail_init (char *client, int watch, int verbose, int debug, int sasl,
 }
 
 static int
 }
 
 static int

-rclient (char *server, char *service)
+rclient (char *server, char *service, char **chosen_server)

 {
     int sd;
     char response[BUFSIZ];
 
 {
     int sd;
     char response[BUFSIZ];
 

+    if (server == NULL)
+       server = servers;
+
+    *chosen_server = server;
+

     if ((sd = client (server, service, response, sizeof(response),
                      sm_debug)) != NOTOK)
        return sd;
     if ((sd = client (server, service, response, sizeof(response),
                      sm_debug)) != NOTOK)
        return sd;

diff --git a/sbr/client.c b/sbr/client.c
index 9978896..974cf8f 100644 (file)
--- a/sbr/client.c
+++ b/sbr/client.c
@@ -14,39 +14,20 @@
 #include <netinet/in.h>
 #include <netdb.h>
 #include <arpa/inet.h>
 #include <netinet/in.h>
 #include <netdb.h>
 #include <arpa/inet.h>

-
-#define        MAXARGS   1000
-
-/*
- * static prototypes
- */
-
-/* client's own static version of several nmh subroutines */
-static char **client_brkstring (char *, char *, char *);
-static int client_brkany (char, char *);
-static char **client_copyip (char **, char **, int);
-static void client_freelist(char **);
+#include <errno.h>

 
 
 int
 
 
 int

-client (char *args, char *service, char *response, int len_response, int debug)
+client (char *server, char *service, char *response, int len_response,
+       int debug)

 {
     int sd, rc;
 {
     int sd, rc;

-    char **ap, *arguments[MAXARGS];

     struct addrinfo hints, *res, *ai;
 
     struct addrinfo hints, *res, *ai;
 

-    ap = arguments;
-    if (args != NULL && *args != 0) {
-       ap = client_copyip (client_brkstring (mh_xstrdup(args), " ", "\n"),
-               ap, MAXARGS);
-    } else {
-       if (servers != NULL && *servers != 0)
-           ap = client_copyip (client_brkstring (mh_xstrdup(servers), " ", "\n"),
-               ap, MAXARGS);
-    }
-    if (ap == arguments) {
-       *ap++ = mh_xstrdup("localhost");
-       *ap = NULL;
+    if (!server || *server == '\0') {
+       snprintf(response, len_response, "Internal error: NULL server name "
+                "passed to client()");
+       return NOTOK;

     }
 
     memset(&hints, 0, sizeof(hints));
     }
 
     memset(&hints, 0, sizeof(hints));


@@ -56,136 +37,69 @@ client (char *args, char *service, char *response, int len_response, int debug)
     hints.ai_family = PF_UNSPEC;
     hints.ai_socktype = SOCK_STREAM;
 
     hints.ai_family = PF_UNSPEC;
     hints.ai_socktype = SOCK_STREAM;
 

-    for (ap = arguments; *ap; ap++) {
-
-       if (debug) {
-           fprintf(stderr, "Trying to connect to \"%s\" ...\n", *ap);
-       }
-
-       rc = getaddrinfo(*ap, service, &hints, &res);
-
-       if (rc) {
-           if (debug) {
-               fprintf(stderr, "Lookup of \"%s\" failed: %s\n", *ap,
-                       gai_strerror(rc));
-           }
-           continue;
-       }
-
-       for (ai = res; ai != NULL; ai = ai->ai_next) {
-           if (debug) {
-               char address[NI_MAXHOST];
-               char port[NI_MAXSERV];
-
-               rc = getnameinfo(ai->ai_addr, ai->ai_addrlen, address,
-                                sizeof(address), port, sizeof port,
-                                NI_NUMERICHOST | NI_NUMERICSERV);
-
-               fprintf(stderr, "Connecting to %s:%s...\n",
-                       rc ? "unknown" : address,
-                       rc ? "--" : port);
-           }
-
-           sd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
-
-           if (sd < 0) {
-               if (debug)
-                   fprintf(stderr, "socket() failed: %s\n", strerror(errno));
-               continue;
-           }
-
-           if (connect(sd, ai->ai_addr, ai->ai_addrlen) == 0) {
-               freeaddrinfo(res);
-               client_freelist(arguments);
-               return sd;
-           }
-
-           if (debug) {
-               fprintf(stderr, "Connection failed: %s\n", strerror(errno));
-           }
+    if (debug) {
+       fprintf(stderr, "Trying to connect to \"%s\" ...\n", server);
+    }

 
 

-           close(sd);
-       }
+    rc = getaddrinfo(server, service, &hints, &res);

 
 

-       freeaddrinfo(res);
+    if (rc) {
+       snprintf(response, len_response, "Lookup of \"%s\" failed: %s",
+                server, gai_strerror(rc));
+       return NOTOK;

     }
 
     }
 

-    client_freelist(arguments);
-    strncpy (response, "no servers available", len_response);
-    return NOTOK;
-}
+    for (ai = res; ai != NULL; ai = ai->ai_next) {
+       if (debug) {
+           char address[NI_MAXHOST];
+           char port[NI_MAXSERV];

 
 

+           rc = getnameinfo(ai->ai_addr, ai->ai_addrlen, address,
+                            sizeof(address), port, sizeof port,
+                            NI_NUMERICHOST | NI_NUMERICSERV);

 
 

-/*
- * Free a list of strings
- */
+           fprintf(stderr, "Connecting to %s:%s...\n",
+                   rc ? "unknown" : address, rc ? "--" : port);
+       }

 
 

-static void
-client_freelist(char **list)
-{
-    free(*list);
-}
+       sd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);

 
 

+       if (sd < 0) {
+           if (debug)
+               fprintf(stderr, "socket() failed: %s\n", strerror(errno));
+           rc = errno;
+           continue;
+       }

 
 

-/*
- * static copies of three nmh subroutines
- */
+       if (connect(sd, ai->ai_addr, ai->ai_addrlen) == 0) {
+           freeaddrinfo(res);
+           return sd;
+       }

 
 

-static char *broken[MAXARGS + 1];
+       rc = errno;

 
 

-static char **
-client_brkstring (char *strg, char *brksep, char *brkterm)
-{
-    int bi;
-    char c, *sp;
-
-    sp = strg;
-
-    for (bi = 0; bi < MAXARGS; bi++) {
-       while (client_brkany (c = *sp, brksep))
-           *sp++ = 0;
-       if (!c || client_brkany (c, brkterm)) {
-           *sp = 0;
-           broken[bi] = 0;
-           return broken;
+       if (debug) {
+           fprintf(stderr, "Connection failed: %s\n", strerror(errno));

        }
 
        }
 

-       broken[bi] = sp;
-       while ((c = *++sp) && !client_brkany (c, brksep) && !client_brkany (c, brkterm))
-           continue;
+       close(sd);

     }
     }

-    broken[MAXARGS] = 0;

 
 

-    return broken;
-}
+    /*
+     * Improve error handling a bit. If we were given multiple IP addresses
+     * then return the old "no servers available" error, but point the user
+     * to -snoop (hopefully that's universal).  Otherwise report a specific
+     * error.
+     */

 
 

+    if (res->ai_next)
+       snprintf(response, len_response, "no servers available (use -snoop "
+                "for details");
+    else
+       snprintf(response, len_response, "Connection to \"%s\" failed: %s",
+                server, strerror(errno));

 
 

-/*
- * returns 1 if chr in strg, 0 otherwise
- */
-static int
-client_brkany (char chr, char *strg)
-{
-    char *sp;
-
-    if (strg)
-       for (sp = strg; *sp; sp++)
-           if (chr == *sp)
-               return 1;
-    return 0;
-}
-
+    freeaddrinfo(res);

 
 

-/*
- * copy a string array and return pointer to end
- */
-static char **
-client_copyip (char **p, char **q, int len_q)
-{
-    while (*p && --len_q > 0)
-       *q++ = *p++;
-
-    *q = NULL;
-
-    return q;
+    return NOTOK;

 }
 }