git: afb60498e0c9 - main - libfetch: Add read buffering

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Mon, 29 Jun 2026 15:52:14 UTC
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=afb60498e0c9836a6e3a7efda8a51710e208a608

commit afb60498e0c9836a6e3a7efda8a51710e208a608
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2026-06-27 14:11:38 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2026-06-29 13:49:19 +0000

    libfetch: Add read buffering
    
    Previously, we would read FTP control connection messages and HTTP
    reponse headers one character at a time.  Now, we read as much as will
    fit in our buffer and look for a newline.  If there is data left over,
    it will be reused by the next fetch_getln() call.  This also requires
    the addition of a fetch_bufread() which takes the buffer into account,
    otherwise the start of the HTTP response body will be stuck in the
    buffer after we read the last line of the header.
    
    This should noticeably improve HTTP performance, especially for small
    transfers.
    
    MFC after:      1 week
---
 lib/libfetch/common.c | 109 ++++++++++++++++++++++++++++++++++++++++----------
 lib/libfetch/common.h |   4 +-
 lib/libfetch/ftp.c    |  11 ++---
 lib/libfetch/http.c   |  61 +++++++++++-----------------
 4 files changed, 118 insertions(+), 67 deletions(-)

diff --git a/lib/libfetch/common.c b/lib/libfetch/common.c
index c5fb45f70da2..c1888a1518a0 100644
--- a/lib/libfetch/common.c
+++ b/lib/libfetch/common.c
@@ -1403,47 +1403,112 @@ fetch_read(conn_t *conn, void *buf, size_t len)
  */
 #define MIN_BUF_SIZE 1024
 
-int
+ssize_t
 fetch_getln(conn_t *conn)
 {
 	char *tmp;
 	size_t tmpsize;
-	ssize_t len;
-	char c;
+	ssize_t rlen;
 
+	/* allocate initial buffer */
 	if (conn->buf == NULL) {
-		if ((conn->buf = malloc(MIN_BUF_SIZE)) == NULL) {
-			errno = ENOMEM;
+		if ((conn->buf = malloc(MIN_BUF_SIZE)) == NULL)
 			return (-1);
-		}
 		conn->bufsize = MIN_BUF_SIZE;
+		conn->buflen = 0;
+		conn->pos = 0;
 	}
 
-	conn->buf[0] = '\0';
-	conn->buflen = 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;
+	}
 
-	do {
-		len = fetch_read(conn, &c, 1);
-		if (len == -1)
+	/* do we have a complete line? */
+	while (conn->pos < conn->buflen)
+		if (conn->buf[conn->pos++] == '\n')
+			goto found;
+
+	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)
 			return (-1);
-		if (len == 0)
+		/* advance and terminate */
+		conn->buflen += rlen;
+		conn->buf[conn->buflen] = '\0';
+		/* connection closed */
+		if (rlen == 0)
 			break;
-		conn->buf[conn->buflen++] = c;
-		if (conn->buflen == conn->bufsize) {
+		/* 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) {
 			tmp = conn->buf;
-			tmpsize = conn->bufsize * 2 + 1;
-			if ((tmp = realloc(tmp, tmpsize)) == NULL) {
-				errno = ENOMEM;
+			tmpsize = conn->bufsize * 2;
+			if ((tmp = realloc(tmp, tmpsize)) == NULL)
 				return (-1);
-			}
 			conn->buf = tmp;
 			conn->bufsize = tmpsize;
 		}
-	} while (c != '\n');
+	}
+	/* 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);
+}
 
-	conn->buf[conn->buflen] = '\0';
-	DEBUGF("<<< %s", conn->buf);
-	return (0);
+
+/*
+ * 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));
 }
 
 
diff --git a/lib/libfetch/common.h b/lib/libfetch/common.h
index 479c5e03c0d1..5ad9bde513a0 100644
--- a/lib/libfetch/common.h
+++ b/lib/libfetch/common.h
@@ -51,6 +51,7 @@ 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 */
@@ -120,7 +121,8 @@ 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);
-int		 fetch_getln(conn_t *);
+ssize_t		 fetch_getln(conn_t *);
+ssize_t		 fetch_bufread(conn_t *, void *, size_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 864eacc5d7ff..fc096054c9a2 100644
--- a/lib/libfetch/ftp.c
+++ b/lib/libfetch/ftp.c
@@ -142,24 +142,21 @@ unmappedaddr(struct sockaddr_in6 *sin6)
 static int
 ftp_chkerr(conn_t *conn)
 {
-	if (fetch_getln(conn) == -1) {
+	ssize_t rlen;
+
+	if ((rlen = fetch_getln(conn)) < 0) {
 		fetch_syserr();
 		return (-1);
 	}
 	if (isftpinfo(conn->buf)) {
 		while (conn->buflen && !isftpreply(conn->buf)) {
-			if (fetch_getln(conn) == -1) {
+			if ((rlen = fetch_getln(conn)) < 0) {
 				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 adcc75fcb83a..5111823bd521 100644
--- a/lib/libfetch/http.c
+++ b/lib/libfetch/http.c
@@ -148,25 +148,30 @@ struct httpio
 static int
 http_new_chunk(struct httpio *io)
 {
-	char *p;
+	unsigned char *p, *eol;
 
 	if (fetch_getln(io->conn) == -1)
 		return (-1);
 
-	if (io->conn->buflen < 2 || !isxdigit((unsigned char)*io->conn->buf))
+	p = (unsigned char *)io->conn->buf;
+	if (io->conn->pos < 2 || !isxdigit(*p))
 		return (-1);
 
-	for (p = io->conn->buf; *p && !isspace((unsigned char)*p); ++p) {
-		if (*p == ';')
+	eol = (unsigned char *)io->conn->buf + io->conn->pos;
+	while (p < eol && !isspace(*p)) {
+		if (*p == ';') {
 			break;
-		if (!isxdigit((unsigned char)*p))
-			return (-1);
-		if (isdigit((unsigned char)*p)) {
+		} else if (*p >= '0' && *p <= '9') {
 			io->chunksize = io->chunksize * 16 +
 			    *p - '0';
-		} else {
+		} else if (*p >= 'A' && *p <= 'F') {
 			io->chunksize = io->chunksize * 16 +
-			    10 + tolower((unsigned char)*p) - 'a';
+			    10 + tolower(*p) - 'A';
+		} else if (*p >= 'a' && *p <= 'f') {
+			io->chunksize = io->chunksize * 16 +
+			    10 + tolower(*p) - 'a';
+		} else {
+			return (-1);
 		}
 	}
 
@@ -221,7 +226,7 @@ http_fillbuf(struct httpio *io, size_t len)
 	if (io->chunked == 0) {
 		if (http_growbuf(io, len) == -1)
 			return (-1);
-		if ((nbytes = fetch_read(io->conn, io->buf, len)) == -1) {
+		if ((nbytes = fetch_bufread(io->conn, io->buf, len)) == -1) {
 			io->error = errno;
 			return (-1);
 		}
@@ -247,7 +252,7 @@ http_fillbuf(struct httpio *io, size_t len)
 		len = io->chunksize;
 	if (http_growbuf(io, len) == -1)
 		return (-1);
-	if ((nbytes = fetch_read(io->conn, io->buf, len)) == -1) {
+	if ((nbytes = fetch_bufread(io->conn, io->buf, len)) == -1) {
 		io->error = errno;
 		return (-1);
 	}
@@ -256,8 +261,8 @@ http_fillbuf(struct httpio *io, size_t len)
 	io->chunksize -= nbytes;
 
 	if (io->chunksize == 0) {
-		if (fetch_read(io->conn, &ch, 1) != 1 || ch != '\r' ||
-		    fetch_read(io->conn, &ch, 1) != 1 || ch != '\n')
+		if (fetch_bufread(io->conn, &ch, 1) != 1 || ch != '\r' ||
+		    fetch_bufread(io->conn, &ch, 1) != 1 || ch != '\n')
 			return (-1);
 	}
 
@@ -515,34 +520,19 @@ 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;
 
-	/*
-	 * 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)
+	if (conn->pos == 0 || conn->buf[0] == '\0')
 		return (hdr_end);
 
 	/* Copy the line to the headerbuf */
-	if (hbuf->bufsize < conn->buflen + 1) {
-		if ((hbuf->buf = realloc(hbuf->buf, conn->buflen + 1)) == NULL)
+	if (hbuf->bufsize < conn->pos + 1) {
+		if ((hbuf->buf = realloc(hbuf->buf, conn->pos + 1)) == NULL)
 			return (hdr_syserror);
-		hbuf->bufsize = conn->buflen + 1;
+		hbuf->bufsize = conn->pos + 1;
 	}
 	strcpy(hbuf->buf, conn->buf);
 	hbuf->buflen = conn->buflen;
@@ -556,12 +546,9 @@ http_next_header(conn_t *conn, http_headerbuf_t *hbuf, const char **p)
 			return (hdr_syserror);
 
 		/*
-		 * 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)
+		 * Note: we previously considered a pure whitespace line
+		 * equivalent to an empty one.  This was incorrect.
 		 */
-		http_conn_trimright(conn);
 		if (conn->buf[0] != ' ' && conn->buf[0] != "\t"[0])
 			break;