[PATCH GSoC v19 09/13] protocol-caps: check object existence regardless of the attributes requested

[Pablo Sabater <pabloosabaterr@gmail.com>]

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>
---
 Documentation/gitprotocol-v2.adoc | 21 ++++++++-----
 protocol-caps.c                   | 45 ++++++++++++++++++++++++----
 t/t5701-git-serve.sh              | 63 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 14 deletions(-)

diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index 2beb70595f..7bf62014c3 100644
--- 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 8858ea4489..02261be14d 100644
--- 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 d4c28bae39..cacff4456c 100755
--- 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 &&
 

-- 
2.54.0