From: Tobias Heider Subject: got-fetch-http: bufio rewrite To: gameoftrees@openbsd.org Date: Mon, 15 Apr 2024 20:12:33 +0200 Here is the current state of my bufio rewrite of got-fetch-http. This should work better for portable. No idea if I'm doing this right but it works. Had to add rpath to pledge but I am pretty sure that was needed anyway. Not sure if we should move the synchronous bufio funcs to lib/bufio.c instead. There's still a lot of low hanging fruit to clean up. Opinions? ok and continue in tree? ----------------------------------------------- commit a8907938447a84dd5ad3c241fa49a603f3bdc7da (work) from: Tobias Heider date: Mon Apr 15 17:16:16 2024 UTC Rewrite using bufio API M libexec/got-fetch-http/Makefile | 1+ 1- M libexec/got-fetch-http/got-fetch-http.c | 159+ 210- 2 files changed, 160 insertions(+), 211 deletions(-) diff 813645df7b21d54ae779e80fc6e7ad9c913b67d6 a8907938447a84dd5ad3c241fa49a603f3bdc7da commit - 813645df7b21d54ae779e80fc6e7ad9c913b67d6 commit + a8907938447a84dd5ad3c241fa49a603f3bdc7da blob - 9af4776950d6855f9ffcf20c80933a62f0d8232f blob + b0bc7c7d83c8479fd1d9b762eaf78551f430d1a4 --- libexec/got-fetch-http/Makefile +++ libexec/got-fetch-http/Makefile @@ -3,7 +3,7 @@ .include "../../got-version.mk" PROG= got-fetch-http -SRCS= got-fetch-http.c hash.c error.c inflate.c pollfd.c +SRCS= got-fetch-http.c bufio.c hash.c error.c inflate.c pollfd.c CPPFLAGS= -I${.CURDIR}/../../include -I${.CURDIR}/../../lib blob - 43382658e0e06dbf79a48f9353be18accc090566 blob + 624edf99f19853ed8d65865fea8d0392b033c1b1 --- libexec/got-fetch-http/got-fetch-http.c +++ libexec/got-fetch-http/got-fetch-http.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2024 Tobias Heider * Copyright (c) 2022 Omar Polo * * Permission to use, copy, modify, and distribute this software for any @@ -30,6 +31,8 @@ #include "got_version.h" +#include "bufio.h" + #define UPLOAD_PACK_ADV "application/x-git-upload-pack-advertisement" #define UPLOAD_PACK_REQ "application/x-git-upload-pack-request" #define UPLOAD_PACK_RES "application/x-git-upload-pack-result" @@ -39,12 +42,38 @@ #define MINIMUM(a, b) ((a) < (b) ? (a) : (b)) #define hasprfx(str, p) (strncasecmp(str, p, strlen(p)) == 0) -#define DEBUG_HTTP 1 +#define DEBUG_HTTP 0 FILE *tmp; static int verbose; +static char * +bufio_getdelim_sync(struct bufio *bio, const char *nl, size_t *len) +{ + int r; + + do { + r = bufio_read(bio); + if (r == -1 && errno != EAGAIN) + errx(1, "bufio_read: %s", bufio_io_err(bio)); + } while (bio->wantev == BUFIO_WANT_READ); + return buf_getdelim(&bio->rbuf, nl, len); +} + +static size_t +bufio_drain_sync(struct bufio *bio, void *d, size_t len) +{ + int r; + + do { + r = bufio_read(bio); + if (r == -1 && errno != EAGAIN) + errx(1, "bufio_read: %s", bufio_io_err(bio)); + } while (bio->wantev == BUFIO_WANT_READ); + return bufio_drain(bio, d, len); +} + static long long hexstrtonum(const char *str, long long min, long long max, const char **errstr) { @@ -68,95 +97,19 @@ hexstrtonum(const char *str, long long min, long long } static int -stdio_tls_write(void *arg, const char *buf, int len) -{ - struct tls *ctx = arg; - ssize_t ret; - - do { - ret = tls_write(ctx, buf, len); - } while (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT); - - if (ret == -1) - warn("tls_write: %s", tls_error(ctx)); - - return ret; -} - -static int -stdio_tls_read(void *arg, char *buf, int len) -{ - struct tls *ctx = arg; - ssize_t ret; - - do { - ret = tls_read(ctx, buf, len); - } while (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT); - - if (ret == -1) - warn("tls_read: %s", tls_error(ctx)); - - return ret; -} - -static int -stdio_tls_close(void *arg) -{ - struct tls *ctx = arg; - int ret; - - do { - ret = tls_close(ctx); - } while (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT); - - return ret; -} - -static FILE * dial(int https, const char *host, const char *port) { - FILE *fp; - struct tls *ctx; - struct tls_config *conf; struct addrinfo hints, *res, *res0; - int r, error, saved_errno, fd = -1; + int error, saved_errno, fd = -1; const char *cause = NULL; - if (https) { - if ((conf = tls_config_new()) == NULL) - errx(1, "failed to create TLS configuration"); - if ((ctx = tls_client()) == NULL) - errx(1, "failed to create TLS client"); - if (tls_configure(ctx, conf) == -1) - errx(1, "TLS configuration failure: %s", - tls_error(ctx)); - tls_config_free(conf); - - if (tls_connect(ctx, host, port) == -1) { - warnx("connect to %s:%s: %s", host, port, - tls_error(ctx)); - tls_close(ctx); - return NULL; - } - do { - r = tls_handshake(ctx); - } while (r == TLS_WANT_POLLIN || r == TLS_WANT_POLLOUT); - fp = funopen(ctx, stdio_tls_read, stdio_tls_write, NULL, - stdio_tls_close); - if (fp == NULL) { - warn("funopen"); - tls_free(ctx); - } - return fp; - } - memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; hints.ai_socktype = SOCK_STREAM; error = getaddrinfo(host, port, &hints, &res0); if (error) { warnx("%s", gai_strerror(error)); - return NULL; + return -1; } for (res = res0; res; res = res->ai_next) { @@ -180,29 +133,20 @@ dial(int https, const char *host, const char *port) if (fd == -1) { warn("%s", cause); - return NULL; + return -1; } - if ((fp = fdopen(fd, "r+")) == NULL) { - warn("fdopen"); - close(fd); - } - return fp; + return fd; } -static FILE * -http_open(int https, const char *method, const char *host, const char *port, - const char *path, const char *path_sufx, const char *query, - const char *ctype) +static int +http_open(struct bufio *bio, int https, const char *method, const char *host, const char *port, + const char *path, const char *path_sufx, const char *query, const char *ctype) { - FILE *fp; const char *chdr = NULL, *te = ""; char *p, *req; int r; - - if ((fp = dial(https, host, port)) == NULL) - return NULL; - + if (path_sufx != NULL && *path && path[strlen(path) - 1] == '/') path_sufx++; /* skip the slash */ @@ -224,65 +168,70 @@ http_open(int https, const char *method, const char *h "%s%s%s\r\n", method, p, host, GOT_USERAGENT, chdr ? chdr : "", ctype ? ctype : "", te); + if (r == -1) + err(1, "asprintf"); free(p); - if (r == -1) - err(1, "asprintf"); if (verbose > 0) fprintf(stderr, "%s: request: %s", getprogname(), req); + - if (fwrite(req, 1, r, fp) != r) { - free(req); - fclose(fp); - return NULL; - } + r = bufio_compose(bio, req, r); + if (r == -1) + err(1, "bufio_compose_fmt"); free(req); - return fp; + do { + r = bufio_write(bio); + if (r == -1 && errno != EAGAIN) + errx(1, "bufio_read: %s", bufio_io_err(bio)); + } while (bio->wantev == BUFIO_WANT_WRITE); + + return 0; } static int -http_parse_reply(FILE *fp, int *chunked, const char *expected_ctype) +http_parse_reply(struct bufio *bio, int *chunked, const char *expected_ctype) { - char *cp, *line = NULL; - size_t linesize = 0; - ssize_t linelen; + char *cp, *line; + size_t linelen; *chunked = 0; - if ((linelen = getline(&line, &linesize, fp)) == -1) { - warn("%s: getline", __func__); + line = bufio_getdelim_sync(bio, "\r\n", &linelen); + if (line == NULL) { + warnx("%s: bufio_getdelim_sync()", __func__); return -1; } if (verbose > 0) fprintf(stderr, "%s: response: %s", getprogname(), line); - if ((cp = strchr(line, '\r')) == NULL) { - warnx("malformed HTTP response"); - goto err; - } - *cp = '\0'; - if ((cp = strchr(line, ' ')) == NULL) { warnx("malformed HTTP response"); - goto err; + return -1; } cp++; if (strncmp(cp, "200 ", 4) != 0) { warnx("malformed HTTP response"); - goto err; + return -1; } + buf_drain(&bio->rbuf, linelen); - while ((linelen = getline(&line, &linesize, fp)) != -1) { - if (line[linelen-1] == '\n') - line[--linelen] = '\0'; - if (linelen > 0 && line[linelen-1] == '\r') - line[--linelen] = '\0'; - - if (*line == '\0') + while(1) { + line = bufio_getdelim_sync(bio, "\r\n", &linelen); + if (line == NULL) { + warnx("%s: bufio_getdelim_sync()", __func__); + return -1; + } + if (*line == '\0') { + buf_drain(&bio->rbuf, linelen); break; + } +#if DEBUG_HTTP + fprintf(stderr, "%s: %s\n", __func__, line); +#endif if (hasprfx(line, "content-type:")) { cp = strchr(line, ':') + 1; @@ -291,68 +240,52 @@ http_parse_reply(FILE *fp, int *chunked, const char *e if (strcmp(cp, expected_ctype) != 0) { warnx("server not using the \"smart\" " "HTTP protocol."); - goto err; + return -1; } } - if (hasprfx(line, "transfer-encoding:")) { cp = strchr(line, ':') + 1; cp += strspn(cp, " \t"); cp[strcspn(cp, " \t")] = '\0'; if (strcmp(cp, "chunked") != 0) { warnx("unknown transfer-encoding"); - goto err; + return -1; } *chunked = 1; } + buf_drain(&bio->rbuf, linelen); } - free(line); return 0; - -err: - free(line); - return -1; } static ssize_t -http_read(FILE *fp, int chunked, size_t *chunksz, void *buf, size_t bufsz) +http_read(struct bufio *bio, int chunked, size_t *chunksz, char *buf, size_t bufsz) { const char *errstr; - char *cp, *line = NULL; - size_t r, linesize = 0; + char *line = NULL; + size_t r; ssize_t ret = 0, linelen; if (!chunked) { - r = fread(buf, 1, bufsz, fp); - if (r == 0 && ferror(fp)) + r = bufio_drain_sync(bio, buf, bufsz); + if (r == 0) return -1; -#if DEBUG_HTTP - fwrite(buf, 1, r, stderr); -#endif return r; } while (bufsz > 0) { if (*chunksz == 0) { again: - if ((linelen = getline(&line, &linesize, fp)) == -1) { - if (ferror(fp)) { - warn("%s: getline", __func__); - ret = -1; - } + line = bufio_getdelim_sync(bio, "\r\n", &linelen); + if (line == NULL) { + buf_drain(&bio->rbuf, linelen); break; } - - if ((cp = strchr(line, '\r')) == NULL) { - warnx("invalid HTTP chunk: missing CR"); - ret = -1; - break; - } - *cp = '\0'; - - if (*line == '\0') + if (*line == '\0') { + buf_drain(&bio->rbuf, linelen); goto again; /* was the CRLF after the chunk */ + } *chunksz = hexstrtonum(line, 0, INT_MAX, &errstr); if (errstr != NULL) { @@ -362,14 +295,15 @@ http_read(FILE *fp, int chunked, size_t *chunksz, void break; } - if (*chunksz == 0) + if (*chunksz == 0) { + buf_drain(&bio->rbuf, linelen); break; + } + buf_drain(&bio->rbuf, linelen); } - r = fread(buf, 1, MINIMUM(*chunksz, bufsz), fp); + r = bufio_drain_sync(bio, buf, MINIMUM(*chunksz, bufsz)); if (r == 0) { - if (ferror(fp)) - ret = -1; break; } @@ -384,73 +318,77 @@ http_read(FILE *fp, int chunked, size_t *chunksz, void *chunksz -= r; } - free(line); return ret; } static void -http_chunk(FILE *fp, const void *buf, size_t len) +http_chunk(struct bufio *bio, const void *buf, size_t len) { - /* fprintf(stderr, "> %.*s", (int)len, (char *)buf); */ + int r; + bufio_compose_fmt(bio, "%zx\r\n", len); + bufio_compose(bio, buf, len); + bufio_compose(bio, "\r\n", 2); - fprintf(fp, "%zx\r\n", len); - if (fwrite(buf, 1, len, fp) != len || - fwrite("\r\n", 1, 2, fp) != 2) - err(1, "%s fwrite", __func__); + do { + r = bufio_write(bio); + if (r == -1 && errno != EAGAIN) + errx(1, "bufio_read: %s", bufio_io_err(bio)); + } while (bio->wantev == BUFIO_WANT_WRITE); } static int get_refs(int https, const char *host, const char *port, const char *path) { + struct bufio bio; char buf[HTTP_BUFSIZ]; const char *errstr, *sufx = "/info/refs"; - FILE *fp; size_t skip, chunksz = 0; ssize_t r; int chunked; + int sock; + int ret = -1; - fp = http_open(https, "GET", host, port, path, sufx, - "service=git-upload-pack", NULL); - if (fp == NULL) + if ((sock = dial(https, host, port)) == -1) return -1; - if (http_parse_reply(fp, &chunked, UPLOAD_PACK_ADV) == -1) { - fclose(fp); - return -1; + bufio_init(&bio); + bufio_set_fd(&bio, sock); + if (https && bufio_starttls(&bio, host, 0, NULL, 0, NULL, 0)) { + warnx("bufio_starttls"); + goto err; } + if (http_open(&bio, https, "GET", host, port, path, sufx, + "service=git-upload-pack", NULL) == -1) + goto err; + + if (http_parse_reply(&bio, &chunked, UPLOAD_PACK_ADV) == -1) + goto err; + /* skip first pack; why git over http is like this? */ - r = http_read(fp, chunked, &chunksz, buf, 4); - if (r <= 0) { - fclose(fp); - return -1; - } + r = http_read(&bio, chunked, &chunksz, buf, 4); + if (r <= 0) + goto err; buf[4] = '\0'; skip = hexstrtonum(buf, 0, INT_MAX, &errstr); if (errstr != NULL) { warnx("pktlen is %s", errstr); - fclose(fp); - return -1; + goto err; } /* TODO: validate it's # service=git-upload-pack\n */ while (skip > 0) { - r = http_read(fp, chunked, &chunksz, buf, + r = http_read(&bio, chunked, &chunksz, buf, MINIMUM(skip, sizeof(buf))); - if (r <= 0) { - fclose(fp); - return -1; - } - + if (r <= 0) + goto err; skip -= r; } for (;;) { - r = http_read(fp, chunked, &chunksz, buf, sizeof(buf)); - if (r == -1) { - fclose(fp); - return -1; - } + r = http_read(&bio, chunked, &chunksz, buf, sizeof(buf)); + if (r == -1) + goto err; if (r == 0) break; @@ -459,27 +397,41 @@ get_refs(int https, const char *host, const char *port } fflush(stdout); - fclose(fp); - return 0; + ret = 0; +err: + bufio_close(&bio); + bufio_free(&bio); + return ret; } static int upload_request(int https, const char *host, const char *port, const char *path, FILE *in) { + struct bufio bio; const char *errstr; char buf[HTTP_BUFSIZ]; - FILE *fp; ssize_t r; size_t chunksz = 0; long long t; int chunked; + int sock; + int ret = -1; - fp = http_open(https, "POST", host, port, path, "/git-upload-pack", - NULL, UPLOAD_PACK_REQ); - if (fp == NULL) + if ((sock = dial(https, host, port)) == -1) return -1; + bufio_init(&bio); + bufio_set_fd(&bio, sock); + if (https && bufio_starttls(&bio, host, 0, NULL, 0, NULL, 0)) { + warnx("bufio_starttls"); + goto err; + } + + if (http_open(&bio, https, "POST", host, port, path, "/git-upload-pack", + NULL, UPLOAD_PACK_REQ) == -1) + goto err; + for (;;) { r = fread(buf, 1, 4, in); if (r != 4) @@ -495,8 +447,8 @@ upload_request(int https, const char *host, const char /* no idea why 0000 is not enough. */ if (t == 0) { const char *x = "00000009done\n"; - http_chunk(fp, x, strlen(x)); - http_chunk(fp, NULL, 0); + http_chunk(&bio, x, strlen(x)); + http_chunk(&bio, NULL, 0); break; } @@ -509,18 +461,16 @@ upload_request(int https, const char *host, const char if (r != t - 4) goto err; - http_chunk(fp, buf, t); + http_chunk(&bio, buf, t); } - if (http_parse_reply(fp, &chunked, UPLOAD_PACK_RES) == -1) + if (http_parse_reply(&bio, &chunked, UPLOAD_PACK_RES) == -1) goto err; for (;;) { - r = http_read(fp, chunked, &chunksz, buf, sizeof(buf)); - if (r == -1) { - fclose(fp); - return -1; - } + r = http_read(&bio, chunked, &chunksz, buf, sizeof(buf)); + if (r == -1) + goto err; if (r == 0) break; @@ -528,12 +478,11 @@ upload_request(int https, const char *host, const char fwrite(buf, 1, r, stdout); } - fclose(fp); - return 0; - + ret = 0; err: - fclose(fp); - return -1; + bufio_close(&bio); + bufio_free(&bio); + return ret; } static __dead void @@ -558,7 +507,7 @@ main(int argc, char **argv) #endif #if !DEBUG_HTTP || defined(PROFILE) - if (pledge("stdio inet dns", NULL) == -1) + if (pledge("stdio inet dns rpath", NULL) == -1) err(1, "pledge"); #endif