Pablo Sabater [off-list ref] writes:

The static allow-list in expand_atom() is hardcoded to only allow
"objectname" and "objectsize" for remote queries. This works because
up to this point all servers will either support object-info with name
and size or they do not support them at all, but we cannot expect that
in a future different servers with different git versions to have the
same object-info capabilities. Therefore, the allow_list needs to be
dynamic depending on what the server advertises.

The client will now:

1. Request the protocol option that the placeholder refers to (i.e.
   "size" when "%(objectsize)").

"when" -> "for"?

2. Filters the request in fetch_object_info() dropping any option that
   the server does not advertise.

3. After the fetching, the options that haven't been dropped are the ones
   fetched and supported by the server, these supported options are
   mapped and remote_allowed_atoms is populated with the placeholders.

4. expand_atom() checks remote_allowed_atoms with the same behaviour as
   the static allow_list had.

I am not sure I follow the above entirely.  Could you add a
concrete example to the commit message?

For instance, if the client wants "%(objectsize) %(objectcolor)" and
the server only supports 'size' but not 'color', the filtering in
step (2) prevents the client from asking about the color, requesting
only the size instead.  When the server says the size is 42, step (3)
uses that to substitute '%(objectsize)'.  Would the end result then
be "42 %(objectcolor)"?

-static const char *remote_object_info_atoms[] = {
-	"objectname",
-	"objectsize",
+	struct string_list remote_allowed_atoms;
 };
+#define EXPAND_DATA_INIT  { .mode = S_IFINVALID, .type = OBJ_BAD, \
+			    .remote_allowed_atoms = STRING_LIST_INIT_NODUP }

Hmph, is this list expected to change over time?  One-line-per-item
format would be more suited for updates if it is the case.

This is not a new issue, but if the caller does not ask for
anything in object_info_options, no call to
transport_fetch_object_info() is made here.  This is so even
though we went through quite a lot of work, including the
connection establishment and teardown below.

By failing to contact the remote side, we wouldn't even know if
the objects being queried actually exist there, which is
probably even worse.

And if the request is only for "%(objectname)", an empty
object_info_options is given to get_remote_info().

 	if (get_remote_info(opt, count, argv, &remote_object_info,
-			    &object_info_oids))
+			    &object_info_oids, &object_info_options))
 		goto cleanup;