Merge branch 'mk/http-backend-content-length'
authorJunio C Hamano <gitster@pobox.com>
Fri, 17 Aug 2018 20:09:57 +0000 (13:09 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 17 Aug 2018 20:09:57 +0000 (13:09 -0700)
The http-backend (used for smart-http transport) used to slurp the
whole input until EOF, without paying attention to CONTENT_LENGTH
that is supplied in the environment and instead expecting the Web
server to close the input stream. This has been fixed.

* mk/http-backend-content-length:
t5562: avoid non-portable "export FOO=bar" construct
http-backend: respect CONTENT_LENGTH for receive-pack
http-backend: respect CONTENT_LENGTH as specified by rfc3875
http-backend: cleanup writing to child process

config.c
config.h
help.c
http-backend.c
t/t5562-http-backend-content-length.sh [new file with mode: 0755]
t/t5562/invoke-with-content-length.pl [new file with mode: 0755]
index 10522f399e6eb5300b55eaeaf8fcbf6546e53d11..4446909229a0f7b14d7428ad9860c9a655c77df8 100644 (file)
--- a/config.c
+++ b/config.c
@@ -933,7 +933,7 @@ int git_parse_ulong(const char *value, unsigned long *ret)
        return 1;
 }
 
-static int git_parse_ssize_t(const char *value, ssize_t *ret)
+int git_parse_ssize_t(const char *value, ssize_t *ret)
 {
        intmax_t tmp;
        if (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_t)))
index bb2f506b27137e8d66ab4cd96439143dced94780..1d56fe6aa28fe1a244118f00733e3829217b71ca 100644 (file)
--- a/config.h
+++ b/config.h
@@ -82,6 +82,7 @@ extern void git_config(config_fn_t fn, void *);
 extern int config_with_options(config_fn_t fn, void *,
                               struct git_config_source *config_source,
                               const struct config_options *opts);
+extern int git_parse_ssize_t(const char *, ssize_t *);
 extern int git_parse_ulong(const char *, unsigned long *);
 extern int git_parse_maybe_bool(const char *);
 extern int git_config_int(const char *, const char *);
diff --git a/help.c b/help.c
index 3ebf0568dba1f20039b6e4d02c18c652078ad2cb..9c0b5a8de974727e98efc1ad28fd49655290ffd8 100644 (file)
--- a/help.c
+++ b/help.c
@@ -693,6 +693,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
                else
                        printf("no commit associated with this build\n");
                printf("sizeof-long: %d\n", (int)sizeof(long));
+               printf("sizeof-size_t: %d\n", (int)sizeof(size_t));
                /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
        }
        return 0;
index bd0442a805a16bf2313b65a54fdfd4bf937b13ad..88c38c834ba479447be8eb64f3bc5331cdb9ed49 100644 (file)
@@ -279,12 +279,18 @@ 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
  * 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);
@@ -321,13 +327,54 @@ 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;
        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);
@@ -339,11 +386,18 @@ 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));
+                       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)
@@ -361,9 +415,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;
@@ -376,18 +429,34 @@ static void inflate_request(const char *prog_name, int out, int buffer_input)
        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");
-       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);
 }
 
+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");
@@ -395,6 +464,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;
@@ -413,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))
@@ -421,9 +491,11 @@ 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 if (req_len >= 0)
+               pipe_fixed_length(argv[0], cld.in, req_len);
        else
                close(0);
 
diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh
new file mode 100755 (executable)
index 0000000..43570ce
--- /dev/null
@@ -0,0 +1,156 @@
+#!/bin/sh
+
+test_description='test git-http-backend respects CONTENT_LENGTH'
+. ./test-lib.sh
+
+test_lazy_prereq GZIP 'gzip --version'
+
+verify_http_result() {
+       # some fatal errors still produce status 200
+       # so check if there is the error message
+       if grep 'fatal:' act.err
+       then
+               return 1
+       fi
+
+       if ! grep "Status" act.out >act
+       then
+               printf "Status: 200 OK\r\n" >act
+       fi
+       printf "Status: $1\r\n" >exp &&
+       test_cmp exp act
+}
+
+test_http_env() {
+       handler_type="$1"
+       request_body="$2"
+       shift
+       env \
+               CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
+               QUERY_STRING="/repo.git/git-$handler_type-pack" \
+               PATH_TRANSLATED="$PWD/.git/git-$handler_type-pack" \
+               GIT_HTTP_EXPORT_ALL=TRUE \
+               REQUEST_METHOD=POST \
+               "$TEST_DIRECTORY"/t5562/invoke-with-content-length.pl \
+                   "$request_body" git http-backend >act.out 2>act.err
+}
+
+ssize_b100dots() {
+       # hardcoded ((size_t) SSIZE_MAX) + 1
+       case "$(build_option sizeof-size_t)" in
+       8) echo 9223372036854775808;;
+       4) echo 2147483648;;
+       *) die "Unexpected ssize_t size: $(build_option sizeof-size_t)";;
+       esac
+}
+
+test_expect_success 'setup' '
+       HTTP_CONTENT_ENCODING="identity" &&
+       export HTTP_CONTENT_ENCODING &&
+       git config http.receivepack true &&
+       test_commit c0 &&
+       test_commit c1 &&
+       hash_head=$(git rev-parse HEAD) &&
+       hash_prev=$(git rev-parse HEAD~1) &&
+       printf "want %s" "$hash_head" | packetize >fetch_body &&
+       printf 0000 >>fetch_body &&
+       printf "have %s" "$hash_prev" | packetize >>fetch_body &&
+       printf done | packetize >>fetch_body &&
+       test_copy_bytes 10 <fetch_body >fetch_body.trunc &&
+       hash_next=$(git commit-tree -p HEAD -m next HEAD^{tree}) &&
+       printf "%s %s refs/heads/newbranch\\0report-status\\n" "$_z40" "$hash_next" | packetize >push_body &&
+       printf 0000 >>push_body &&
+       echo "$hash_next" | git pack-objects --stdout >>push_body &&
+       test_copy_bytes 10 <push_body >push_body.trunc &&
+       : >empty_body
+'
+
+test_expect_success GZIP 'setup, compression related' '
+       gzip -c fetch_body >fetch_body.gz &&
+       test_copy_bytes 10 <fetch_body.gz >fetch_body.gz.trunc &&
+       gzip -c push_body >push_body.gz &&
+       test_copy_bytes 10 <push_body.gz >push_body.gz.trunc
+'
+
+test_expect_success 'fetch plain' '
+       test_http_env upload fetch_body &&
+       verify_http_result "200 OK"
+'
+
+test_expect_success 'fetch plain truncated' '
+       test_http_env upload fetch_body.trunc &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success 'fetch plain empty' '
+       test_http_env upload empty_body &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success GZIP 'fetch gzipped' '
+       test_env HTTP_CONTENT_ENCODING="gzip" test_http_env upload fetch_body.gz &&
+       verify_http_result "200 OK"
+'
+
+test_expect_success GZIP 'fetch gzipped truncated' '
+       test_env HTTP_CONTENT_ENCODING="gzip" test_http_env upload fetch_body.gz.trunc &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success GZIP 'fetch gzipped empty' '
+       test_env HTTP_CONTENT_ENCODING="gzip" test_http_env upload empty_body &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success GZIP 'push plain' '
+       test_when_finished "git branch -D newbranch" &&
+       test_http_env receive push_body &&
+       verify_http_result "200 OK" &&
+       git rev-parse newbranch >act.head &&
+       echo "$hash_next" >exp.head &&
+       test_cmp act.head exp.head
+'
+
+test_expect_success 'push plain truncated' '
+       test_http_env receive push_body.trunc &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success 'push plain empty' '
+       test_http_env receive empty_body &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success GZIP 'push gzipped' '
+       test_when_finished "git branch -D newbranch" &&
+       test_env HTTP_CONTENT_ENCODING="gzip" test_http_env receive push_body.gz &&
+       verify_http_result "200 OK" &&
+       git rev-parse newbranch >act.head &&
+       echo "$hash_next" >exp.head &&
+       test_cmp act.head exp.head
+'
+
+test_expect_success GZIP 'push gzipped truncated' '
+       test_env HTTP_CONTENT_ENCODING="gzip" test_http_env receive push_body.gz.trunc &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success GZIP 'push gzipped empty' '
+       test_env HTTP_CONTENT_ENCODING="gzip" test_http_env receive empty_body &&
+       ! verify_http_result "200 OK"
+'
+
+test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
+       NOT_FIT_IN_SSIZE=$(ssize_b100dots) &&
+       env \
+               CONTENT_TYPE=application/x-git-upload-pack-request \
+               QUERY_STRING=/repo.git/git-upload-pack \
+               PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
+               GIT_HTTP_EXPORT_ALL=TRUE \
+               REQUEST_METHOD=POST \
+               CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
+               git http-backend </dev/zero >/dev/null 2>err &&
+       grep "fatal:.*CONTENT_LENGTH" err
+'
+
+test_done
diff --git a/t/t5562/invoke-with-content-length.pl b/t/t5562/invoke-with-content-length.pl
new file mode 100755 (executable)
index 0000000..6c2aae7
--- /dev/null
@@ -0,0 +1,37 @@
+#!/usr/bin/perl
+use 5.008;
+use strict;
+use warnings;
+
+my $body_filename = $ARGV[0];
+my @command = @ARGV[1 .. $#ARGV];
+
+# read data
+my $body_size = -s $body_filename;
+$ENV{"CONTENT_LENGTH"} = $body_size;
+open(my $body_fh, "<", $body_filename) or die "Cannot open $body_filename: $!";
+my $body_data;
+defined read($body_fh, $body_data, $body_size) or die "Cannot read $body_filename: $!";
+close($body_fh);
+
+my $exited = 0;
+$SIG{"CHLD"} = sub {
+        $exited = 1;
+};
+
+# write data
+my $pid = open(my $out, "|-", @command);
+{
+        # disable buffering at $out
+        my $old_selected = select;
+        select $out;
+        $| = 1;
+        select $old_selected;
+}
+print $out $body_data or die "Cannot write data: $!";
+
+sleep 60; # is interrupted by SIGCHLD
+if (!$exited) {
+        close($out);
+        die "Command did not exit after reading whole body";
+}