[bug #62131] [preconv] should test for non-seekable stream input

[G. Branden Robinson]

URL:
  <https://savannah.gnu.org/bugs/?62131>

                 Summary: [preconv] should test for non-seekable stream input
                 Project: GNU troff
            Submitted by: gbranden
            Submitted on: Wed 02 Mar 2022 07:30:34 PM UTC
                Category: Preprocessor preconv
                Severity: 3 - Normal
              Item Group: Incorrect behaviour
                  Status: In Progress
                 Privacy: Public
             Assigned to: gbranden
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None

    _______________________________________________________

Details:

preconv(1) should test for non-seekable stream input instead of assuming a
biconditional between non-seekability and an operand of "-" (a special token
indicating the standard input stream).

This issue is spawned from bug #55334.

Here's Ingo's original comment:


I would like to raise a minor concern, maybe you can tweak your solution. 
Comparing the filename to "-" seems fragile.  There may be other ways to get a
non-seekable file descriptor, for example someone passing "/dev/stdin" on the
command line or someone passing the name of a file created with mkfifo(1).

Instead of strcmp(filename, "-") in do_file(), i suggest doing something like
the following inside detect_file_encoding():

-rewind(fp);
+if (fseek(fp, 0L, SEEK_SET) == -1)
+    goto end;

That seems like the canonical way to test for the feature actually needed,
which feels much more robust than some crude guesswork based on tangentially
related string data.  Testing at the place where the feature is really needed
(and where other error handling is already present) also seems easier to
understand than having an additional check one level up in the call stack.

What do you think?


Here's my pending patch, still in test.  It's the same method Ingo proposed.


diff --git a/src/preproc/preconv/preconv.cpp
b/src/preproc/preconv/preconv.cpp
index b5b5a8cd2..581333658 100644
--- a/src/preproc/preconv/preconv.cpp
+++ b/src/preproc/preconv/preconv.cpp
@@ -1070,7 +1070,8 @@ end:
 }
 
 // ---------------------------------------------------------
-// Handle an input file.  If filename is '-' handle stdin.
+// Handle an input file.  If `filename` is "-", read the
+// standard input stream.
 //
 // Return 1 on success, 0 otherwise.
 // ---------------------------------------------------------
@@ -1079,21 +1080,28 @@ do_file(const char *filename)
 {
   FILE *fp;
   string BOM, data;
+  bool is_seekable = false;
+  const char *reported_filename;
 
-  if (strcmp(filename, "-")) {
-    if (debug_flag)
-      fprintf(stderr, "file '%s':\n", filename);
+  if (strcmp(filename, "-") == 0) {
+    fp = stdin;
+    reported_filename = "<standard input>";
+  }
+  else
     fp = fopen(filename, FOPEN_RB);
-    if (!fp) {
-      error("can't open '%1': %2", filename, strerror(errno));
-      return 0;
-    }
+  if (!fp) {
+    error("can't open '%1': %2", reported_filename, strerror(errno));
+    return 0;
+  }
+  if (debug_flag)
+    fprintf(stderr, "processing '%s'\n", reported_filename);
+  if (fseek(fp, 0L, SEEK_SET) == 0) {
+    is_seekable = true;
   }
   else {
+    SET_BINARY(fileno(fp));
     if (debug_flag)
-      fprintf(stderr, "standard input:\n");
-    SET_BINARY(fileno(stdin));
-    fp = stdin;
+      fprintf(stderr, " stream is not seekable: %s\n", strerror(errno));
   }
   const char *BOM_encoding = get_BOM(fp, BOM, data);
   // Determine the encoding.
@@ -1121,7 +1129,7 @@ do_file(const char *filename)
     if (!file_encoding) {
       if (debug_flag)
        fprintf(stderr, "  no coding tag\n");
-      if (strcmp(filename, "-"))
+      if (is_seekable)
          file_encoding = detect_file_encoding(fp);
       if (!file_encoding) {
         if (debug_flag)
@@ -1286,4 +1294,8 @@ main(int argc, char **argv)
   return nbad != 0;
 }
 
-/* end of preconv.cpp */
+// Local Variables:
+// fill-column: 72
+// mode: C++
+// End:
+// vim: set cindent noexpandtab shiftwidth=2 textwidth=72:





    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?62131>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/