Merge branch 'jk/fix-remote-curl-url-wo-proto'
authorJunio C Hamano <gitster@pobox.com>
Thu, 15 Sep 2016 21:11:15 +0000 (14:11 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 15 Sep 2016 21:11:15 +0000 (14:11 -0700)
"git fetch http::/site/path" did not die correctly and segfaulted
instead.

* jk/fix-remote-curl-url-wo-proto:
remote-curl: handle URLs without protocol

1  2 
http.c
t/t5550-http-fetch-dumb.sh
diff --combined http.c
index cd40b012f89397db67e851101ab38c2c57fd18c9,057f250b9c4323744587f28a64ad432a7c1bcc93..edce47ca75fbff3b0c8a6f15a881b7471f55fa83
--- 1/http.c
--- 2/http.c
+++ b/http.c
@@@ -11,7 -11,6 +11,7 @@@
  #include "gettext.h"
  #include "transport.h"
  
 +static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
  #if LIBCURL_VERSION_NUM >= 0x070a08
  long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
  #else
@@@ -115,7 -114,6 +115,7 @@@ static unsigned long http_auth_methods 
  
  static struct curl_slist *pragma_header;
  static struct curl_slist *no_pragma_header;
 +static struct curl_slist *extra_http_headers;
  
  static struct active_request_slot *active_queue_head;
  
@@@ -325,19 -323,6 +325,19 @@@ static int http_options(const char *var
  #endif
        }
  
 +      if (!strcmp("http.extraheader", var)) {
 +              if (!value) {
 +                      return config_error_nonbool(var);
 +              } else if (!*value) {
 +                      curl_slist_free_all(extra_http_headers);
 +                      extra_http_headers = NULL;
 +              } else {
 +                      extra_http_headers =
 +                              curl_slist_append(extra_http_headers, value);
 +              }
 +              return 0;
 +      }
 +
        /* Fall back on the default ones */
        return git_default_config(var, value, cb);
  }
@@@ -461,7 -446,8 +461,7 @@@ static int sockopt_callback(void *clien
  
        rc = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&ka, len);
        if (rc < 0)
 -              warning("unable to set SO_KEEPALIVE on socket %s",
 -                      strerror(errno));
 +              warning_errno("unable to set SO_KEEPALIVE on socket");
  
        return 0; /* CURL_SOCKOPT_OK only exists since curl 7.21.5 */
  }
@@@ -478,125 -464,6 +478,125 @@@ static void set_curl_keepalive(CURL *c
  }
  #endif
  
 +static void redact_sensitive_header(struct strbuf *header)
 +{
 +      const char *sensitive_header;
 +
 +      if (skip_prefix(header->buf, "Authorization:", &sensitive_header) ||
 +          skip_prefix(header->buf, "Proxy-Authorization:", &sensitive_header)) {
 +              /* The first token is the type, which is OK to log */
 +              while (isspace(*sensitive_header))
 +                      sensitive_header++;
 +              while (*sensitive_header && !isspace(*sensitive_header))
 +                      sensitive_header++;
 +              /* Everything else is opaque and possibly sensitive */
 +              strbuf_setlen(header,  sensitive_header - header->buf);
 +              strbuf_addstr(header, " <redacted>");
 +      }
 +}
 +
 +static void curl_dump_header(const char *text, unsigned char *ptr, size_t size, int hide_sensitive_header)
 +{
 +      struct strbuf out = STRBUF_INIT;
 +      struct strbuf **headers, **header;
 +
 +      strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
 +              text, (long)size, (long)size);
 +      trace_strbuf(&trace_curl, &out);
 +      strbuf_reset(&out);
 +      strbuf_add(&out, ptr, size);
 +      headers = strbuf_split_max(&out, '\n', 0);
 +
 +      for (header = headers; *header; header++) {
 +              if (hide_sensitive_header)
 +                      redact_sensitive_header(*header);
 +              strbuf_insert((*header), 0, text, strlen(text));
 +              strbuf_insert((*header), strlen(text), ": ", 2);
 +              strbuf_rtrim((*header));
 +              strbuf_addch((*header), '\n');
 +              trace_strbuf(&trace_curl, (*header));
 +      }
 +      strbuf_list_free(headers);
 +      strbuf_release(&out);
 +}
 +
 +static void curl_dump_data(const char *text, unsigned char *ptr, size_t size)
 +{
 +      size_t i;
 +      struct strbuf out = STRBUF_INIT;
 +      unsigned int width = 60;
 +
 +      strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
 +              text, (long)size, (long)size);
 +      trace_strbuf(&trace_curl, &out);
 +
 +      for (i = 0; i < size; i += width) {
 +              size_t w;
 +
 +              strbuf_reset(&out);
 +              strbuf_addf(&out, "%s: ", text);
 +              for (w = 0; (w < width) && (i + w < size); w++) {
 +                      unsigned char ch = ptr[i + w];
 +
 +                      strbuf_addch(&out,
 +                                     (ch >= 0x20) && (ch < 0x80)
 +                                     ? ch : '.');
 +              }
 +              strbuf_addch(&out, '\n');
 +              trace_strbuf(&trace_curl, &out);
 +      }
 +      strbuf_release(&out);
 +}
 +
 +static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp)
 +{
 +      const char *text;
 +      enum { NO_FILTER = 0, DO_FILTER = 1 };
 +
 +      switch (type) {
 +      case CURLINFO_TEXT:
 +              trace_printf_key(&trace_curl, "== Info: %s", data);
 +      default:                /* we ignore unknown types by default */
 +              return 0;
 +
 +      case CURLINFO_HEADER_OUT:
 +              text = "=> Send header";
 +              curl_dump_header(text, (unsigned char *)data, size, DO_FILTER);
 +              break;
 +      case CURLINFO_DATA_OUT:
 +              text = "=> Send data";
 +              curl_dump_data(text, (unsigned char *)data, size);
 +              break;
 +      case CURLINFO_SSL_DATA_OUT:
 +              text = "=> Send SSL data";
 +              curl_dump_data(text, (unsigned char *)data, size);
 +              break;
 +      case CURLINFO_HEADER_IN:
 +              text = "<= Recv header";
 +              curl_dump_header(text, (unsigned char *)data, size, NO_FILTER);
 +              break;
 +      case CURLINFO_DATA_IN:
 +              text = "<= Recv data";
 +              curl_dump_data(text, (unsigned char *)data, size);
 +              break;
 +      case CURLINFO_SSL_DATA_IN:
 +              text = "<= Recv SSL data";
 +              curl_dump_data(text, (unsigned char *)data, size);
 +              break;
 +      }
 +      return 0;
 +}
 +
 +void setup_curl_trace(CURL *handle)
 +{
 +      if (!trace_want(&trace_curl))
 +              return;
 +      curl_easy_setopt(handle, CURLOPT_VERBOSE, 1L);
 +      curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
 +      curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
 +}
 +
 +
  static CURL *get_curl_handle(void)
  {
        CURL *result = curl_easy_init();
                warning("protocol restrictions not applied to curl redirects because\n"
                        "your curl version is too old (>= 7.19.4)");
  #endif
 -
        if (getenv("GIT_CURL_VERBOSE"))
 -              curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
 +              curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
 +      setup_curl_trace(result);
  
        curl_easy_setopt(result, CURLOPT_USERAGENT,
                user_agent ? user_agent : git_user_agent());
         * precedence here, as in CURL.
         */
        if (!curl_http_proxy) {
-               if (!strcmp(http_auth.protocol, "https")) {
+               if (http_auth.protocol && !strcmp(http_auth.protocol, "https")) {
                        var_override(&curl_http_proxy, getenv("HTTPS_PROXY"));
                        var_override(&curl_http_proxy, getenv("https_proxy"));
                } else {
@@@ -811,10 -678,8 +811,10 @@@ void http_init(struct remote *remote, c
        if (remote)
                var_override(&http_proxy_authmethod, remote->http_proxy_authmethod);
  
 -      pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
 -      no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
 +      pragma_header = curl_slist_append(http_copy_default_headers(),
 +              "Pragma: no-cache");
 +      no_pragma_header = curl_slist_append(http_copy_default_headers(),
 +              "Pragma:");
  
  #ifdef USE_CURL_MULTI
        {
@@@ -900,9 -765,6 +900,9 @@@ void http_cleanup(void
  #endif
        curl_global_cleanup();
  
 +      curl_slist_free_all(extra_http_headers);
 +      extra_http_headers = NULL;
 +
        curl_slist_free_all(pragma_header);
        pragma_header = NULL;
  
@@@ -1225,7 -1087,7 +1225,7 @@@ void append_remote_object_url(struct st
  
        strbuf_addf(buf, "objects/%.*s/", 2, hex);
        if (!only_two_digit_prefix)
 -              strbuf_addf(buf, "%s", hex+2);
 +              strbuf_addstr(buf, hex + 2);
  }
  
  char *get_remote_object_url(const char *url, const char *hex,
@@@ -1301,16 -1163,6 +1301,16 @@@ int run_one_slot(struct active_request_
        return handle_curl_result(results);
  }
  
 +struct curl_slist *http_copy_default_headers(void)
 +{
 +      struct curl_slist *headers = NULL, *h;
 +
 +      for (h = extra_http_headers; h; h = h->next)
 +              headers = curl_slist_append(headers, h->data);
 +
 +      return headers;
 +}
 +
  static CURLcode curlinfo_strbuf(CURL *curl, CURLINFO info, struct strbuf *buf)
  {
        char *ptr;
@@@ -1528,7 -1380,7 +1528,7 @@@ static int http_request(const char *url
  {
        struct active_request_slot *slot;
        struct slot_results results;
 -      struct curl_slist *headers = NULL;
 +      struct curl_slist *headers = http_copy_default_headers();
        struct strbuf buf = STRBUF_INIT;
        const char *accept_language;
        int ret;
@@@ -1975,19 -1827,8 +1975,19 @@@ static size_t fwrite_sha1_file(char *pt
        unsigned char expn[4096];
        size_t size = eltsize * nmemb;
        int posn = 0;
 -      struct http_object_request *freq =
 -              (struct http_object_request *)data;
 +      struct http_object_request *freq = data;
 +      struct active_request_slot *slot = freq->slot;
 +
 +      if (slot) {
 +              CURLcode c = curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE,
 +                                              &slot->http_code);
 +              if (c != CURLE_OK)
 +                      die("BUG: curl_easy_getinfo for HTTP code failed: %s",
 +                              curl_easy_strerror(c));
 +              if (slot->http_code >= 400)
 +                      return size;
 +      }
 +
        do {
                ssize_t retval = xwrite(freq->localfile,
                                        (char *) ptr + posn, size - posn);
@@@ -2053,7 -1894,8 +2053,7 @@@ struct http_object_request *new_http_ob
        }
  
        if (freq->localfile < 0) {
 -              error("Couldn't create temporary file %s: %s",
 -                    freq->tmpfile, strerror(errno));
 +              error_errno("Couldn't create temporary file %s", freq->tmpfile);
                goto abort;
        }
  
                        prev_posn = 0;
                        lseek(freq->localfile, 0, SEEK_SET);
                        if (ftruncate(freq->localfile, 0) < 0) {
 -                              error("Couldn't truncate temporary file %s: %s",
 -                                        freq->tmpfile, strerror(errno));
 +                              error_errno("Couldn't truncate temporary file %s",
 +                                          freq->tmpfile);
                                goto abort;
                        }
                }
        freq->slot = get_active_slot();
  
        curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);
 +      curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
        curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
        curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
        curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
index dc9b87d6b41efc472eac1a2b768bf56c01caf4d1,9249140f99890f67d57d689884ba2975bb1bd39f..7641417b4a3848fa9d065572e5a8248fea5c7574
@@@ -91,55 -91,6 +91,55 @@@ test_expect_success 'configured usernam
        expect_askpass pass user@host
  '
  
 +test_expect_success 'set up repo with http submodules' '
 +      git init super &&
 +      set_askpass user@host pass@host &&
 +      (
 +              cd super &&
 +              git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
 +              git commit -m "add submodule"
 +      )
 +'
 +
 +test_expect_success 'cmdline credential config passes to submodule via clone' '
 +      set_askpass wrong pass@host &&
 +      test_must_fail git clone --recursive super super-clone &&
 +      rm -rf super-clone &&
 +
 +      set_askpass wrong pass@host &&
 +      git -c "credential.$HTTPD_URL.username=user@host" \
 +              clone --recursive super super-clone &&
 +      expect_askpass pass user@host
 +'
 +
 +test_expect_success 'cmdline credential config passes submodule via fetch' '
 +      set_askpass wrong pass@host &&
 +      test_must_fail git -C super-clone fetch --recurse-submodules &&
 +
 +      set_askpass wrong pass@host &&
 +      git -C super-clone \
 +          -c "credential.$HTTPD_URL.username=user@host" \
 +          fetch --recurse-submodules &&
 +      expect_askpass pass user@host
 +'
 +
 +test_expect_success 'cmdline credential config passes submodule update' '
 +      # advance the submodule HEAD so that a fetch is required
 +      git commit --allow-empty -m foo &&
 +      git push "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git" HEAD &&
 +      sha1=$(git rev-parse HEAD) &&
 +      git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&
 +
 +      set_askpass wrong pass@host &&
 +      test_must_fail git -C super-clone submodule update &&
 +
 +      set_askpass wrong pass@host &&
 +      git -C super-clone \
 +          -c "credential.$HTTPD_URL.username=user@host" \
 +          submodule update &&
 +      expect_askpass pass user@host
 +'
 +
  test_expect_success 'fetch changes via http' '
        echo content >>file &&
        git commit -a -m two &&
@@@ -263,15 -214,15 +263,15 @@@ check_language () 
                >expect
                ;;
        ?*)
 -              echo "Accept-Language: $1" >expect
 +              echo "=> Send header: Accept-Language: $1" >expect
                ;;
        esac &&
 -      GIT_CURL_VERBOSE=1 \
 +      GIT_TRACE_CURL=true \
        LANGUAGE=$2 \
        git ls-remote "$HTTPD_URL/dumb/repo.git" >output 2>&1 &&
        tr -d '\015' <output |
        sort -u |
 -      sed -ne '/^Accept-Language:/ p' >actual &&
 +      sed -ne '/^=> Send header: Accept-Language:/ p' >actual &&
        test_cmp expect actual
  }
  
@@@ -295,9 -246,17 +295,17 @@@ ja;q=0.95, zh;q=0.94, sv;q=0.93, pt;q=0
  '
  
  test_expect_success 'git client does not send an empty Accept-Language' '
 -      GIT_CURL_VERBOSE=1 LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr &&
 -      ! grep "^Accept-Language:" stderr
 +      GIT_TRACE_CURL=true LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr &&
 +      ! grep "^=> Send header: Accept-Language:" stderr
  '
  
+ test_expect_success 'remote-http complains cleanly about malformed urls' '
+       # do not actually issue "list" or other commands, as we do not
+       # want to rely on what curl would actually do with such a broken
+       # URL. This is just about making sure we do not segfault during
+       # initialization.
+       test_must_fail git remote-http http::/example.com/repo.git
+ '
  stop_httpd
  test_done