Date: Mon, 29 Jun 2026 15:54:21 +0000 From: Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav <des@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: fde60e3a4e92 - main - Revert "libfetch: Add read buffering" Message-ID: <6a42952d.3986f.2afcdae9@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=fde60e3a4e922e7931c39f10105e3fbe33f54534 commit fde60e3a4e922e7931c39f10105e3fbe33f54534 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2026-06-29 15:53:55 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2026-06-29 15:53:55 +0000 Revert "libfetch: Add read buffering" This reverts commit afb60498e0c9836a6e3a7efda8a51710e208a608. --- lib/libfetch/common.c | 109 ++++++++++---------------------------------------- lib/libfetch/common.h | 4 +- lib/libfetch/ftp.c | 11 +++-- lib/libfetch/http.c | 61 +++++++++++++++++----------- 4 files changed, 67 insertions(+), 118 deletions(-) diff --git a/lib/libfetch/common.c b/lib/libfetch/common.c index c1888a1518a0..c5fb45f70da2 100644 --- a/lib/libfetch/common.c +++ b/lib/libfetch/common.c @@ -1403,112 +1403,47 @@ fetch_read(conn_t *conn, void *buf, size_t len) */ #define MIN_BUF_SIZE 1024 -ssize_t +int fetch_getln(conn_t *conn) { char *tmp; size_t tmpsize; - ssize_t rlen; + ssize_t len; + char c; - /* allocate initial buffer */ if (conn->buf == NULL) { - if ((conn->buf = malloc(MIN_BUF_SIZE)) == NULL) + if ((conn->buf = malloc(MIN_BUF_SIZE)) == NULL) { + errno = ENOMEM; return (-1); - conn->bufsize = MIN_BUF_SIZE; - conn->buflen = 0; - conn->pos = 0; - } - - /* move residual data up */ - if (conn->buflen > 0) { - if (conn->pos < conn->buflen) { - memmove(conn->buf, conn->buf + conn->pos, - conn->buflen - conn->pos); } - conn->buflen -= conn->pos; - conn->pos -= conn->pos; + conn->bufsize = MIN_BUF_SIZE; } - /* do we have a complete line? */ - while (conn->pos < conn->buflen) - if (conn->buf[conn->pos++] == '\n') - goto found; + conn->buf[0] = '\0'; + conn->buflen = 0; - for (;;) { - /* read as much as we can right now */ - rlen = fetch_read(conn, conn->buf + conn->buflen, - conn->bufsize - conn->buflen - 1); - /* error */ - if (rlen < 0) + do { + len = fetch_read(conn, &c, 1); + if (len == -1) return (-1); - /* advance and terminate */ - conn->buflen += rlen; - conn->buf[conn->buflen] = '\0'; - /* connection closed */ - if (rlen == 0) + if (len == 0) break; - /* look for a newline */ - while (conn->pos < conn->buflen) - if (conn->buf[conn->pos++] == '\n') - goto found; - /* do we need a bigger buffer? */ - if (conn->buflen > conn->bufsize / 2) { + conn->buf[conn->buflen++] = c; + if (conn->buflen == conn->bufsize) { tmp = conn->buf; - tmpsize = conn->bufsize * 2; - if ((tmp = realloc(tmp, tmpsize)) == NULL) + tmpsize = conn->bufsize * 2 + 1; + if ((tmp = realloc(tmp, tmpsize)) == NULL) { + errno = ENOMEM; return (-1); + } conn->buf = tmp; conn->bufsize = tmpsize; } - } - /* connection closed, return what's left */ - conn->pos = conn->buflen; -found: - rlen = conn->pos; - if (rlen > 0 && conn->buf[rlen - 1] == '\n') - conn->buf[--rlen] = '\0'; - if (rlen > 0 && conn->buf[rlen - 1] == '\r') - conn->buf[--rlen] = '\0'; - DEBUGF("<<< %.*s\n", (int)rlen, conn->buf); - return (rlen); -} + } while (c != '\n'); - -/* - * Read from a connection, taking previously buffered data into account. - */ -ssize_t -fetch_bufread(conn_t *conn, void *buf, size_t len) -{ - ssize_t rlen; - - /* avoid overflow */ - if (len > SSIZE_MAX) - len = SSIZE_MAX; - - /* allocate initial buffer */ - if (conn->buf == NULL) { - conn->bufsize = MIN_BUF_SIZE; - while (conn->bufsize < len) - conn->bufsize *= 2; - if ((conn->buf = malloc(conn->bufsize)) == NULL) - return (-1); - conn->buflen = 0; - conn->pos = 0; - } - - /* return residual data first */ - if (conn->buflen > conn->pos) { - if (len > conn->buflen - conn->pos) - rlen = conn->buflen - conn->pos; - else - rlen = len; - memcpy(buf, conn->buf + conn->pos, rlen); - conn->pos += rlen; - return (rlen); - } - - return (fetch_read(conn, buf, len)); + conn->buf[conn->buflen] = '\0'; + DEBUGF("<<< %s", conn->buf); + return (0); } diff --git a/lib/libfetch/common.h b/lib/libfetch/common.h index 5ad9bde513a0..479c5e03c0d1 100644 --- a/lib/libfetch/common.h +++ b/lib/libfetch/common.h @@ -51,7 +51,6 @@ struct fetchconn { char *buf; /* buffer */ size_t bufsize; /* buffer size */ size_t buflen; /* length of buffer contents */ - size_t pos; /* current position in buffer */ int err; /* last protocol reply code */ #ifdef WITH_SSL SSL *ssl; /* SSL handle */ @@ -121,8 +120,7 @@ int fetch_ssl_cb_verify_crt(int, X509_STORE_CTX*); #endif int fetch_ssl(conn_t *, const struct url *, int); ssize_t fetch_read(conn_t *, void *, size_t); -ssize_t fetch_getln(conn_t *); -ssize_t fetch_bufread(conn_t *, void *, size_t); +int fetch_getln(conn_t *); ssize_t fetch_write(conn_t *, const void *, size_t); ssize_t fetch_writev(conn_t *, struct iovec *, int); int fetch_putln(conn_t *, const char *, size_t); diff --git a/lib/libfetch/ftp.c b/lib/libfetch/ftp.c index fc096054c9a2..864eacc5d7ff 100644 --- a/lib/libfetch/ftp.c +++ b/lib/libfetch/ftp.c @@ -142,21 +142,24 @@ unmappedaddr(struct sockaddr_in6 *sin6) static int ftp_chkerr(conn_t *conn) { - ssize_t rlen; - - if ((rlen = fetch_getln(conn)) < 0) { + if (fetch_getln(conn) == -1) { fetch_syserr(); return (-1); } if (isftpinfo(conn->buf)) { while (conn->buflen && !isftpreply(conn->buf)) { - if ((rlen = fetch_getln(conn)) < 0) { + if (fetch_getln(conn) == -1) { fetch_syserr(); return (-1); } } } + while (conn->buflen && + isspace((unsigned char)conn->buf[conn->buflen - 1])) + conn->buflen--; + conn->buf[conn->buflen] = '\0'; + if (!isftpreply(conn->buf)) { ftp_seterr(FTP_PROTOCOL_ERROR); return (-1); diff --git a/lib/libfetch/http.c b/lib/libfetch/http.c index 5111823bd521..adcc75fcb83a 100644 --- a/lib/libfetch/http.c +++ b/lib/libfetch/http.c @@ -148,30 +148,25 @@ struct httpio static int http_new_chunk(struct httpio *io) { - unsigned char *p, *eol; + char *p; if (fetch_getln(io->conn) == -1) return (-1); - p = (unsigned char *)io->conn->buf; - if (io->conn->pos < 2 || !isxdigit(*p)) + if (io->conn->buflen < 2 || !isxdigit((unsigned char)*io->conn->buf)) return (-1); - eol = (unsigned char *)io->conn->buf + io->conn->pos; - while (p < eol && !isspace(*p)) { - if (*p == ';') { + for (p = io->conn->buf; *p && !isspace((unsigned char)*p); ++p) { + if (*p == ';') break; - } else if (*p >= '0' && *p <= '9') { + if (!isxdigit((unsigned char)*p)) + return (-1); + if (isdigit((unsigned char)*p)) { io->chunksize = io->chunksize * 16 + *p - '0'; - } else if (*p >= 'A' && *p <= 'F') { - io->chunksize = io->chunksize * 16 + - 10 + tolower(*p) - 'A'; - } else if (*p >= 'a' && *p <= 'f') { - io->chunksize = io->chunksize * 16 + - 10 + tolower(*p) - 'a'; } else { - return (-1); + io->chunksize = io->chunksize * 16 + + 10 + tolower((unsigned char)*p) - 'a'; } } @@ -226,7 +221,7 @@ http_fillbuf(struct httpio *io, size_t len) if (io->chunked == 0) { if (http_growbuf(io, len) == -1) return (-1); - if ((nbytes = fetch_bufread(io->conn, io->buf, len)) == -1) { + if ((nbytes = fetch_read(io->conn, io->buf, len)) == -1) { io->error = errno; return (-1); } @@ -252,7 +247,7 @@ http_fillbuf(struct httpio *io, size_t len) len = io->chunksize; if (http_growbuf(io, len) == -1) return (-1); - if ((nbytes = fetch_bufread(io->conn, io->buf, len)) == -1) { + if ((nbytes = fetch_read(io->conn, io->buf, len)) == -1) { io->error = errno; return (-1); } @@ -261,8 +256,8 @@ http_fillbuf(struct httpio *io, size_t len) io->chunksize -= nbytes; if (io->chunksize == 0) { - if (fetch_bufread(io->conn, &ch, 1) != 1 || ch != '\r' || - fetch_bufread(io->conn, &ch, 1) != 1 || ch != '\n') + if (fetch_read(io->conn, &ch, 1) != 1 || ch != '\r' || + fetch_read(io->conn, &ch, 1) != 1 || ch != '\n') return (-1); } @@ -520,19 +515,34 @@ clean_http_headerbuf(http_headerbuf_t *buf) init_http_headerbuf(buf); } +/* Remove whitespace at the end of the buffer */ +static void +http_conn_trimright(conn_t *conn) +{ + while (conn->buflen && + isspace((unsigned char)conn->buf[conn->buflen - 1])) + conn->buflen--; + conn->buf[conn->buflen] = '\0'; +} + static hdr_t http_next_header(conn_t *conn, http_headerbuf_t *hbuf, const char **p) { unsigned int i, len; - if (conn->pos == 0 || conn->buf[0] == '\0') + /* + * Have to do the stripping here because of the first line. So + * it's done twice for the subsequent lines. No big deal + */ + http_conn_trimright(conn); + if (conn->buflen == 0) return (hdr_end); /* Copy the line to the headerbuf */ - if (hbuf->bufsize < conn->pos + 1) { - if ((hbuf->buf = realloc(hbuf->buf, conn->pos + 1)) == NULL) + if (hbuf->bufsize < conn->buflen + 1) { + if ((hbuf->buf = realloc(hbuf->buf, conn->buflen + 1)) == NULL) return (hdr_syserror); - hbuf->bufsize = conn->pos + 1; + hbuf->bufsize = conn->buflen + 1; } strcpy(hbuf->buf, conn->buf); hbuf->buflen = conn->buflen; @@ -546,9 +556,12 @@ http_next_header(conn_t *conn, http_headerbuf_t *hbuf, const char **p) return (hdr_syserror); /* - * Note: we previously considered a pure whitespace line - * equivalent to an empty one. This was incorrect. + * Note: we carry on the idea from the previous version + * that a pure whitespace line is equivalent to an empty + * one (so it's not continuation and will be handled when + * we are called next) */ + http_conn_trimright(conn); if (conn->buf[0] != ' ' && conn->buf[0] != "\t"[0]) break;home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a42952d.3986f.2afcdae9>