Browse Source

lib: convert Curl_get_line to use dynbuf

Create the line in a dynbuf. Aborts the reading of the file on
errors. Avoids having to always allocate maximum amount from the
start. Avoids direct malloc.

Closes #12846
Daniel Stenberg 2 months ago
parent
commit
142ac257b3
7 changed files with 96 additions and 120 deletions
  1. 5 13
      lib/altsvc.c
  2. 7 22
      lib/cookie.c
  3. 23 32
      lib/curl_get_line.c
  4. 4 3
      lib/curl_get_line.h
  5. 5 12
      lib/hsts.c
  6. 7 3
      lib/netrc.c
  7. 45 35
      tests/unit/unit3200.c

+ 5 - 13
lib/altsvc.c

@@ -209,7 +209,6 @@ static CURLcode altsvc_add(struct altsvcinfo *asi, char *line)
 static CURLcode altsvc_load(struct altsvcinfo *asi, const char *file)
 {
   CURLcode result = CURLE_OK;
-  char *line = NULL;
   FILE *fp;
 
   /* we need a private copy of the file name so that the altsvc cache file
@@ -221,11 +220,10 @@ static CURLcode altsvc_load(struct altsvcinfo *asi, const char *file)
 
   fp = fopen(file, FOPEN_READTEXT);
   if(fp) {
-    line = malloc(MAX_ALTSVC_LINE);
-    if(!line)
-      goto fail;
-    while(Curl_get_line(line, MAX_ALTSVC_LINE, fp)) {
-      char *lineptr = line;
+    struct dynbuf buf;
+    Curl_dyn_init(&buf, MAX_ALTSVC_LINE);
+    while(Curl_get_line(&buf, fp)) {
+      char *lineptr = Curl_dyn_ptr(&buf);
       while(*lineptr && ISBLANK(*lineptr))
         lineptr++;
       if(*lineptr == '#')
@@ -234,16 +232,10 @@ static CURLcode altsvc_load(struct altsvcinfo *asi, const char *file)
 
       altsvc_add(asi, lineptr);
     }
-    free(line); /* free the line buffer */
+    Curl_dyn_free(&buf); /* free the line buffer */
     fclose(fp);
   }
   return result;
-
-fail:
-  Curl_safefree(asi->filename);
-  free(line);
-  fclose(fp);
-  return CURLE_OUT_OF_MEMORY;
 }
 
 /*

+ 7 - 22
lib/cookie.c

@@ -1205,7 +1205,6 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
                                     bool newsession)
 {
   struct CookieInfo *c;
-  char *line = NULL;
   FILE *handle = NULL;
 
   if(!inc) {
@@ -1241,16 +1240,14 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
 
     c->running = FALSE; /* this is not running, this is init */
     if(fp) {
-
-      line = malloc(MAX_COOKIE_LINE);
-      if(!line)
-        goto fail;
-      while(Curl_get_line(line, MAX_COOKIE_LINE, fp)) {
-        char *lineptr = line;
+      struct dynbuf buf;
+      Curl_dyn_init(&buf, MAX_COOKIE_LINE);
+      while(Curl_get_line(&buf, fp)) {
+        char *lineptr = Curl_dyn_ptr(&buf);
         bool headerline = FALSE;
-        if(checkprefix("Set-Cookie:", line)) {
+        if(checkprefix("Set-Cookie:", lineptr)) {
           /* This is a cookie line, get it! */
-          lineptr = &line[11];
+          lineptr += 11;
           headerline = TRUE;
           while(*lineptr && ISBLANK(*lineptr))
             lineptr++;
@@ -1258,7 +1255,7 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
 
         Curl_cookie_add(data, c, headerline, TRUE, lineptr, NULL, NULL, TRUE);
       }
-      free(line); /* free the line buffer */
+      Curl_dyn_free(&buf); /* free the line buffer */
 
       /*
        * Remove expired cookies from the hash. We must make sure to run this
@@ -1274,18 +1271,6 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
   c->running = TRUE;          /* now, we're running */
 
   return c;
-
-fail:
-  free(line);
-  /*
-   * Only clean up if we allocated it here, as the original could still be in
-   * use by a share handle.
-   */
-  if(!inc)
-    Curl_cookie_cleanup(c);
-  if(handle)
-    fclose(handle);
-  return NULL; /* out of memory */
 }
 
 /*

+ 23 - 32
lib/curl_get_line.c

@@ -33,14 +33,16 @@
 #include "memdebug.h"
 
 /*
- * Curl_get_line() makes sure to only return complete whole lines that fit in
- * 'len' bytes and end with a newline.
+ * Curl_get_line() makes sure to only return complete whole lines that end
+ * newlines.
  */
-char *Curl_get_line(char *buf, int len, FILE *input)
+int Curl_get_line(struct dynbuf *buf, FILE *input)
 {
-  bool partial = FALSE;
+  CURLcode result;
+  char buffer[128];
+  Curl_dyn_reset(buf);
   while(1) {
-    char *b = fgets(buf, len, input);
+    char *b = fgets(buffer, sizeof(buffer), input);
 
     if(b) {
       size_t rlen = strlen(b);
@@ -48,39 +50,28 @@ char *Curl_get_line(char *buf, int len, FILE *input)
       if(!rlen)
         break;
 
-      if(b[rlen-1] == '\n') {
-        /* b is \n terminated */
-        if(partial) {
-          partial = FALSE;
-          continue;
-        }
-        return b;
-      }
-      else if(feof(input)) {
-        if(partial)
-          /* Line is already too large to return, ignore rest */
-          break;
+      result = Curl_dyn_addn(buf, b, rlen);
+      if(result)
+        /* too long line or out of memory */
+        return 0; /* error */
 
-        if(rlen + 1 < (size_t) len) {
-          /* b is EOF terminated, insert missing \n */
-          b[rlen] = '\n';
-          b[rlen + 1] = '\0';
-          return b;
-        }
-        else
-          /* Maximum buffersize reached + EOF
-           * This line is impossible to add a \n to so we'll ignore it
-           */
-          break;
+      else if(b[rlen-1] == '\n')
+        /* end of the line */
+        return 1; /* all good */
+
+      else if(feof(input)) {
+        /* append a newline */
+        result = Curl_dyn_addn(buf, "\n", 1);
+        if(result)
+          /* too long line or out of memory */
+          return 0; /* error */
+        return 1; /* all good */
       }
-      else
-        /* Maximum buffersize reached */
-        partial = TRUE;
     }
     else
       break;
   }
-  return NULL;
+  return 0;
 }
 
 #endif /* if not disabled */

+ 4 - 3
lib/curl_get_line.h

@@ -24,8 +24,9 @@
  *
  ***************************************************************************/
 
-/* get_line() makes sure to only return complete whole lines that fit in 'len'
- * bytes and end with a newline. */
-char *Curl_get_line(char *buf, int len, FILE *input);
+#include "dynbuf.h"
+
+/* Curl_get_line() returns complete lines that end with a newline. */
+int Curl_get_line(struct dynbuf *buf, FILE *input);
 
 #endif /* HEADER_CURL_GET_LINE_H */

+ 5 - 12
lib/hsts.c

@@ -511,7 +511,6 @@ static CURLcode hsts_pull(struct Curl_easy *data, struct hsts *h)
 static CURLcode hsts_load(struct hsts *h, const char *file)
 {
   CURLcode result = CURLE_OK;
-  char *line = NULL;
   FILE *fp;
 
   /* we need a private copy of the file name so that the hsts cache file
@@ -523,11 +522,10 @@ static CURLcode hsts_load(struct hsts *h, const char *file)
 
   fp = fopen(file, FOPEN_READTEXT);
   if(fp) {
-    line = malloc(MAX_HSTS_LINE);
-    if(!line)
-      goto fail;
-    while(Curl_get_line(line, MAX_HSTS_LINE, fp)) {
-      char *lineptr = line;
+    struct dynbuf buf;
+    Curl_dyn_init(&buf, MAX_HSTS_LINE);
+    while(Curl_get_line(&buf, fp)) {
+      char *lineptr = Curl_dyn_ptr(&buf);
       while(*lineptr && ISBLANK(*lineptr))
         lineptr++;
       if(*lineptr == '#')
@@ -536,15 +534,10 @@ static CURLcode hsts_load(struct hsts *h, const char *file)
 
       hsts_add(h, lineptr);
     }
-    free(line); /* free the line buffer */
+    Curl_dyn_free(&buf); /* free the line buffer */
     fclose(fp);
   }
   return result;
-
-fail:
-  Curl_safefree(h->filename);
-  fclose(fp);
-  return CURLE_OUT_OF_MEMORY;
 }
 
 /*

+ 7 - 3
lib/netrc.c

@@ -53,6 +53,8 @@ enum host_lookup_state {
 #define NETRC_FAILED -1
 #define NETRC_SUCCESS 0
 
+#define MAX_NETRC_LINE 4096
+
 /*
  * Returns zero on success.
  */
@@ -80,13 +82,14 @@ static int parsenetrc(const char *host,
   file = fopen(netrcfile, FOPEN_READTEXT);
   if(file) {
     bool done = FALSE;
-    char netrcbuffer[4096];
-    int  netrcbuffsize = (int)sizeof(netrcbuffer);
+    struct dynbuf buf;
+    Curl_dyn_init(&buf, MAX_NETRC_LINE);
 
-    while(!done && Curl_get_line(netrcbuffer, netrcbuffsize, file)) {
+    while(!done && Curl_get_line(&buf, file)) {
       char *tok;
       char *tok_end;
       bool quoted;
+      char *netrcbuffer = Curl_dyn_ptr(&buf);
       if(state == MACDEF) {
         if((netrcbuffer[0] == '\n') || (netrcbuffer[0] == '\r'))
           state = NOTHING;
@@ -245,6 +248,7 @@ static int parsenetrc(const char *host,
     } /* while Curl_get_line() */
 
 out:
+    Curl_dyn_free(&buf);
     if(!retcode) {
       /* success */
       if(login_alloc) {

+ 45 - 35
tests/unit/unit3200.c

@@ -69,7 +69,7 @@ static const char *filecontents[] = {
   "LINE1\n"
   C4096 "SOME EXTRA TEXT",
 
-  /* First and third line should be read */
+  /* Only first should be read */
   "LINE1\n"
   C4096 "SOME EXTRA TEXT\n"
   "LINE3\n",
@@ -84,11 +84,13 @@ static const char *filecontents[] = {
 
 UNITTEST_START
   size_t i;
+  int rc = 0;
   for(i = 0; i < NUMTESTS; i++) {
     FILE *fp;
-    char buf[4096];
+    struct dynbuf buf;
     int len = 4096;
     char *line;
+    Curl_dyn_init(&buf, len);
 
     fp = fopen(arg, "wb");
     abort_unless(fp != NULL, "Cannot open testfile");
@@ -101,65 +103,73 @@ UNITTEST_START
     fprintf(stderr, "Test %zd...", i);
     switch(i) {
       case 0:
-        line = Curl_get_line(buf, len, fp);
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE1\n", line),
-          "First line failed (1)");
-        line = Curl_get_line(buf, len, fp);
+                    "First line failed (1)");
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE2 NEWLINE\n", line),
-          "Second line failed (1)");
-        line = Curl_get_line(buf, len, fp);
-        abort_unless(line == NULL, "Missed EOF (1)");
+                    "Second line failed (1)");
+        rc = Curl_get_line(&buf, fp);
+        abort_unless(!Curl_dyn_len(&buf), "Missed EOF (1)");
         break;
       case 1:
-        line = Curl_get_line(buf, len, fp);
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE1\n", line),
-          "First line failed (2)");
-        line = Curl_get_line(buf, len, fp);
+                    "First line failed (2)");
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE2 NONEWLINE\n", line),
-          "Second line failed (2)");
-        line = Curl_get_line(buf, len, fp);
-        abort_unless(line == NULL, "Missed EOF (2)");
+                    "Second line failed (2)");
+        rc = Curl_get_line(&buf, fp);
+        abort_unless(!Curl_dyn_len(&buf), "Missed EOF (2)");
         break;
       case 2:
-        line = Curl_get_line(buf, len, fp);
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE1\n", line),
-          "First line failed (3)");
-        line = Curl_get_line(buf, len, fp);
-        fail_unless(line == NULL,
-          "Did not detect max read on EOF (3)");
+                    "First line failed (3)");
+        rc = Curl_get_line(&buf, fp);
+        fail_unless(!Curl_dyn_len(&buf),
+                    "Did not detect max read on EOF (3)");
         break;
       case 3:
-        line = Curl_get_line(buf, len, fp);
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE1\n", line),
-          "First line failed (4)");
-        line = Curl_get_line(buf, len, fp);
-        fail_unless(line == NULL,
-          "Did not ignore partial on EOF (4)");
+                    "First line failed (4)");
+        rc = Curl_get_line(&buf, fp);
+        fail_unless(!Curl_dyn_len(&buf),
+                    "Did not ignore partial on EOF (4)");
         break;
       case 4:
-        line = Curl_get_line(buf, len, fp);
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE1\n", line),
-          "First line failed (5)");
-        line = Curl_get_line(buf, len, fp);
-        fail_unless(line && !strcmp("LINE3\n", line),
-          "Third line failed (5)");
-        line = Curl_get_line(buf, len, fp);
-        abort_unless(line == NULL, "Missed EOF (5)");
+                    "First line failed (5)");
+        rc = Curl_get_line(&buf, fp);
+        fail_unless(!Curl_dyn_len(&buf),
+                    "Did not bail out on too long line");
         break;
       case 5:
-        line = Curl_get_line(buf, len, fp);
+        rc = Curl_get_line(&buf, fp);
+        line = Curl_dyn_ptr(&buf);
         fail_unless(line && !strcmp("LINE1\x1aTEST\n", line),
-          "Missed/Misinterpreted ^Z (6)");
-        line = Curl_get_line(buf, len, fp);
-        abort_unless(line == NULL, "Missed EOF (6)");
+                    "Missed/Misinterpreted ^Z (6)");
+        rc = Curl_get_line(&buf, fp);
+        abort_unless(!Curl_dyn_len(&buf), "Missed EOF (6)");
         break;
       default:
         abort_unless(1, "Unknown case");
         break;
     }
+    Curl_dyn_free(&buf);
     fclose(fp);
     fprintf(stderr, "OK\n");
   }
+  return rc;
 UNITTEST_STOP
 
 #ifdef __GNUC__