From 6b1fae1dfbbdb6dc352567c0fc45a9e87474192d Mon Sep 17 00:00:00 2001 From: Max Kirillov Date: Sun, 10 Jun 2018 18:05:19 +0300 Subject: http-backend: cleanup writing to child process As explained in [1], we should not assume the reason why the writing has failed, and even if the reason is that child has existed not the reason why it have done so. So instead just say that writing has failed. [1] https://public-inbox.org/git/20180604044408.GD14451@sigill.intra.peff.net/ Signed-off-by: Max Kirillov Signed-off-by: Junio C Hamano --- http-backend.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'http-backend.c') diff --git a/http-backend.c b/http-backend.c index adaef16fad..cefdfd6fc6 100644 --- a/http-backend.c +++ b/http-backend.c @@ -279,6 +279,12 @@ static struct rpc_service *select_service(struct strbuf *hdr, const char *name) return svc; } +static void write_to_child(int out, const unsigned char *buf, ssize_t len, const char *prog_name) +{ + if (write_in_full(out, buf, len) < 0) + die("unable to write to '%s'", prog_name); +} + /* * This is basically strbuf_read(), except that if we * hit max_request_buffer we die (we'd rather reject a @@ -361,9 +367,8 @@ static void inflate_request(const char *prog_name, int out, int buffer_input) die("zlib error inflating request, result %d", ret); n = stream.total_out - cnt; - if (write_in_full(out, out_buf, n) < 0) - die("%s aborted reading request", prog_name); - cnt += n; + write_to_child(out, out_buf, stream.total_out - cnt, prog_name); + cnt = stream.total_out; if (ret == Z_STREAM_END) goto done; @@ -382,8 +387,7 @@ static void copy_request(const char *prog_name, int out) ssize_t n = read_request(0, &buf); if (n < 0) die_errno("error reading request body"); - if (write_in_full(out, buf, n) < 0) - die("%s aborted reading request", prog_name); + write_to_child(out, buf, n, prog_name); close(out); free(buf); } -- cgit v1.2.3 From c79edf73f4b018310428632887f9ce2ce32d839a Mon Sep 17 00:00:00 2001 From: Max Kirillov Date: Sun, 10 Jun 2018 18:05:20 +0300 Subject: http-backend: respect CONTENT_LENGTH as specified by rfc3875 http-backend reads whole input until EOF. However, the RFC 3875 specifies that a script must read only as many bytes as specified by CONTENT_LENGTH environment variable. Web server may exercise the specification by not closing the script's standard input after writing content. In that case http-backend would hang waiting for the input. The issue is known to happen with IIS/Windows, for example. Make http-backend read only CONTENT_LENGTH bytes, if it's defined, rather than the whole input until EOF. If the variable is not defined, keep older behavior of reading until EOF because it is used to support chunked transfer-encoding. This commit only fixes buffered input, whcih reads whole body before processign it. Non-buffered input is going to be fixed in subsequent commit. Signed-off-by: Florian Manschwetus [mk: fixed trivial build failures and polished style issues] Helped-by: Junio C Hamano Signed-off-by: Max Kirillov Signed-off-by: Junio C Hamano --- http-backend.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 7 deletions(-) (limited to 'http-backend.c') diff --git a/http-backend.c b/http-backend.c index cefdfd6fc6..d0b6cb1b09 100644 --- a/http-backend.c +++ b/http-backend.c @@ -290,7 +290,7 @@ static void write_to_child(int out, const unsigned char *buf, ssize_t len, const * hit max_request_buffer we die (we'd rather reject a * maliciously large request than chew up infinite memory). */ -static ssize_t read_request(int fd, unsigned char **out) +static ssize_t read_request_eof(int fd, unsigned char **out) { size_t len = 0, alloc = 8192; unsigned char *buf = xmalloc(alloc); @@ -327,7 +327,46 @@ static ssize_t read_request(int fd, unsigned char **out) } } -static void inflate_request(const char *prog_name, int out, int buffer_input) +static ssize_t read_request_fixed_len(int fd, ssize_t req_len, unsigned char **out) +{ + unsigned char *buf = NULL; + ssize_t cnt = 0; + + if (max_request_buffer < req_len) { + die("request was larger than our maximum size (%lu): " + "%" PRIuMAX "; try setting GIT_HTTP_MAX_REQUEST_BUFFER", + max_request_buffer, (uintmax_t)req_len); + } + + buf = xmalloc(req_len); + cnt = read_in_full(fd, buf, req_len); + if (cnt < 0) { + free(buf); + return -1; + } + *out = buf; + return cnt; +} + +static ssize_t get_content_length(void) +{ + ssize_t val = -1; + const char *str = getenv("CONTENT_LENGTH"); + + if (str && !git_parse_ssize_t(str, &val)) + die("failed to parse CONTENT_LENGTH: %s", str); + return val; +} + +static ssize_t read_request(int fd, unsigned char **out, ssize_t req_len) +{ + if (req_len < 0) + return read_request_eof(fd, out); + else + return read_request_fixed_len(fd, req_len, out); +} + +static void inflate_request(const char *prog_name, int out, int buffer_input, ssize_t req_len) { git_zstream stream; unsigned char *full_request = NULL; @@ -345,7 +384,7 @@ static void inflate_request(const char *prog_name, int out, int buffer_input) if (full_request) n = 0; /* nothing left to read */ else - n = read_request(0, &full_request); + n = read_request(0, &full_request, req_len); stream.next_in = full_request; } else { n = xread(0, in_buf, sizeof(in_buf)); @@ -381,10 +420,10 @@ done: free(full_request); } -static void copy_request(const char *prog_name, int out) +static void copy_request(const char *prog_name, int out, ssize_t req_len) { unsigned char *buf; - ssize_t n = read_request(0, &buf); + ssize_t n = read_request(0, &buf, req_len); if (n < 0) die_errno("error reading request body"); write_to_child(out, buf, n, prog_name); @@ -399,6 +438,7 @@ static void run_service(const char **argv, int buffer_input) const char *host = getenv("REMOTE_ADDR"); int gzipped_request = 0; struct child_process cld = CHILD_PROCESS_INIT; + ssize_t req_len = get_content_length(); if (encoding && !strcmp(encoding, "gzip")) gzipped_request = 1; @@ -425,9 +465,9 @@ static void run_service(const char **argv, int buffer_input) close(1); if (gzipped_request) - inflate_request(argv[0], cld.in, buffer_input); + inflate_request(argv[0], cld.in, buffer_input, req_len); else if (buffer_input) - copy_request(argv[0], cld.in); + copy_request(argv[0], cld.in, req_len); else close(0); -- cgit v1.2.3 From 6c213e863aeb0af078bf82deefb22da20427c2ab Mon Sep 17 00:00:00 2001 From: Max Kirillov Date: Fri, 27 Jul 2018 06:48:59 +0300 Subject: http-backend: respect CONTENT_LENGTH for receive-pack Push passes to another commands, as described in https://public-inbox.org/git/20171129032214.GB32345@sigill.intra.peff.net/ As it gets complicated to correctly track the data length, instead transfer the data through parent process and cut the pipe as the specified length is reached. Do it only when CONTENT_LENGTH is set, otherwise pass the input directly to the forked commands. Add tests for cases: * CONTENT_LENGTH is set, script's stdin has more data, with all combinations of variations: fetch or push, plain or compressed body, correct or truncated input. * CONTENT_LENGTH is specified to a value which does not fit into ssize_t. Helped-by: Junio C Hamano Signed-off-by: Max Kirillov Signed-off-by: Junio C Hamano --- http-backend.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) (limited to 'http-backend.c') diff --git a/http-backend.c b/http-backend.c index d0b6cb1b09..e88d29f62b 100644 --- a/http-backend.c +++ b/http-backend.c @@ -373,6 +373,8 @@ static void inflate_request(const char *prog_name, int out, int buffer_input, ss unsigned char in_buf[8192]; unsigned char out_buf[8192]; unsigned long cnt = 0; + int req_len_defined = req_len >= 0; + size_t req_remaining_len = req_len; memset(&stream, 0, sizeof(stream)); git_inflate_init_gzip_only(&stream); @@ -387,8 +389,15 @@ static void inflate_request(const char *prog_name, int out, int buffer_input, ss n = read_request(0, &full_request, req_len); stream.next_in = full_request; } else { - n = xread(0, in_buf, sizeof(in_buf)); + ssize_t buffer_len; + if (req_len_defined && req_remaining_len <= sizeof(in_buf)) + buffer_len = req_remaining_len; + else + buffer_len = sizeof(in_buf); + n = xread(0, in_buf, buffer_len); stream.next_in = in_buf; + if (req_len_defined && n > 0) + req_remaining_len -= n; } if (n <= 0) @@ -431,6 +440,23 @@ static void copy_request(const char *prog_name, int out, ssize_t req_len) free(buf); } +static void pipe_fixed_length(const char *prog_name, int out, size_t req_len) +{ + unsigned char buf[8192]; + size_t remaining_len = req_len; + + while (remaining_len > 0) { + size_t chunk_length = remaining_len > sizeof(buf) ? sizeof(buf) : remaining_len; + ssize_t n = xread(0, buf, chunk_length); + if (n < 0) + die_errno("Reading request failed"); + write_to_child(out, buf, n, prog_name); + remaining_len -= n; + } + + close(out); +} + static void run_service(const char **argv, int buffer_input) { const char *encoding = getenv("HTTP_CONTENT_ENCODING"); @@ -457,7 +483,7 @@ static void run_service(const char **argv, int buffer_input) "GIT_COMMITTER_EMAIL=%s@http.%s", user, host); cld.argv = argv; - if (buffer_input || gzipped_request) + if (buffer_input || gzipped_request || req_len >= 0) cld.in = -1; cld.git_cmd = 1; if (start_command(&cld)) @@ -468,6 +494,8 @@ static void run_service(const char **argv, int buffer_input) inflate_request(argv[0], cld.in, buffer_input, req_len); else if (buffer_input) copy_request(argv[0], cld.in, req_len); + else if (req_len >= 0) + pipe_fixed_length(argv[0], cld.in, req_len); else close(0); -- cgit v1.2.3