curl_get_line: enhance the API

To make sure callers can properly differentiate between errors and know
cleanly when EOF happens. Updated all users and unit test 3200.

Triggered by a remark by ZeroPath

Closes #19140

lib/altsvc.c

@@ -228,14 +228,18 @@ static CURLcode altsvc_load(struct altsvcinfo *asi, const char *file)
 
   fp = curlx_fopen(file, FOPEN_READTEXT);
   if(fp) {
+    bool eof = FALSE;
     struct dynbuf buf;
     curlx_dyn_init(&buf, MAX_ALTSVC_LINE);
-    while(Curl_get_line(&buf, fp)) {
+    do {
-      const char *lineptr = curlx_dyn_ptr(&buf);
+      result = Curl_get_line(&buf, fp, &eof);
-      curlx_str_passblanks(&lineptr);
+      if(!result) {
-      if(curlx_str_single(&lineptr, '#'))
+        const char *lineptr = curlx_dyn_ptr(&buf);
-        altsvc_add(asi, lineptr);
+        curlx_str_passblanks(&lineptr);
-    }
+        if(curlx_str_single(&lineptr, '#'))
+          altsvc_add(asi, lineptr);
+      }
+    } while(!result && !eof);
     curlx_dyn_free(&buf); /* free the line buffer */
     curlx_fclose(fp);
   }

lib/cookie.c

@@ -1205,19 +1205,27 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
     ci->running = FALSE; /* this is not running, this is init */
     if(fp) {
       struct dynbuf buf;
+      bool eof = FALSE;
+      CURLcode result;
       curlx_dyn_init(&buf, MAX_COOKIE_LINE);
-      while(Curl_get_line(&buf, fp)) {
+      do {
-        const char *lineptr = curlx_dyn_ptr(&buf);
+        result = Curl_get_line(&buf, fp, &eof);
-        bool headerline = FALSE;
+        if(!result) {
-        if(checkprefix("Set-Cookie:", lineptr)) {
+          const char *lineptr = curlx_dyn_ptr(&buf);
-          /* This is a cookie line, get it! */
+          bool headerline = FALSE;
-          lineptr += 11;
+          if(checkprefix("Set-Cookie:", lineptr)) {
-          headerline = TRUE;
+            /* This is a cookie line, get it! */
-          curlx_str_passblanks(&lineptr);
+            lineptr += 11;
-        }
+            headerline = TRUE;
+            curlx_str_passblanks(&lineptr);
+          }
 
-        Curl_cookie_add(data, ci, headerline, TRUE, lineptr, NULL, NULL, TRUE);
+          (void)Curl_cookie_add(data, ci, headerline, TRUE, lineptr, NULL,
-      }
+                                NULL, TRUE);
+          /* File reading cookie failures are not propagated back to the
+             caller because there is no way to do that */
+        }
+      } while(!result && !eof);
       curlx_dyn_free(&buf); /* free the line buffer */
 
       /*

lib/curl_get_line.c

@@ -32,63 +32,43 @@
 /* The last #include file should be: */
 #include "memdebug.h"
 
-static int appendnl(struct dynbuf *buf)
+#define appendnl(b)                             \
-{
+  curlx_dyn_addn(buf, "\n", 1)
-  CURLcode result = curlx_dyn_addn(buf, "\n", 1);
-  if(result)
-    /* too long line or out of memory */
-    return 0; /* error */
-  return 1; /* all good */
-}
 
 /*
- * Curl_get_line() makes sure to only return complete whole lines that end
+ * Curl_get_line() returns only complete whole lines that end with newline.
- * newlines.
+ * When 'eof' is set TRUE, the last line has been read.
  */
-int Curl_get_line(struct dynbuf *buf, FILE *input)
+CURLcode Curl_get_line(struct dynbuf *buf, FILE *input, bool *eof)
 {
   CURLcode result;
   char buffer[128];
   curlx_dyn_reset(buf);
   while(1) {
-    char *b = fgets(buffer, sizeof(buffer), input);
     size_t rlen;
+    char *b = fgets(buffer, sizeof(buffer), input);
 
-    if(b) {
+    *eof = feof(input);
-      rlen = strlen(b);
-
-      if(!rlen)
-        break;
 
+    rlen = b ? strlen(b) : 0;
+    if(rlen) {
       result = curlx_dyn_addn(buf, b, rlen);
       if(result)
         /* too long line or out of memory */
-        return 0; /* error */
+        return result;
-
-      else if(b[rlen-1] == '\n')
-        /* end of the line */
-        return 1; /* all good */
-
-      else if(feof(input))
-        /* append a newline */
-        return appendnl(buf);
-    }
-    else {
-      rlen = curlx_dyn_len(buf);
-      if(rlen) {
-        b = curlx_dyn_ptr(buf);
-
-        if(b[rlen-1] != '\n')
-          /* append a newline */
-          return appendnl(buf);
-
-        return 1; /* all good */
-      }
-      else
-        break;
     }
+    /* now check the full line */
+    rlen = curlx_dyn_len(buf);
+    b = curlx_dyn_ptr(buf);
+    if(rlen && (b[rlen-1] == '\n'))
+      /* LF at end of the line */
+      return CURLE_OK; /* all good */
+    if(*eof)
+      /* append a newline */
+      return appendnl(buf);
+    /* otherwise get next line to append */
   }
-  return 0;
+  return CURLE_FAILED_INIT;
 }
 
 #endif /* if not disabled */

lib/curl_get_line.h

@@ -27,6 +27,6 @@
 #include "curlx/dynbuf.h"
 
 /* Curl_get_line() returns complete lines that end with a newline. */
-int Curl_get_line(struct dynbuf *buf, FILE *input);
+CURLcode Curl_get_line(struct dynbuf *buf, FILE *input, bool *eof);
 
 #endif /* HEADER_CURL_GET_LINE_H */

lib/hsts.c

@@ -526,20 +526,24 @@ static CURLcode hsts_load(struct hsts *h, const char *file)
   fp = curlx_fopen(file, FOPEN_READTEXT);
   if(fp) {
     struct dynbuf buf;
+    bool eof = FALSE;
     curlx_dyn_init(&buf, MAX_HSTS_LINE);
-    while(Curl_get_line(&buf, fp)) {
+    do {
-      const char *lineptr = curlx_dyn_ptr(&buf);
+      result = Curl_get_line(&buf, fp, &eof);
-      curlx_str_passblanks(&lineptr);
+      if(!result) {
+        const char *lineptr = curlx_dyn_ptr(&buf);
+        curlx_str_passblanks(&lineptr);
 
-      /*
+        /*
-       * Skip empty or commented lines, since we know the line will have a
+         * Skip empty or commented lines, since we know the line will have a
-       * trailing newline from Curl_get_line we can treat length 1 as empty.
+         * trailing newline from Curl_get_line we can treat length 1 as empty.
-       */
+         */
-      if((*lineptr == '#') || strlen(lineptr) <= 1)
+        if((*lineptr == '#') || strlen(lineptr) <= 1)
-        continue;
+          continue;
 
-      hsts_add(h, lineptr);
+        hsts_add(h, lineptr);
-    }
+      }
+    } while(!result && !eof);
     curlx_dyn_free(&buf); /* free the line buffer */
     curlx_fclose(fp);
   }

lib/netrc.c

@@ -81,22 +81,27 @@ static NETRCcode file2memory(const char *filename, struct dynbuf *filebuf)
   curlx_dyn_init(&linebuf, MAX_NETRC_LINE);
 
   if(file) {
+    CURLcode result = CURLE_OK;
+    bool eof;
     ret = NETRC_OK;
-    while(Curl_get_line(&linebuf, file)) {
+    do {
-      CURLcode result;
+      const char *line;
-      const char *line = curlx_dyn_ptr(&linebuf);
+      result = Curl_get_line(&linebuf, file, &eof);
-      /* skip comments on load */
+      if(!result) {
-      curlx_str_passblanks(&line);
+        line = curlx_dyn_ptr(&linebuf);
-      if(*line == '#')
+        /* skip comments on load */
-        continue;
+        curlx_str_passblanks(&line);
-      result = curlx_dyn_add(filebuf, line);
+        if(*line == '#')
-      if(result) {
+          continue;
-        ret = curl2netrc(result);
+        result = curlx_dyn_add(filebuf, line);
-        goto done;
       }
-    }
+      if(result) {
+        curlx_dyn_free(filebuf);
+        ret = curl2netrc(result);
+        break;
+      }
+    } while(!eof);
   }
-done:
   curlx_dyn_free(&linebuf);
   if(file)
     curlx_fclose(file);

tests/unit/unit3200.c

@@ -76,12 +76,13 @@ static CURLcode test_unit3200(const char *arg)
 #endif
 
   size_t i;
-  int rc = 0;
+  CURLcode result = CURLE_OK;
   for(i = 0; i < CURL_ARRAYSIZE(filecontents); i++) {
     FILE *fp;
     struct dynbuf buf;
     size_t len = 4096;
     char *line;
+    bool eof;
     curlx_dyn_init(&buf, len);
 
     fp = curlx_fopen(arg, "wb");
@@ -95,63 +96,63 @@ static CURLcode test_unit3200(const char *arg)
     curl_mfprintf(stderr, "Test %zd...", i);
     switch(i) {
       case 0:
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE1\n", line),
+        fail_unless(!result && line && !strcmp("LINE1\n", line),
                     "First line failed (1)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE2 NEWLINE\n", line),
+        fail_unless(!result && line && !strcmp("LINE2 NEWLINE\n", line),
                     "Second line failed (1)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
-        abort_unless(!curlx_dyn_len(&buf), "Missed EOF (1)");
+        abort_unless(eof, "Missed EOF (1)");
         break;
       case 1:
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE1\n", line),
+        fail_unless(!result && line && !strcmp("LINE1\n", line),
                     "First line failed (2)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE2 NONEWLINE\n", line),
+        fail_unless(!result && line && !strcmp("LINE2 NONEWLINE\n", line),
                     "Second line failed (2)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
-        abort_unless(!curlx_dyn_len(&buf), "Missed EOF (2)");
+        abort_unless(eof, "Missed EOF (2)");
         break;
       case 2:
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE1\n", line),
+        fail_unless(!result && line && !strcmp("LINE1\n", line),
                     "First line failed (3)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         fail_unless(!curlx_dyn_len(&buf),
                     "Did not detect max read on EOF (3)");
         break;
       case 3:
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE1\n", line),
+        fail_unless(!result && line && !strcmp("LINE1\n", line),
                     "First line failed (4)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         fail_unless(!curlx_dyn_len(&buf),
                     "Did not ignore partial on EOF (4)");
         break;
       case 4:
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE1\n", line),
+        fail_unless(!result && line && !strcmp("LINE1\n", line),
                     "First line failed (5)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         fail_unless(!curlx_dyn_len(&buf),
                     "Did not bail out on too long line");
         break;
       case 5:
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
         line = curlx_dyn_ptr(&buf);
-        fail_unless(rc && line && !strcmp("LINE1\x1aTEST\n", line),
+        fail_unless(!result && line && !strcmp("LINE1\x1aTEST\n", line),
                     "Missed/Misinterpreted ^Z (6)");
-        rc = Curl_get_line(&buf, fp);
+        result = Curl_get_line(&buf, fp, &eof);
-        abort_unless(!curlx_dyn_len(&buf), "Missed EOF (6)");
+        abort_unless(eof, "Missed EOF (6)");
         break;
       default:
         abort_unless(1, "Unknown case");
@@ -161,7 +162,7 @@ static CURLcode test_unit3200(const char *arg)
     curlx_fclose(fp);
     curl_mfprintf(stderr, "OK\n");
   }
-  return (CURLcode)rc;
+  return result;
 
 #endif