Fix handling of errors with strtol(), factor out port parsing

  * Set errno to 0 before calling strtol()
  * Fix warnings in gssconn_{server,client}.c
  * Add tr_parse_port() to tr_inet_util.[ch] and use throughout the
    codebase for parsing port numbers

diff --git a/common/tr_inet_util.c b/common/tr_inet_util.c
index 6d84918..28f52fe 100644 (file)
--- a/common/tr_inet_util.c
+++ b/common/tr_inet_util.c
@@ -39,6 +39,7 @@
 #include <talloc.h>
 
 #include <tr_inet_util.h>
+#include <errno.h>
 
 /**
  * Determine whether a string is a valid address of a given family
@@ -158,7 +159,7 @@ char *tr_parse_host(TALLOC_CTX *mem_ctx, const char *s, int *port_out)
 {
   const char *colon;
   char *hostname;
-  long int port;
+  int port;
 
   if (s == NULL)
     return NULL;
@@ -189,9 +190,9 @@ char *tr_parse_host(TALLOC_CTX *mem_ctx, const char *s, int *port_out)
     if (colon == NULL) {
       *port_out = 0;
     } else {
-      port = strtol(colon+1, NULL, 10);
-      if (tr_str_all_digits(colon+1) && (port > 0) && (port <= 65535))
-        *port_out = (int) port;
+      port = tr_parse_port(colon+1);
+      if ((port > 0) && tr_str_all_digits(colon+1))
+        *port_out = port;
       else
         *port_out = -1;
     }
@@ -218,4 +219,35 @@ TR_NAME *tr_hostname_and_port_to_name(TR_NAME *hn, int port)
   }
 
   return retval;
+}
+
+/**
+ * Parse a string containing a port
+ *
+ * Returns the port number, which is always in the range 1-65535.
+ * On error, returns < 0. The absolute value is an error code from errno.h
+ *
+ * @param s
+ * @return port number or < 0 on error
+ */
+int tr_parse_port(const char *s)
+{
+  long port;
+  char *end;
+
+  errno = 0; /* strtol sets this, make sure it's zero to avoid false positives */
+  port = strtol(s, &end, 10);
+  if (errno) {
+    return -errno;
+  }
+
+  if (*end != '\0') {
+    return -EINVAL;
+  }
+
+  if ((port <= 0) || (port > 65535)) {
+    return -ERANGE;
+  }
+
+  return (int) port;
 }
\ No newline at end of file

diff --git a/gsscon/test/gsscon_client.c b/gsscon/test/gsscon_client.c
index 37430ee..c2815d2 100644 (file)
--- a/gsscon/test/gsscon_client.c
+++ b/gsscon/test/gsscon_client.c
@@ -68,6 +68,7 @@ int main (int argc, const char *argv[])
     int err = 0;
     int fd = -1;
     int port = kDefaultPort;
+    long tmp;
     const char *server = "127.0.0.1";
     const char *clientName = NULL;
     const char *serviceName = "host";
@@ -77,8 +78,14 @@ int main (int argc, const char *argv[])
         
     for (i = 1; (i < argc) && !err; i++) {
         if ((strcmp (argv[i], "--port") == 0) && (i < (argc - 1))) {
-            port = strtol (argv[++i], NULL, 0);
-            if (port == 0) { err = errno; }
+            errno = 0; /* ensure this starts off as 0 */
+            tmp = strtol (argv[++i], NULL, 0);
+            if (errno)
+                err = errno;
+            else if ((tmp <= 0) || (tmp > 65535))
+                err = ERANGE;
+            else
+                port = (int) tmp;
         } else if ((strcmp (argv[i], "--server") == 0) && (i < (argc - 1))) {
             server = argv[++i];
         } else if ((strcmp(argv[i], "--cprinc") == 0) && (i < (argc - 1))) {
@@ -93,7 +100,7 @@ int main (int argc, const char *argv[])
     if (!err) {
         printf ("%s: Starting up...\n", argv[0]);
         
-        err = gsscon_connect (server, port, serviceName, &fd, &gssContext);
+        err = gsscon_connect (server, (unsigned int) port, serviceName, &fd, &gssContext);
     }
     
     if (!err) {

diff --git a/gsscon/test/gsscon_server.c b/gsscon/test/gsscon_server.c
index be56372..3ca69c4 100644 (file)
--- a/gsscon/test/gsscon_server.c
+++ b/gsscon/test/gsscon_server.c
@@ -129,6 +129,7 @@ int main (int argc, const char *argv[])
     int err = 0;
     OM_uint32 minorStatus;
     int port = kDefaultPort;
+    long tmp;
     int listenFD = -1;
     gss_ctx_id_t gssContext = GSS_C_NO_CONTEXT;
     int i = 0;
@@ -136,8 +137,14 @@ int main (int argc, const char *argv[])
 
     for (i = 1; (i < argc) && !err; i++) {
         if ((strcmp (argv[i], "--port") == 0) && (i < (argc - 1))) {
-            port = strtol (argv[++i], NULL, 0);
-            if (port == 0) { err = errno; }
+            errno = 0; /* ensure this starts off at 0 */
+            tmp = strtol (argv[++i], NULL, 0);
+            if (errno)
+                err = errno;
+            else if ((tmp <= 0) || (tmp > 65535))
+                err = ERANGE;
+            else
+                port = (int) tmp;
         } else if ((strcmp(argv[i], "--service") == 0) && (i < (argc - 1))) {
             gServiceName = argv[++i];
         } else {

diff --git a/include/tr_inet_util.h b/include/tr_inet_util.h
index 44d8ba5..f24bbf8 100644 (file)
--- a/include/tr_inet_util.h
+++ b/include/tr_inet_util.h
@@ -41,5 +41,6 @@
 char *tr_parse_host(TALLOC_CTX *mem_ctx, const char *s, int *port_out);
 
 TR_NAME *tr_hostname_and_port_to_name(TR_NAME *hn, int port);
+int tr_parse_port(const char *s);
 
 #endif //TRUST_ROUTER_TR_INET_UTIL_H

diff --git a/tid/example/tidc_main.c b/tid/example/tidc_main.c
index 02cd1db..f1dfce5 100644 (file)
--- a/tid/example/tidc_main.c
+++ b/tid/example/tidc_main.c
@@ -42,6 +42,7 @@
 #include <tid_internal.h>
 #include <trust_router/tr_dh.h>
 #include <trust_router/tid.h>
+#include <tr_inet_util.h>
 
 static void tidc_resp_handler (TIDC_INSTANCE * tidc, 
                        TID_REQ *req,
@@ -146,7 +147,19 @@ static error_t parse_option(int key, char *arg, struct argp_state *state)
       break;
 
     case 4:
-      arguments->port=strtol(arg, NULL, 10); /* optional */
+      arguments->port=tr_parse_port(arg); /* optional */
+      if (arguments->port < 0) {
+        switch(-(arguments->port)) {
+          case ERANGE:
+            printf("\nError parsing port (%s): port must be an integer in the range 1 - 65535\n\n", arg);
+            break;
+
+          default:
+            printf("\nError parsing port (%s): %s\n\n", arg, strerror(-arguments->port));
+            break;
+        }
+        argp_usage(state);
+      }
       break;
 
     default:

diff --git a/tr/trmon_main.c b/tr/trmon_main.c
index 7ec6a4f..b1a2a63 100644 (file)
--- a/tr/trmon_main.c
+++ b/tr/trmon_main.c
@@ -39,6 +39,7 @@
 
 #include <mon_internal.h>
 #include <tr_debug.h>
+#include <tr_inet_util.h>
 
 
 /* command-line option setup */
@@ -75,7 +76,7 @@ struct cmdline_args {
 /* parser for individual options - fills in a struct cmdline_args */
 static error_t parse_option(int key, char *arg, struct argp_state *state)
 {
-  long tmp_l = 0;
+  int err = 0;
 
   /* get a shorthand to the command line argument structure, part of state */
   struct cmdline_args *arguments=state->input;
@@ -84,7 +85,6 @@ static error_t parse_option(int key, char *arg, struct argp_state *state)
     case 'v':
       print_version_info();
       exit(0);
-      break;
 
     case ARGP_KEY_ARG: /* handle argument (not option) */
       switch (state->arg_num) {
@@ -93,31 +93,40 @@ static error_t parse_option(int key, char *arg, struct argp_state *state)
           break;
 
         case 1:
-          tmp_l = strtol(arg, NULL, 10);
-          if (errno || (tmp_l < 0) || (tmp_l > 65535)) /* max valid port */
+          arguments->port=tr_parse_port(arg); /* optional */
+          if (arguments->port < 0) {
+            switch(-(arguments->port)) {
+              case ERANGE:
+                printf("\nError parsing port (%s): port must be an integer in the range 1 - 65535\n\n", arg);
+                break;
+
+              default:
+                printf("\nError parsing port (%s): %s\n\n", arg, strerror(-arguments->port));
+                break;
+            }
             argp_usage(state);
-
-          arguments->port = (int) tmp_l; /* we already checked the range */
+          }
           break;
 
         case 2:
           arguments->command=mon_cmd_from_string(arg);
           if (arguments->command == MON_CMD_UNKNOWN) {
-            printf("\nUnknown command '%s'\n", arg);
-            argp_usage(state);
+            printf("\nUnknown command '%s'\n\n", arg);
+            err = 1;
           }
           break;
 
         default:
           if (arguments->n_options >= MAX_OPTIONS) {
-            printf("\nToo many command options given, limit is %d\n", MAX_OPTIONS);
-            argp_usage(state);
+            printf("\nToo many command options given, limit is %d\n\n", MAX_OPTIONS);
+            err = 1;
+            break;
           }
 
           arguments->options[arguments->n_options] = mon_opt_type_from_string(arg);
           if (arguments->options[arguments->n_options] == OPT_TYPE_UNKNOWN) {
-            printf("\nUnknown command option '%s'\n", arg);
-            argp_usage(state);
+            printf("\nUnknown command option '%s'\n\n", arg);
+            err = 1;
           }
           arguments->n_options++;
           break;
@@ -127,7 +136,7 @@ static error_t parse_option(int key, char *arg, struct argp_state *state)
     case ARGP_KEY_END: /* no more arguments */
       if (state->arg_num < 3) {
         /* not enough arguments encountered */
-        argp_usage(state);
+        err = 1;
       }
       break;
 
@@ -135,6 +144,11 @@ static error_t parse_option(int key, char *arg, struct argp_state *state)
       return ARGP_ERR_UNKNOWN;
   }
 
+  if (err) {
+    argp_usage(state);
+    return EINVAL; /* argp_usage() usually does not return, but just in case */
+  }
+
   return 0; /* success */
 }
 

diff --git a/tr/trpc_main.c b/tr/trpc_main.c
index 039bf51..db29417 100644 (file)
--- a/tr/trpc_main.c
+++ b/tr/trpc_main.c
@@ -41,6 +41,7 @@
 #include <gsscon.h>
 #include <tr_debug.h>
 #include <tr_trp.h>
+#include <tr_inet_util.h>
 
 
 /* command-line option setup */
@@ -92,7 +93,19 @@ static error_t parse_option(int key, char *arg, struct argp_state *state)
       break;
 
     case 2:
-      arguments->port=strtol(arg, NULL, 10); /* optional */
+      arguments->port=tr_parse_port(arg); /* optional */
+      if (arguments->port < 0) {
+        switch(-(arguments->port)) {
+          case ERANGE:
+            printf("\nError parsing port (%s): port must be an integer in the range 1 - 65535\n\n", arg);
+            break;
+
+          default:
+            printf("\nError parsing port (%s): %s\n\n", arg, strerror(-arguments->port));
+            break;
+        }
+        argp_usage(state);
+      }
       break;
 
     default: