handle long input lines when reading from file

This addresses GitHub isse #377.

Before, input lines longer than the static buffer used to read
data would be handled as if each buffer part were one line.
This could result in splitting input target names into multiple
different target names.

Since the static buffer was significantly smaller then the maximum
length of DNS names (just short of 255 one octet characters),
valid long DNS names could not be read as targets via file.

This commit fixes the splitting of long lines, and also increases
the maximum length of target names to 255 one octet characters.

The additional line parsing code is kept similar to the existing
code and attempts to keep all intended and/or useful features to
minimize the risk of regressions.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2049f15dc7b146764b110df47669a35614a277e1..20c462ebc45dd3539b1d6c7c01138b0983820034 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,8 +3,12 @@ Next
 
 ## Bugfixes and other changes
 
-- Fix fallback to SO\_TIMESTAMP if SO\_TIMESTAMPNS is not available (#375, thanks
-  @auerswal)
+- Fix fallback to SO\_TIMESTAMP if SO\_TIMESTAMPNS is not available (#375,
+  thanks @auerswal)
+
+- When reading target names from file or standard input, lines longer
+  than the static buffer are no longer interpreted as more than one line
+  (#378, thanks @auerswal)
 
 ## New features
 

diff --git a/ci/test-issue-377.pl b/ci/test-issue-377.pl
new file mode 100755 (executable)
index 0000000..c20b72c
--- /dev/null
+++ b/ci/test-issue-377.pl
@@ -0,0 +1,328 @@
+#!/usr/bin/perl -w
+
+# regression testing for github issue #377
+# (lines longer than the fixed buffer were split into multiple lines)
+
+use Test::Command tests => 66;
+use File::Temp;
+
+{
+# the issue was noticed with a very long target name (too long for DNS)
+my $tmpfile = File::Temp->new();
+print $tmpfile "a"x300 .".invalid\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# the issue was noticed with a very long target name (too long for DNS)
+# (no newline)
+my $tmpfile = File::Temp->new();
+print $tmpfile "a"x300 .".invalid";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# a too long word can be found in two consecutive parts of the line
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x200 ."a"x300 .".invalid\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# a too long word can be found in two consecutive parts of the line
+# (no newline)
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x200 ."a"x300 .".invalid";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# a too long word starting with the second part of the line (256B buffer)
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x255 ."a"x300 .".invalid\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# a too long word starting with the second part of the line (256B buffer)
+# (no newline)
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x255 ."a"x300 .".invalid";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# first part of line read into buffer may be blank
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x400 ."a"x300 .".invalid\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# first part of line read into buffer may be blank
+# (no newline)
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x400 ."a"x300 .".invalid";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("fping: target name too long\n");
+}
+
+{
+# lines longer than the line buffer shall not be split - 132B buffer
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x100 ."127.0.0.1 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x122 ."127.0.0.2 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x127 ."127.0.0.3 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x131 ."127.0.0.4 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x150 ."127.0.0.5 "." "x(131-9)."host.name.invalid\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive
+127.0.0.2 is alive
+127.0.0.3 is alive
+127.0.0.4 is alive
+127.0.0.5 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# lines longer than the line buffer shall not be split - 132B buffer
+# (last line without newline)
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x100 ."127.0.0.1 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x122 ."127.0.0.2 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x127 ."127.0.0.3 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x131 ."127.0.0.4 "." "x(131-9)."host.name.invalid\n";
+print $tmpfile " "x150 ."127.0.0.5 "." "x(131-9)."host.name.invalid";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive
+127.0.0.2 is alive
+127.0.0.3 is alive
+127.0.0.4 is alive
+127.0.0.5 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# lines longer than the line buffer shall not be split - 256B buffer
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x240 ."127.0.0.1 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x246 ."127.0.0.2 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x251 ."127.0.0.3 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x255 ."127.0.0.4 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x275 ."127.0.0.5 "." "x(255-9)."host.name.invalid\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive
+127.0.0.2 is alive
+127.0.0.3 is alive
+127.0.0.4 is alive
+127.0.0.5 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# lines longer than the line buffer shall not be split - 256B buffer
+# (last line without newline)
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x240 ."127.0.0.1 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x246 ."127.0.0.2 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x251 ."127.0.0.3 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x255 ."127.0.0.4 "." "x(255-9)."host.name.invalid\n";
+print $tmpfile " "x275 ."127.0.0.5 "." "x(255-9)."host.name.invalid";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive
+127.0.0.2 is alive
+127.0.0.3 is alive
+127.0.0.4 is alive
+127.0.0.5 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline shorter than 131 bytes
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(131-10-9) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline with 131 bytes
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(131-9) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline with length between 131 and 255 bytes
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(255-10-9) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline with 255 bytes
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(255-9) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline with word split between two 256 byte buffers
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(255-5) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline with between 255 and 510 bytes
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(255*2-10-9) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# line without newline with 510 bytes
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x(255*2-9) ."127.0.0.1";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(0);
+$cmd->stdout_is_eq("127.0.0.1 is alive\n");
+$cmd->stderr_is_eq("");
+}
+
+{
+# invalid target name with a comment sign inside
+my $tmpfile = File::Temp->new();
+for (my $i = 1; $i < 4; $i++) {
+    print $tmpfile " "x(255-11+$i)."127.0.0.$i#host.name.invalid\n";
+}
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(2);
+$cmd->stdout_is_eq("");
+$cmd->stderr_like(qr{127\.0\.0\.1\#host\.name\.invalid: .*
+127\.0\.0\.2\#host\.name\.invalid: .*
+127\.0\.0\.3\#host\.name\.invalid: .*\n});
+}
+
+{
+# blank lines are ignored
+my $tmpfile = File::Temp->new();
+print $tmpfile " "x100 ."\n";
+print $tmpfile " "x131 ."\n";
+print $tmpfile " "x150 ."\n";
+print $tmpfile " "x255 ."\n";
+print $tmpfile " "x275 ."\n";
+print $tmpfile " "x500 ."\n";
+print $tmpfile " "x510 ."\n";
+print $tmpfile " "x550 ."\n";
+print $tmpfile " "x575;
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("");
+}
+
+{
+# many comment lines, most needing more than one 256B buffer
+my $tmpfile = File::Temp->new();
+for (my $i = 1; $i < 4; $i++) {
+    print $tmpfile " "x(255-11+$i)."# this is a comment\n";
+}
+for (my $i = 1; $i < 8; $i++) {
+    print $tmpfile " "x(255-8+$i)."# x y z\n";
+}
+print $tmpfile "#"." "x300 ."localhost\n";
+print $tmpfile " "x300 ."#"." "x300 ."127.0.0.1\n";
+close($tmpfile);
+
+my $cmd = Test::Command->new(cmd => "cat ".$tmpfile->filename." | fping");
+$cmd->exit_is_num(1);
+$cmd->stdout_is_eq("");
+$cmd->stderr_is_eq("");
+}

diff --git a/doc/fping.pod b/doc/fping.pod
index 2bb9330d9face1845d890d3974dbf1dfd61edaca..52c01c92a498be233c2b61897409a44bc4ead7fd 100644 (file)
--- a/doc/fping.pod
+++ b/doc/fping.pod
@@ -327,6 +327,9 @@ The number of addresses that can be generated using the C<-g>, C<--generate>
 option is limited to 131072 (the number of host addresses in one 111-bit IPv6
 prefix, two addresses more than the host addresses in one 15-bit IPv4 prefix).
 
+The length of target names read from file or standard input is limited to
+255 bytes.
+
 If fping was configured with C<--enable-safe-limits>, the following values are
 not allowed for non-root users:
 

diff --git a/src/fping.c b/src/fping.c
index cc1c339f0210f4dce4e12c3eddd98523af484c61..93bf046bb9c98f89e234038de426a7a2d6c370b7 100644 (file)
--- a/src/fping.c
+++ b/src/fping.c
@@ -140,6 +140,7 @@ extern int h_errno;
 #define MAX_PING_DATA (MAX_IP_PACKET - SIZE_IP_HDR - SIZE_ICMP_HDR)
 
 #define MAX_GENERATE 131072 /* maximum number of hosts that -g can generate */
+#define MAX_TARGET_NAME_LEN 255 /* maximum target name length read from file */
 
 /* sized so as to be like traditional ping */
 #define DEFAULT_PING_DATA_SIZE 56
@@ -1174,8 +1175,10 @@ int main(int argc, char **argv)
     }
     else if (filename) {
         FILE *ping_file;
-        char line[132];
-        char host[132];
+        char line[MAX_TARGET_NAME_LEN + 1];
+        char host[MAX_TARGET_NAME_LEN + 1];
+        char scratch[MAX_TARGET_NAME_LEN + 1];
+        int skip, non_empty;
 
         if (strcmp(filename, "-") == 0)
             ping_file = fdopen(0, "r");
@@ -1185,14 +1188,72 @@ int main(int argc, char **argv)
         if (!ping_file)
             errno_crash_and_burn("fopen");
 
+        /*
+         * Read the first word of every non-comment line, skip everything else.
+         * (Empty and blank lines are ignored.  Lines where the first non-blank
+         * character is a '#' are interpreted as comments and ignored.)
+        */
         while (fgets(line, sizeof(line), ping_file)) {
-            if (sscanf(line, "%s", host) != 1)
-                continue;
+            skip = non_empty = 0;
 
-            if ((!*host) || (host[0] == '#')) /* magic to avoid comments */
+            /* skip over a prefix of the line where sscanf finds nothing */
+            if ((sscanf(line, "%s", host) != 1) || (!*host)) {
                 continue;
+            }
+
+            /* the first word of the line can indicate a comment line */
+            if (host[0] == '#') {
+                skip = 1; /* skip remainder of line */
+            } else {
+                non_empty = 1; /* we have something to add as a target name */
+                /*
+                 * We have found the start of a word.
+                 * This part of the line may contain all of the first word.
+                 */
+                if (!strchr(line, '\n') && (strlen(line) == sizeof(line) - 1)) {
+                    char discard1[MAX_TARGET_NAME_LEN + 1];
+                    char discard2[MAX_TARGET_NAME_LEN + 1];
+                    if (sscanf(line, "%s%s", discard1, discard2) == 2) {
+                        skip = 1; /* a second word starts in this part */
+                    }
+                    if (isspace(line[sizeof(line) - 2])) {
+                        skip = 1; /* the first word ends in this part */
+                    }
+                }
+            }
+            /* read remainder of this input line */
+            while (!strchr(line, '\n') && fgets(line, sizeof(line), ping_file)) {
+                if (skip) {
+                    continue; /* skip rest of data in this input line */
+                }
+                if (isspace(line[0])) {
+                    skip = 1; /* first word ended in previous part */
+                    continue;
+                }
+                if ((sscanf(line, "%s", scratch) != 1) || (!*scratch)) {
+                    skip = 1; /* empty or blank part of line, skip the rest */
+                    continue;
+                }
+                if (sizeof(host) - strlen(host) < strlen(scratch) + 1) {
+                    fprintf(stderr, "%s: target name too long\n", prog);
+                    exit(1);
+                }
+                /* append remainder of word started in previous line part */
+                strncat(host, scratch, sizeof(host) - strlen(host) - 1);
+                /*
+                 * Since the "host" buffer is the same size as the "line"
+                 * buffer, a target name that fits into the "host" buffer
+                 * cannot use more than two consecutive line parts.
+                 * A target name that uses two consecutive line parts
+                 * and fits into the "host" buffer must end before the
+                 * end of the second "line" buffer.  Thus the rest of
+                 * the line can be skipped.
+                 */
+                skip = 1;
+            }
 
-            add_name(host);
+            if (non_empty)
+                add_name(host);
         }
 
         fclose(ping_file);