protocol-caps: check object existence regardless of the attributes requested

Currently, send_info() only checks for existence when the attribute
'size' is also requested. Requesting a bare OID, without attributes only
echoes back the OID.

Extract the existence check to be done regardless of the number of
attributes requested.

While at it, introduce a wrapper called get_object_info() similar to
odb_read_object_info() that returns OBJ_BAD on fail and adds
OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK flags.
OBJECT_INFO_SKIP_FETCH_OBJECT is so a server with a partial clone
doesn't trigger fetching objects when it gets an object-info request
with an OID that is not available locally. A server should only report
what it has locally.

Tighten the condition used to determine whether an object is
recognized. get_object_info() returns OBJ_BAD for unknown objects,
but OBJ_NONE (0) can also mean "not found". Change the check from '< 0'
to '<= OBJ_NONE' to cover both as unrecognized.

With this patch, a bare OID has two possible responses:

1. Recognized OID: the server answers with "<OID>"

2. Unrecognized OID: the server answers with "<OID> SP"

Update the object-info section in 'gitprotocol-v2.adoc':
- Require full obj-oid explicitly.
- Fix parentheses.
- Define obj-size explicitly.
- Make obj-size optional in obj-info and document the behavior
  for unrecognized object IDs.
- Describe the attr header as zero or more pkt-lines, one per attribute,
  matching what the server implements. A request with no attributes gets
  no header.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index 2beb70595fc1e5f163016941a564c7bf552041f3..7bf62014c3917d59d9d0a5818507736a1c2d8579 100644 (file)
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -568,21 +568,26 @@ An `object-info` request takes the following arguments:
 
        oid <oid>
        Indicates to the server an object which the client wants to obtain
-       information for.
+       information for. They must be full OIDs.
 
-The response of `object-info` is a list of the requested object ids
-and associated requested information, each separated by a single space.
+The response of `object-info` consists of one pkt-line per requested attribute,
+echoing the attributes the server will report, followed by one pkt-line per
+requested object id with its information, each field separated by a single
+space.
 
        output = info flush-pkt
 
-       info = PKT-LINE(attrs) LF)
-               *PKT-LINE(obj-info LF)
-
-       attrs = attr | attrs SP attrs
+       info = *PKT-LINE(attr LF)
+              *PKT-LINE(obj-info LF)
 
        attr = "size"
 
-       obj-info = obj-id SP obj-size
+       obj-size = 1*DIGIT
+
+       obj-info = obj-id [SP [obj-size]]
+
+If the server does not recognize the OID, the response will be `<oid> SP`
+regardless of the number of attributes requested.
 
 bundle-uri
 ~~~~~~~~~~

diff --git a/protocol-caps.c b/protocol-caps.c
index 8858ea4489d71448aa71d2089aedda9badf44ea0..02261be14d817ae12f08d28ba15baa83f750426a 100644 (file)
--- a/protocol-caps.c
+++ b/protocol-caps.c
@@ -30,6 +30,32 @@ static int parse_oid(const char *line, struct string_list *oid_str_list)
        return 1;
 }
 
+/*
+ * odb_read_object_info_extended() wrapper. Similar to odb_read_object_info()
+ * but uses the flags:
+ *
+ * - OBJECT_INFO_SKIP_FETCH_OBJECT so a server won't fetch an object when a
+ *   object-info request asks for an OID that it doesn't have.
+ *
+ * - OBJECT_INFO_QUICK to avoid re-scanning packs when the object is not found.
+ */
+static enum object_type get_object_info(struct object_database *odb,
+                          const struct object_id *oid,
+                          size_t *sizep)
+{
+       enum object_type type;
+       struct object_info oi = OBJECT_INFO_INIT;
+
+       oi.typep = &type;
+       oi.sizep = sizep;
+       if (odb_read_object_info_extended(odb, oid, &oi,
+                                         OBJECT_INFO_LOOKUP_REPLACE |
+                                         OBJECT_INFO_SKIP_FETCH_OBJECT |
+                                         OBJECT_INFO_QUICK) < 0)
+               return OBJ_BAD;
+       return type;
+}
+
 /*
  * Validates and send requested info back to the client. Any errors detected
  * are returned as they are detected.
@@ -62,15 +88,22 @@ static void send_info(struct repository *r, struct packet_writer *writer,
 
                strbuf_addstr(&send_buffer, oid_str);
 
+               /*
+                * Check the existence of the object first.
+                * If an object is not recognized by the server append SP to
+                * the response.
+                */
+               if (get_object_info(r->objects, &oid, &object_size) <= OBJ_NONE) {
+                       strbuf_addstr(&send_buffer, " ");
+                       goto write;
+               }
+
                if (info->size) {
-                       if (odb_read_object_info(r->objects, &oid, &object_size) < 0) {
-                               strbuf_addstr(&send_buffer, " ");
-                       } else {
-                               strbuf_addf(&send_buffer, " %"PRIuMAX,
-                                           (uintmax_t)object_size);
-                       }
+                       strbuf_addf(&send_buffer, " %"PRIuMAX,
+                                   (uintmax_t)object_size);
                }
 
+write:
                packet_writer_write(writer, "%s", send_buffer.buf);
                strbuf_reset(&send_buffer);
        }

diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index d4c28bae39e2adcc95162ea45cdacd25cf2ced63..cacff4456c6fea23f913f4141b9fb2b9c00fa338 100755 (executable)
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -7,6 +7,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+unknown_oid=$(printf "test" | git hash-object --stdin)
+
 test_expect_success 'setup to generate files with expected content' '
        printf "agent=git/%s" "$(git version | cut -d" " -f3)" >agent_capability &&
 
@@ -364,6 +366,67 @@ test_expect_success 'basics of object-info' '
        test_cmp expect actual
 '
 
+test_expect_success 'bare OID request' '
+       test_config transfer.advertiseObjectInfo true &&
+
+       test-tool pkt-line pack >in <<-EOF &&
+       command=object-info
+       object-format=$(test_oid algo)
+       0001
+       oid $(git rev-parse two:two.t)
+       0000
+       EOF
+
+       cat >expect <<-EOF &&
+       $(git rev-parse two:two.t)
+       0000
+       EOF
+
+       test-tool serve-v2 --stateless-rpc <in >out &&
+       test-tool pkt-line unpack <out >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'object-info with bare unrecognized OID' '
+       test_config transfer.advertiseObjectInfo true &&
+
+       test-tool pkt-line pack >in <<-EOF &&
+       command=object-info
+       object-format=$(test_oid algo)
+       0001
+       oid $unknown_oid
+       0000
+       EOF
+
+       printf "%s \n" "$unknown_oid" >expect &&
+       printf "0000\n" >>expect &&
+
+       test-tool serve-v2 --stateless-rpc <in >out &&
+       test-tool pkt-line unpack <out >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'object-info with size for unrecognized OID' '
+       test_config transfer.advertiseObjectInfo true &&
+
+       test-tool pkt-line pack >in <<-EOF &&
+       command=object-info
+       object-format=$(test_oid algo)
+       0001
+       size
+       oid $unknown_oid
+       0000
+       EOF
+
+       printf "size\n" >expect &&
+       printf "%s \n" "$unknown_oid" >>expect &&
+       printf "0000\n" >>expect &&
+
+       test-tool serve-v2 --stateless-rpc <in >out &&
+       test-tool pkt-line unpack <out >actual &&
+       test_cmp expect actual
+'
+
 test_expect_success 'test capability advertisement with uploadpack.advertiseBundleURIs' '
        test_config uploadpack.advertiseBundleURIs true &&