git: fde60e3a4e92 - main - Revert "libfetch: Add read buffering"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 29 Jun 2026 15:54:21 UTC
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;