tool_paramhlp: use feof(3) to identify EOF correctly when using fread(3)

This loop was using the number of bytes read from the file as condition
to keep reading.

From Linux's fread(3) man page:
> On success, fread() and fwrite() return the number of items read or
> written. This number equals the number of bytes transferred only when
> size is 1. If an error occurs, or the end of the file is reached, the
> return value is a short item count (or zero).
>
> The file position indicator for the stream is advanced by the number
> of bytes successfully read or written.
>
> fread() does not distinguish between end-of-file and error, and
> callers must use feof(3) and ferror(3) to determine which occurred.

This means that nread!=0 doesn't make much sense as an end condition for
the loop: nread==0 doesn't necessarily mean that EOF has been reached or
an error has occured (but that is usually the case) and nread!=0 doesn't
necessarily mean that EOF has not been reached or that no read errors
have occured. feof(3) and ferror(3) should be uses when using fread(3).

Currently curl has to performs an extra fread(3) call to get a return
value equal to 0 to stop looping.

This usually "works" (even though nread==0 shouldn't be interpreted as
EOF) if stdin is a pipe because EOF usually marks the "real" end of the
stream, so the extra fread(3) call will return immediately and the extra
read syscall won't be noticeable:

    bash-5.1$ strace -e read curl -s -F file=@- 0x0.st <<< a 2>&1 |
    > tail -n 5
    read(0, "a\n", 4096)                    = 2
    read(0, "", 4096)                       = 0
    read(0, "", 4096)                       = 0
    http://0x0.st/oRs.txt
    +++ exited with 0 +++
    bash-5.1$

But this doesn't work if curl is reading from stdin, stdin is a
terminal, and the EOF is being emulated using a shell with ^D. Two
consecutive ^D will be required in this case to actually make curl stop
reading:

    bash-5.1$ curl -F file=@- 0x0.st
    a
    ^D^D
    http://0x0.st/oRs.txt
    bash-5.1$

A possible workaround to this issue is to use a program that handles EOF
correctly to indirectly send data to curl's stdin:

    bash-5.1$ cat - | curl -F file=@- 0x0.st
    a
    ^D
    http://0x0.st/oRs.txt
    bash-5.1$

This patch makes curl handle EOF properly when using fread(3) in
file2memory() so that the workaround is not necessary.

Since curl was previously ignoring read errors caused by this fread(3),
ferror(3) is also used in the condition of the loop: read errors and EOF
will have the same meaning; this is done to somewhat preserve the old
behaviour instead of making the command fail when a read error occurs.

Closes #8701

diff --git a/src/tool_formparse.c b/src/tool_formparse.c
index 844b18d154e5a593768e68ca3f1f27a477901771..5ea14f76417c7730ad2b69440e49a499c0351c71 100644 (file)
--- a/src/tool_formparse.c
+++ b/src/tool_formparse.c
@@ -125,21 +125,20 @@ static struct tool_mime *tool_mime_new_filedata(struct tool_mime *parent,
     else {  /* Not suitable for direct use, buffer stdin data. */
       size_t stdinsize = 0;
 
-      if(file2memory(&data, &stdinsize, stdin) != PARAM_OK) {
-        /* Out of memory. */
+      switch(file2memory(&data, &stdinsize, stdin)) {
+      case PARAM_NO_MEM:
         return m;
-      }
-
-      if(ferror(stdin)) {
+      case PARAM_READ_ERROR:
         result = CURLE_READ_ERROR;
-        Curl_safefree(data);
-        data = NULL;
-      }
-      else if(!stdinsize) {
-        /* Zero-length data has been freed. Re-create it. */
-        data = strdup("");
-        if(!data)
-          return m;
+        break;
+      default:
+        if(!stdinsize) {
+          /* Zero-length data has been freed. Re-create it. */
+          data = strdup("");
+          if(!data)
+            return m;
+        }
+        break;
       }
       size = curlx_uztoso(stdinsize);
       origin = 0;

diff --git a/src/tool_getparam.h b/src/tool_getparam.h
index 599a0ac8858bcf33b3618928de3ff137c2516a0e..4f578d513961b8a2c8b5b3c7f6ed94a41d93dfcc 100644 (file)
--- a/src/tool_getparam.h
+++ b/src/tool_getparam.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2021, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2022, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -45,6 +45,7 @@ typedef enum {
   PARAM_NO_NOT_BOOLEAN,
   PARAM_CONTDISP_SHOW_HEADER, /* --include and --remote-header-name */
   PARAM_CONTDISP_RESUME_FROM, /* --continue-at and --remote-header-name */
+  PARAM_READ_ERROR,
   PARAM_LAST
 } ParameterError;
 

diff --git a/src/tool_helpers.c b/src/tool_helpers.c
index d47a8d244bcb57bdcbca0e050b56221e2cf901d3..f604def68d60befa73480af0edd67e2b6f973559 100644 (file)
--- a/src/tool_helpers.c
+++ b/src/tool_helpers.c
@@ -72,6 +72,8 @@ const char *param2text(int res)
     return "showing headers and --remote-header-name cannot be combined";
   case PARAM_CONTDISP_RESUME_FROM:
     return "--continue-at and --remote-header-name cannot be combined";
+  case PARAM_READ_ERROR:
+    return "error encountered when reading a file";
   default:
     return "unknown error";
   }

diff --git a/src/tool_operate.c b/src/tool_operate.c
index bf6aff6eea99952f605c4b96cd6be407ed17a7bf..cb587e0a64c41cd3baff872f5dd78009d1a14e5d 100644 (file)
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -2621,6 +2621,8 @@ CURLcode operate(struct GlobalConfig *global, int argc, argv_item_t argv[])
         tool_list_engines();
       else if(res == PARAM_LIBCURL_UNSUPPORTED_PROTOCOL)
         result = CURLE_UNSUPPORTED_PROTOCOL;
+      else if(res == PARAM_READ_ERROR)
+        result = CURLE_READ_ERROR;
       else
         result = CURLE_FAILED_INIT;
     }

diff --git a/src/tool_paramhlp.c b/src/tool_paramhlp.c
index 273805ef0d1769b51de79c4f18e26f4ec086cd37..5c731980e00978316d6748c103d4663d7c663e97 100644 (file)
--- a/src/tool_paramhlp.c
+++ b/src/tool_paramhlp.c
@@ -94,10 +94,16 @@ ParameterError file2memory(char **bufp, size_t *size, FILE *file)
     do {
       char buffer[4096];
       nread = fread(buffer, 1, sizeof(buffer), file);
+      if(ferror(file)) {
+        curlx_dyn_free(&dyn);
+        *size = 0;
+        *bufp = NULL;
+        return PARAM_READ_ERROR;
+      }
       if(nread)
         if(curlx_dyn_addn(&dyn, buffer, nread))
           return PARAM_NO_MEM;
-    } while(nread);
+    } while(!feof(file));
     *size = curlx_dyn_len(&dyn);
     *bufp = curlx_dyn_ptr(&dyn);
   }