feat(ipinfo): accept multiple fields

diff --git a/man/mtr.8.in b/man/mtr.8.in
index 14219fcc2c160f2280906e8f002d16899ccc9915..80bdf2309da104e73ab1e11c3ef965822ae52b4c 100644 (file)
--- a/man/mtr.8.in
+++ b/man/mtr.8.in
@@ -359,8 +359,10 @@ I%Interarrival Jitter
 Example:
 -o "LSD NBAW  X"
 .TP
-.B \-y \fIn\fR, \fB\-\-ipinfo \fIn
-Displays information about each IP hop.  Valid values for \fIn\fR are:
+.B \-y \fIFIELDS\fR, \fB\-\-ipinfo \fIFIELDS
+Displays information about each IP hop.
+.I FIELDS
+can be one value or a comma-separated list of values.  Valid field values are:
 .TS
 tab(%);
 ll.
@@ -372,7 +374,7 @@ ll.
 .TE
 .br
 
-It is possible to cycle between these fields at runtime (using the \fBy\fR key).
+In curses mode, use the \fBy\fR key to cycle between these fields one at a time.
 IP information lookup is disabled by default unless this option or
 \fB-z\fR is used.
 .TP

diff --git a/test/cmdparse.py b/test/cmdparse.py
index 914a4785e32dbf1e804d511c3baabd41564d5311..6001caf1b61eb5be88798b02ba981f12990c9844 100755 (executable)
--- a/test/cmdparse.py
+++ b/test/cmdparse.py
@@ -98,7 +98,6 @@ class TestMtrCommandParse(unittest.TestCase):
 
         reply = self.run_mtr(
             '--report',
-
             '--report-cycles',
             '1',
             '--no-dns',
@@ -176,6 +175,31 @@ class TestMtrCommandParse(unittest.TestCase):
             self.assertNotEqual(reply.returncode, 0)
             self.assertIn('Failed to resolve host', reply.stderr)
 
+    def test_ipinfo_accepts_multiple_fields(self):
+        'Test parsing a comma-separated ipinfo field list.'
+
+        reply = self.run_mtr('--help')
+        if '--ipinfo' not in reply.stdout:
+            self.skipTest('No ipinfo support')
+
+        reply = self.run_mtr('--ipinfo', '0,1', '--help')
+
+        self.assertEqual(reply.returncode, 0)
+        self.assertEqual(reply.stderr, '')
+
+    def test_ipinfo_rejects_bad_field_list(self):
+        'Test malformed comma-separated ipinfo field lists.'
+
+        reply = self.run_mtr('--help')
+        if '--ipinfo' not in reply.stdout:
+            self.skipTest('No ipinfo support')
+
+        for fields in ('0,,1', '0,5', '0,x'):
+            reply = self.run_mtr('--ipinfo', fields, '--help')
+
+            self.assertNotEqual(reply.returncode, 0)
+            self.assertIn('ipinfo', reply.stderr)
+
     def test_port_with_tcp_succeeds_flag(self):
         'Test that specifying -P with -T (TCP) succeeds.'
 

diff --git a/ui/asn.c b/ui/asn.c
index 659f91018d3a1809297473af7891fd14d101891d..432a84a4c18cec00dcd74d056026dcd657e87315 100644 (file)
--- a/ui/asn.c
+++ b/ui/asn.c
@@ -67,7 +67,8 @@
 #define UNKN   "???"
 
 static int iihash = 0;
-static char fmtinfo[32];
+static char fmtinfo[128];
+static char fmtfield[128];
 
 /* items width: ASN, Route, Country, Registry, Allocated */
 static const int iiwidth[] = { 12, 19, 4, 8, 11 };       /* item len + space */
@@ -263,7 +264,8 @@ static char *ipinfo_lookup(
 /* originX.asn.cymru.com txtrec:    ASN | Route | Country | Registry | Allocated */
 static char *split_txtrec(
     struct mtr_ctl *ctl,
-    char *txt_rec)
+    char *txt_rec,
+    int ipinfo_no)
 {
     char *prev;
     char *next;
@@ -298,10 +300,10 @@ static char *split_txtrec(
 
     if (i > ctl->ipinfo_max)
         ctl->ipinfo_max = i;
-    if (ctl->ipinfo_no >= i) {
+    if (ipinfo_no >= i) {
         return (*items)[0];
     } else
-        return (*items)[ctl->ipinfo_no];
+        return (*items)[ipinfo_no];
 }
 
 #ifdef ENABLE_IPV6
@@ -328,9 +330,69 @@ static bool is_well_known_nat64(struct in6_addr *addr){
 }
 #endif
 
+void set_ipinfo_field(
+    struct mtr_ctl *ctl,
+    int ipinfo_no)
+{
+    ctl->ipinfo_no = ipinfo_no;
+    ctl->ipinfo_fields[0] = ipinfo_no;
+    ctl->ipinfo_field_count = 1;
+}
+
+void parse_ipinfo_fields(
+    struct mtr_ctl *ctl,
+    const char *fields)
+{
+    char *fields_copy;
+    char *field;
+    char *next;
+
+    fields_copy = xstrdup(fields);
+    ctl->ipinfo_field_count = 0;
+
+    for (field = fields_copy; field != NULL; field = next) {
+        char *end;
+        int ipinfo_no;
+
+        next = strchr(field, ',');
+        if (next != NULL) {
+            *next = '\0';
+            next++;
+        }
+
+        if (*field == '\0') {
+            error(EXIT_FAILURE, 0, "empty ipinfo field in '%s'", fields);
+        }
+
+        errno = 0;
+        ipinfo_no = strtol(field, &end, 0);
+        if (errno != 0 || field == end || *end != '\0') {
+            error(EXIT_FAILURE, errno, "invalid ipinfo field '%s' in '%s'",
+                  field, fields);
+        }
+        if (ipinfo_no < 0 || 4 < ipinfo_no) {
+            error(EXIT_FAILURE, 0, "ipinfo value %d out of range (0 - 4)",
+                  ipinfo_no);
+        }
+        if (ctl->ipinfo_field_count == MAX_IPINFO_FIELDS) {
+            error(EXIT_FAILURE, 0, "too many ipinfo fields in '%s'", fields);
+        }
+
+        ctl->ipinfo_fields[ctl->ipinfo_field_count++] = ipinfo_no;
+    }
+
+    if (ctl->ipinfo_field_count == 0) {
+        error(EXIT_FAILURE, 0, "empty ipinfo field in '%s'", fields);
+    }
+
+    ctl->ipinfo_no = ctl->ipinfo_fields[0];
+    free(fields_copy);
+}
+
 static char *get_ipinfo(
     struct mtr_ctl *ctl,
-    ip_t * addr)
+    ip_t * addr,
+    int ipinfo_no)
 {
     char key[NAMELEN];
     char lookup_key[NAMELEN];
@@ -381,7 +443,7 @@ static char *get_ipinfo(
         item.key = key;
         item.data = NULL;
         if ((found_item = hsearch(item, FIND))) {
-            if (!(val = (*((items_t *) found_item->data))[ctl->ipinfo_no]))
+            if (!(val = (*((items_t *) found_item->data))[ipinfo_no]))
                 val = (*((items_t *) found_item->data))[0];
             DEB_syslog(LOG_INFO, "Found (hashed): %s", val);
         }
@@ -389,7 +451,7 @@ static char *get_ipinfo(
 
     if (!val) {
         DEB_syslog(LOG_INFO, "Lookup: %s", key);
-        if ((val = split_txtrec(ctl, ipinfo_lookup(lookup_key)))) {
+        if ((val = split_txtrec(ctl, ipinfo_lookup(lookup_key), ipinfo_no))) {
             DEB_syslog(LOG_INFO, "Looked up: %s", key);
             if (iihash)
                 if ((item.key = xstrdup(key))) {
@@ -419,22 +481,86 @@ ATTRIBUTE_CONST int get_iiwidth(
     return iiwidth[ipinfo_no % len];
 }
 
+int get_iiwidth_selected(
+    struct mtr_ctl *ctl)
+{
+    int width = 0;
+    int i;
+
+    for (i = 0; i < ctl->ipinfo_field_count; i++) {
+        if (i)
+            width++;
+        width += get_iiwidth(ctl->ipinfo_fields[i]);
+        if (ctl->ipinfo_fields[i] == 0)
+            width += 2;        /* align header: AS */
+    }
+
+    return width;
+}
+
+int ipinfo_field_selected(
+    struct mtr_ctl *ctl,
+    int ipinfo_no)
+{
+    int i;
+
+    for (i = 0; i < ctl->ipinfo_field_count; i++) {
+        if (ctl->ipinfo_fields[i] == ipinfo_no)
+            return 1;
+    }
+
+    return 0;
+}
+
+char *fmt_ipinfo_field(
+    struct mtr_ctl *ctl,
+    ip_t * addr,
+    int ipinfo_no)
+{
+    char *ipinfo = get_ipinfo(ctl, addr, ipinfo_no);
+    char fmt[8];
+
+    snprintf(fmt, sizeof(fmt), "%s%%-%ds", ipinfo_no ? "" : "AS",
+             get_iiwidth(ipinfo_no));
+    snprintf(fmtfield, sizeof(fmtfield), fmt, ipinfo ? ipinfo : UNKN);
+
+    return fmtfield;
+}
+
 char *fmt_ipinfo(
     struct mtr_ctl *ctl,
     ip_t * addr)
 {
-    char *ipinfo = get_ipinfo(ctl, addr);
-    char fmt[8];
-    snprintf(fmt, sizeof(fmt), "%s%%-%ds", ctl->ipinfo_no ? "" : "AS",
-             get_iiwidth(ctl->ipinfo_no));
-    snprintf(fmtinfo, sizeof(fmtinfo), fmt, ipinfo ? ipinfo : UNKN);
+    size_t offset = 0;
+    int i;
+
+    fmtinfo[0] = '\0';
+    for (i = 0; i < ctl->ipinfo_field_count; i++) {
+        int ipinfo_no = ctl->ipinfo_fields[i];
+        char *ipinfo = fmt_ipinfo_field(ctl, addr, ipinfo_no);
+        int written;
+
+        if (i && offset < sizeof(fmtinfo) - 1) {
+            fmtinfo[offset++] = ' ';
+            fmtinfo[offset] = '\0';
+        }
+
+        written = snprintf(fmtinfo + offset, sizeof(fmtinfo) - offset, "%s",
+                           ipinfo);
+        if (written < 0 || (size_t) written >= sizeof(fmtinfo) - offset) {
+            fmtinfo[sizeof(fmtinfo) - 1] = '\0';
+            break;
+        }
+        offset += written;
+    }
+
     return fmtinfo;
 }
 
 int is_printii(
     struct mtr_ctl *ctl)
 {
-    return (ctl->ipinfo_no >= 0);
+    return (ctl->ipinfo_field_count > 0);
 }
 
 void asn_open(

diff --git a/ui/asn.h b/ui/asn.h
index bd8c069be3d0100ba1c7009db8035b89c07c60f9..64d304d6abd11b7b6891b5fb372e45da126c9654 100644 (file)
--- a/ui/asn.h
+++ b/ui/asn.h
@@ -25,9 +25,24 @@ extern void asn_close(
 extern char *fmt_ipinfo(
     struct mtr_ctl *ctl,
     ip_t * addr);
+extern char *fmt_ipinfo_field(
+    struct mtr_ctl *ctl,
+    ip_t * addr,
+    int ipinfo_no);
+extern int ipinfo_field_selected(
+    struct mtr_ctl *ctl,
+    int ipinfo_no);
 extern ATTRIBUTE_CONST size_t get_iiwidth_len(
     void);
 extern ATTRIBUTE_CONST int get_iiwidth(
     int ipinfo_no);
+extern int get_iiwidth_selected(
+    struct mtr_ctl *ctl);
 extern int is_printii(
     struct mtr_ctl *ctl);
+extern void set_ipinfo_field(
+    struct mtr_ctl *ctl,
+    int ipinfo_no);
+extern void parse_ipinfo_fields(
+    struct mtr_ctl *ctl,
+    const char *fields);

diff --git a/ui/curses.c b/ui/curses.c
index 736b1d299b1df0d532627b3d90893ca8cd55399f..90e75dd53bd8bf2a9af6049d6af3c4e177c1811f 100644 (file)
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -949,7 +949,7 @@ void mtr_curses_redraw(
 
 #ifdef HAVE_IPINFO
         if (is_printii(ctl))
-            padding += get_iiwidth(ctl->ipinfo_no);
+            padding += get_iiwidth_selected(ctl);
 #endif
         max_cols =
             maxx <= SAVED_PINGS + padding ? maxx - padding : SAVED_PINGS;

diff --git a/ui/mtr.c b/ui/mtr.c
index 671204dbd96e9b343ba510b65cf0c6626d1ee389..fd97680555fb7372f8bc8335005793da0df35d6d 100644 (file)
--- a/ui/mtr.c
+++ b/ui/mtr.c
@@ -147,7 +147,7 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
     fputs(" -b, --show-ips                   show IP numbers and host names\n", out);
     fputs(" -o, --order FIELDS               select output fields\n", out);
 #ifdef HAVE_IPINFO
-    fputs(" -y, --ipinfo NUMBER              select IP information in output\n",
+    fputs(" -y, --ipinfo FIELDS              select IP information fields in output\n",
           out);
     fputs(" -z, --aslookup                   display AS number\n", out);
     fputs("     --ipinfo_provider4           provider for IPv4 AS lookups\n", out);
@@ -759,15 +759,10 @@ static void parse_arg(
 #endif
 #ifdef HAVE_IPINFO
         case 'y':
-            ctl->ipinfo_no =
-                strtoint_or_err(optarg, "invalid argument");
-            if (ctl->ipinfo_no < 0 || 4 < ctl->ipinfo_no) {
-                error(EXIT_FAILURE, 0, "value %d out of range (0 - 4)",
-                      ctl->ipinfo_no);
-            }
+            parse_ipinfo_fields(ctl, optarg);
             break;
         case 'z':
-            ctl->ipinfo_no = 0;
+            set_ipinfo_field(ctl, 0);
             break;
         case OPT_IPINFO4:
             ctl->ipinfo_provider4 = optarg;
@@ -1066,6 +1061,7 @@ int main(
     ctl.maxDisplayPath = 8;
     ctl.probe_timeout = 10 * 1000000;
     ctl.ipinfo_no = -1;
+    ctl.ipinfo_field_count = 0;
     ctl.ipinfo_max = -1;
 #ifdef HAVE_IPINFO
     ctl.ipinfo_provider4 = "origin.asn.cymru.com";

diff --git a/ui/mtr.h b/ui/mtr.h
index 09df6dbb14ad28b7f33eb88f0e491c92c6b479e1..64cdc4a68fcbd05839e5d0d98e05bbf2b377b833 100644 (file)
--- a/ui/mtr.h
+++ b/ui/mtr.h
@@ -73,6 +73,7 @@ typedef int time_t;
 #define MAXPACKET 65535          /* largest test packet size */
 #define MINPACKET 28            /* 20 bytes IP header and 8 bytes ICMP or UDP */
 #define MAXLABELS 8             /* http://kb.juniper.net/KB2190 (+ 3 just in case) */
+#define MAX_IPINFO_FIELDS 5
 
 /* Stream Control Transmission Protocol is defined in netinet/in.h */
 #ifdef IPPROTO_SCTP
@@ -92,6 +93,8 @@ struct mtr_ctl {
     char *InterfaceAddress;
     char LocalHostname[128];
     int ipinfo_no;
+    int ipinfo_fields[MAX_IPINFO_FIELDS];
+    int ipinfo_field_count;
     int ipinfo_max;
     int cpacketsize;            /* packet size used by ping */
     int bitpattern;             /* packet bit pattern used by ping */

diff --git a/ui/report.c b/ui/report.c
index 11353c02b6025b0de251f841a0a2b4502c9153bc..43bf48c6422f648e45a2e4e912132a6429aa08ce 100644 (file)
--- a/ui/report.c
+++ b/ui/report.c
@@ -171,6 +171,7 @@ void report_close(
     char fmt[16];
     size_t len = 0;
     size_t len_hosts = 33;
+    size_t stat_start = len_hosts;
 #ifdef HAVE_IPINFO
     int len_tmp;
     const size_t iiwidth_len = get_iiwidth_len();
@@ -191,13 +192,11 @@ void report_close(
     }
 #ifdef HAVE_IPINFO
     len_tmp = len_hosts;
-    if (ctl->ipinfo_no >= 0 && iiwidth_len) {
-        ctl->ipinfo_no %= iiwidth_len;
+    if (is_printii(ctl) && iiwidth_len) {
+        len_tmp += get_iiwidth_selected(ctl);
+        stat_start = len_tmp;
         if (ctl->reportwide) {
             len_hosts++;        /* space */
-            len_tmp += get_iiwidth(ctl->ipinfo_no);
-            if (!ctl->ipinfo_no)
-                len_tmp += 2;   /* align header: AS */
         }
     }
     snprintf(fmt, sizeof(fmt), "HOST: %%-%ds", len_tmp);
@@ -205,7 +204,7 @@ void report_close(
     snprintf(fmt, sizeof(fmt), "HOST: %%-%zus", len_hosts);
 #endif
     snprintf(buf, sizeof(buf), fmt, ctl->LocalHostname);
-    len = ctl->reportwide ? strlen(buf) : len_hosts;
+    len = ctl->reportwide ? strlen(buf) : stat_start;
     for (i = 0; i < MAXFLD; i++) {
         j = ctl->fld_index[ctl->fld_active[i]];
         if (j < 0)
@@ -236,7 +235,7 @@ void report_close(
 #ifdef HAVE_IPINFO
         }
 #endif
-        len = ctl->reportwide ? strlen(buf) : len_hosts;
+        len = ctl->reportwide ? strlen(buf) : stat_start;
         for (i = 0; i < MAXFLD; i++) {
             j = ctl->fld_index[ctl->fld_active[i]];
             if (j < 0)
@@ -420,8 +419,8 @@ void json_close(struct mtr_ctl *ctl)
             goto on_error;
 
 #ifdef HAVE_IPINFO
-        if (!ctl->ipinfo_no) {
-            char* fmtinfo = fmt_ipinfo(ctl, addr);
+        if (ipinfo_field_selected(ctl, 0)) {
+            char* fmtinfo = fmt_ipinfo_field(ctl, addr, 0);
             if (fmtinfo != NULL)
                 fmtinfo = trim(fmtinfo, '\0');
 
@@ -573,7 +572,7 @@ void csv_close(
         if (at == net_min(ctl)) {
             printf("Mtr_Version,Start_Time,Status,Host,Hop,Ip,");
 #ifdef HAVE_IPINFO
-            if (!ctl->ipinfo_no) {
+            if (ipinfo_field_selected(ctl, 0)) {
                 printf("Asn,");
             }
 #endif
@@ -590,8 +589,8 @@ void csv_close(
             printf("\n");
         }
 #ifdef HAVE_IPINFO
-        if (!ctl->ipinfo_no) {
-            char *fmtinfo = fmt_ipinfo(ctl, addr);
+        if (ipinfo_field_selected(ctl, 0)) {
+            char *fmtinfo = fmt_ipinfo_field(ctl, addr, 0);
             fmtinfo = trim(fmtinfo, '\0');
             printf("MTR.%s,%lld,%s,%s,%d,%s,%s", PACKAGE_VERSION,
                    (long long) now, "OK", ctl->Hostname, at + 1, name,
@@ -646,8 +645,8 @@ void csv_close(
 
                 if (!found) {
 #ifdef HAVE_IPINFO
-                    if (!ctl->ipinfo_no) {
-                        char *fmtinfo = fmt_ipinfo(ctl, addr2);
+                    if (ipinfo_field_selected(ctl, 0)) {
+                        char *fmtinfo = fmt_ipinfo_field(ctl, addr2, 0);
                         fmtinfo = trim(fmtinfo, '\0');
                         printf("MTR.%s,%lld,%s,%s,%d,%s,%s", PACKAGE_VERSION,
                             (long long) now, "OK", ctl->Hostname, at + 1, name,

diff --git a/ui/select.c b/ui/select.c
index 04fe4a27d6e9c210477bb8d902e2028df46d881e..a5e61b672679de615558ae0cfac4aeafb25172e3 100644 (file)
--- a/ui/select.c
+++ b/ui/select.c
@@ -342,9 +342,16 @@ void select_loop(
                 ctl->ipinfo_no++;
                 if (ctl->ipinfo_no > ctl->ipinfo_max)
                     ctl->ipinfo_no = 0;
+                set_ipinfo_field(ctl, ctl->ipinfo_no);
                 break;
             case ActionAS:
-                ctl->ipinfo_no = ctl->ipinfo_no ? 0 : ctl->ipinfo_max;
+                if (ctl->ipinfo_no == 0 && ctl->ipinfo_max >= 0)
+                    set_ipinfo_field(ctl, ctl->ipinfo_max);
+                else if (ctl->ipinfo_no == 0) {
+                    ctl->ipinfo_no = -1;
+                    ctl->ipinfo_field_count = 0;
+                } else
+                    set_ipinfo_field(ctl, 0);
                 break;
 #endif