From 142ac257b3242459b284020c59f1902b9687a954 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 6 Feb 2024 10:15:52 +0100 Subject: [PATCH] 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 Conflict:context adapt Reference:https://github.com/curl/curl/commit/142ac257b3242459b284020c59f1902b9687a954 --- lib/altsvc.c | 18 +++------- lib/cookie.c | 29 ++++------------ lib/curl_get_line.c | 55 +++++++++++++---------------- lib/curl_get_line.h | 7 ++-- lib/hsts.c | 17 +++------ lib/netrc.c | 10 ++++-- tests/unit/unit3200.c | 80 ++++++++++++++++++++++++------------------- 7 files changed, 96 insertions(+), 120 deletions(-) diff --git a/lib/altsvc.c b/lib/altsvc.c index e9f62bf0e..c12d7bda1 100644 --- a/lib/altsvc.c +++ b/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; } /* diff --git a/lib/cookie.c b/lib/cookie.c index dc319b611..d10dd572b 100644 --- a/lib/cookie.c +++ b/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 */ } /* diff --git a/lib/curl_get_line.c b/lib/curl_get_line.c index 686abe751..100207331 100644 --- a/lib/curl_get_line.c +++ b/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 */ diff --git a/lib/curl_get_line.h b/lib/curl_get_line.h index 0ff32c5c2..7907cde88 100644 --- a/lib/curl_get_line.h +++ b/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 */ diff --git a/lib/hsts.c b/lib/hsts.c index 8725a35c1..607755e6b 100644 --- a/lib/hsts.c +++ b/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; } /* diff --git a/lib/netrc.c b/lib/netrc.c index 038c6dca6..cd2a2844e 100644 --- a/lib/netrc.c +++ b/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) { diff --git a/tests/unit/unit3200.c b/tests/unit/unit3200.c index 0544bcc93..6f508ce07 100644 --- a/tests/unit/unit3200.c +++ b/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 #else -- 2.33.0