Merge branch 'js/packet-read-line-check-null'
authorJunio C Hamano <gitster@pobox.com>
Tue, 27 Feb 2018 18:33:54 +0000 (10:33 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 27 Feb 2018 18:33:55 +0000 (10:33 -0800)
Some low level protocol codepath could crash when they get an
unexpected flush packet, which is now fixed.

* js/packet-read-line-check-null:
always check for NULL return from packet_read_line()
correct error messages for NULL packet_read_line()

1  2 
fetch-pack.c
remote-curl.c
diff --combined fetch-pack.c
index 8253d746e0c40492e05360fe6f27bfa85bb39fbc,54acb421ae17c1ed03d8889b4aa4176a338c7d20..d97461296d5692521cd038f9ffa0b5f5c3c3f3dd
@@@ -29,7 -29,6 +29,7 @@@ static int deepen_not_ok
  static int fetch_fsck_objects = -1;
  static int transfer_fsck_objects = -1;
  static int agent_supported;
 +static int server_supports_filtering;
  static struct lock_file shallow_lock;
  static const char *alternate_shallow_file;
  
@@@ -261,8 -260,8 +261,8 @@@ static enum ack_type get_ack(int fd, st
        char *line = packet_read_line(fd, &len);
        const char *arg;
  
-       if (!len)
-               die(_("git fetch-pack: expected ACK/NAK, got EOF"));
+       if (!line)
+               die(_("git fetch-pack: expected ACK/NAK, got a flush packet"));
        if (!strcmp(line, "NAK"))
                return NAK;
        if (skip_prefix(line, "ACK ", &arg)) {
@@@ -380,8 -379,6 +380,8 @@@ static int find_common(struct fetch_pac
                        if (deepen_not_ok)      strbuf_addstr(&c, " deepen-not");
                        if (agent_supported)    strbuf_addf(&c, " agent=%s",
                                                            git_user_agent_sanitized());
 +                      if (args->filter_options.choice)
 +                              strbuf_addstr(&c, " filter");
                        packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
                        strbuf_release(&c);
                } else
                        packet_buf_write(&req_buf, "deepen-not %s", s->string);
                }
        }
 +      if (server_supports_filtering && args->filter_options.choice)
 +              packet_buf_write(&req_buf, "filter %s",
 +                               args->filter_options.filter_spec);
        packet_buf_flush(&req_buf);
        state_len = req_buf.len;
  
  
        flushes = 0;
        retval = -1;
 +      if (args->no_dependents)
 +              goto done;
        while ((oid = get_rev())) {
                packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid));
                print_verbose(args, "have %s", oid_to_hex(oid));
@@@ -717,7 -709,6 +717,7 @@@ static int everything_local(struct fetc
  {
        struct ref *ref;
        int retval;
 +      int old_save_commit_buffer = save_commit_buffer;
        timestamp_t cutoff = 0;
  
        save_commit_buffer = 0;
                }
        }
  
 -      if (!args->deepen) {
 -              for_each_ref(mark_complete_oid, NULL);
 -              for_each_cached_alternate(mark_alternate_complete);
 -              commit_list_sort_by_date(&complete);
 -              if (cutoff)
 -                      mark_recent_complete_commits(args, cutoff);
 -      }
 +      if (!args->no_dependents) {
 +              if (!args->deepen) {
 +                      for_each_ref(mark_complete_oid, NULL);
 +                      for_each_cached_alternate(mark_alternate_complete);
 +                      commit_list_sort_by_date(&complete);
 +                      if (cutoff)
 +                              mark_recent_complete_commits(args, cutoff);
 +              }
  
 -      /*
 -       * Mark all complete remote refs as common refs.
 -       * Don't mark them common yet; the server has to be told so first.
 -       */
 -      for (ref = *refs; ref; ref = ref->next) {
 -              struct object *o = deref_tag(lookup_object(ref->old_oid.hash),
 -                                           NULL, 0);
 +              /*
 +               * Mark all complete remote refs as common refs.
 +               * Don't mark them common yet; the server has to be told so first.
 +               */
 +              for (ref = *refs; ref; ref = ref->next) {
 +                      struct object *o = deref_tag(lookup_object(ref->old_oid.hash),
 +                                                   NULL, 0);
  
 -              if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE))
 -                      continue;
 +                      if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE))
 +                              continue;
  
 -              if (!(o->flags & SEEN)) {
 -                      rev_list_push((struct commit *)o, COMMON_REF | SEEN);
 +                      if (!(o->flags & SEEN)) {
 +                              rev_list_push((struct commit *)o, COMMON_REF | SEEN);
  
 -                      mark_common((struct commit *)o, 1, 1);
 +                              mark_common((struct commit *)o, 1, 1);
 +                      }
                }
        }
  
                print_verbose(args, _("already have %s (%s)"), oid_to_hex(remote),
                              ref->name);
        }
 +
 +      save_commit_buffer = old_save_commit_buffer;
 +
        return retval;
  }
  
@@@ -847,7 -833,7 +847,7 @@@ static int get_pack(struct fetch_pack_a
                argv_array_push(&cmd.args, alternate_shallow_file);
        }
  
 -      if (do_keep) {
 +      if (do_keep || args->from_promisor) {
                if (pack_lockfile)
                        cmd.out = -1;
                cmd_name = "index-pack";
                        argv_array_push(&cmd.args, "-v");
                if (args->use_thin_pack)
                        argv_array_push(&cmd.args, "--fix-thin");
 -              if (args->lock_pack || unpack_limit) {
 +              if (do_keep && (args->lock_pack || unpack_limit)) {
                        char hostname[HOST_NAME_MAX + 1];
                        if (xgethostname(hostname, sizeof(hostname)))
                                xsnprintf(hostname, sizeof(hostname), "localhost");
                }
                if (args->check_self_contained_and_connected)
                        argv_array_push(&cmd.args, "--check-self-contained-and-connected");
 +              if (args->from_promisor)
 +                      argv_array_push(&cmd.args, "--promisor");
        }
        else {
                cmd_name = "unpack-objects";
@@@ -980,13 -964,6 +980,13 @@@ static struct ref *do_fetch_pack(struc
        else
                prefer_ofs_delta = 0;
  
 +      if (server_supports("filter")) {
 +              server_supports_filtering = 1;
 +              print_verbose(args, _("Server supports filter"));
 +      } else if (args->filter_options.choice) {
 +              warning("filtering not recognized by server, ignoring");
 +      }
 +
        if ((agent_feature = server_feature_value("agent", &agent_len))) {
                agent_supported = 1;
                if (agent_len)
diff --combined remote-curl.c
index 6ec535243565d4b1a50e22868966b4aec33776f8,99030774713221d605a2731ca2f3fa46d9115631..e11e619d0da8a3b0748ca1af7d58bce07aaf7431
@@@ -24,7 -24,6 +24,7 @@@ struct options 
        char *deepen_since;
        struct string_list deepen_not;
        struct string_list push_options;
 +      char *filter;
        unsigned progress : 1,
                check_self_contained_and_connected : 1,
                cloning : 1,
@@@ -34,9 -33,7 +34,9 @@@
                thin : 1,
                /* One of the SEND_PACK_PUSH_CERT_* constants. */
                push_cert : 2,
 -              deepen_relative : 1;
 +              deepen_relative : 1,
 +              from_promisor : 1,
 +              no_dependents : 1;
  };
  static struct options options;
  static struct string_list cas_options = STRING_LIST_INIT_DUP;
@@@ -160,15 -157,6 +160,15 @@@ static int set_option(const char *name
                        return -1;
                return 0;
  #endif /* LIBCURL_VERSION_NUM >= 0x070a08 */
 +      } else if (!strcmp(name, "from-promisor")) {
 +              options.from_promisor = 1;
 +              return 0;
 +      } else if (!strcmp(name, "no-dependents")) {
 +              options.no_dependents = 1;
 +              return 0;
 +      } else if (!strcmp(name, "filter")) {
 +              options.filter = xstrdup(value);;
 +              return 0;
        } else {
                return 1 /* unsupported */;
        }
@@@ -351,6 -339,8 +351,8 @@@ static struct discovery *discover_refs(
                 * pkt-line matches our request.
                 */
                line = packet_read_line_buf(&last->buf, &last->len, NULL);
+               if (!line)
+                       die("invalid server response; expected service, got flush packet");
  
                strbuf_reset(&exp);
                strbuf_addf(&exp, "# service=%s", service);
@@@ -834,12 -824,6 +836,12 @@@ static int fetch_git(struct discovery *
                                 options.deepen_not.items[i].string);
        if (options.deepen_relative && options.depth)
                argv_array_push(&args, "--deepen-relative");
 +      if (options.from_promisor)
 +              argv_array_push(&args, "--from-promisor");
 +      if (options.no_dependents)
 +              argv_array_push(&args, "--no-dependents");
 +      if (options.filter)
 +              argv_array_pushf(&args, "--filter=%s", options.filter);
        argv_array_push(&args, url.buf);
  
        for (i = 0; i < nr_heads; i++) {