cat-file: validate remote atoms with an allow-list
`strstr()` is not enough to validate the format placeholders in `remote-object-info` causing two errors: 1. Atoms recognized by `expand_atom()` but the remote doesn't returns 1, but `data->type` contains garbage causing segfault. 2. `expand_atom()` returns 0 for unknown atoms, calling `strbuf_expand_bad_format()` which ends up dying, blocking local queries if the same format is shared. Add an allow-list with the supported atoms at the top of `expand_atom()`. In remote mode, unsupported atoms return 1 leaving the buffer empty, honoring how `for-each-ref` handles known but inapplicable atoms. As extra safety, initialize `data->type` to `OBJ_BAD` and add a `NULL` check for `type_name()` so uninitialized data doesn't cause segfault. Update tests that expect previous `die()` behavior to expect an empty string and add an explicit test for empty string return on unknown placeholder. Update cat-file command documentation regarding `remote-object-info`. Mentored-by: Karthik Nayak <karthik.188@gmail.com> Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>seen
parent
7134a94db6
commit
03e2e84446
|
|
@ -451,7 +451,7 @@ CAVEATS
|
|||
|
||||
Note that since only `%(objectname)` and `%(objectsize)` are currently
|
||||
supported by the `remote-object-info` command. Using any other placeholder in
|
||||
the format string will raise an error.
|
||||
the format string will return an empty string in its position.
|
||||
|
||||
Note that the sizes of objects on disk are reported accurately, but care
|
||||
should be taken in drawing conclusions about which refs or objects are
|
||||
|
|
|
|||
|
|
@ -336,8 +336,18 @@ struct expand_data {
|
|||
* optimized out.
|
||||
*/
|
||||
unsigned skip_object_info : 1;
|
||||
|
||||
/*
|
||||
* Flags about when an object info is being fetched from remote.
|
||||
*/
|
||||
unsigned is_remote:1;
|
||||
};
|
||||
#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
|
||||
|
||||
static const char *remote_object_info_atoms[] = {
|
||||
"objectname",
|
||||
"objectsize",
|
||||
};
|
||||
#define EXPAND_DATA_INIT { .mode = S_IFINVALID }
|
||||
|
||||
static int is_atom(const char *atom, const char *s, int slen)
|
||||
{
|
||||
|
|
@ -348,14 +358,31 @@ static int is_atom(const char *atom, const char *s, int slen)
|
|||
static int expand_atom(struct strbuf *sb, const char *atom, int len,
|
||||
struct expand_data *data)
|
||||
{
|
||||
if (data->is_remote) {
|
||||
size_t i, allowed_nr = ARRAY_SIZE(remote_object_info_atoms);
|
||||
for (i = 0; i < allowed_nr; i++)
|
||||
if (is_atom(remote_object_info_atoms[i], atom, len))
|
||||
break;
|
||||
|
||||
/*
|
||||
* On remote, skip unsupported atoms returning an empty sb,
|
||||
* honoring how for-each-ref handles known but inapplicable
|
||||
* atoms (e.g. %(tagger)).
|
||||
*/
|
||||
if (i == allowed_nr)
|
||||
return 1;
|
||||
}
|
||||
|
||||
if (is_atom("objectname", atom, len)) {
|
||||
if (!data->mark_query)
|
||||
strbuf_add_oid_hex(sb, &data->oid);
|
||||
} else if (is_atom("objecttype", atom, len)) {
|
||||
if (data->mark_query)
|
||||
if (data->mark_query) {
|
||||
data->info.typep = &data->type;
|
||||
else
|
||||
strbuf_addstr(sb, type_name(data->type));
|
||||
} else {
|
||||
const char *t = type_name(data->type);
|
||||
strbuf_addstr(sb, t ? t : "");
|
||||
}
|
||||
} else if (is_atom("objectsize", atom, len)) {
|
||||
if (data->mark_query)
|
||||
data->info.sizep = &data->size;
|
||||
|
|
@ -711,10 +738,6 @@ static int get_remote_info(struct batch_options *opt,
|
|||
CALLOC_ARRAY(*remote_object_info, object_info_oids->nr);
|
||||
gtransport->smart_options->object_info_oids = object_info_oids;
|
||||
|
||||
/* 'objectsize' is the only option currently supported */
|
||||
if (!strstr(opt->format, "%(objectsize)"))
|
||||
die(_("%s is currently not supported with remote-object-info"), opt->format);
|
||||
|
||||
string_list_append(&object_info_options, "size");
|
||||
|
||||
if (object_info_options.nr > 0) {
|
||||
|
|
@ -844,7 +867,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
|
|||
*/
|
||||
data->size = *remote_object_info[i].sizep;
|
||||
opt->batch_mode = BATCH_MODE_INFO;
|
||||
data->is_remote = 1;
|
||||
batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
|
||||
data->is_remote = 0;
|
||||
} else {
|
||||
report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -236,6 +236,21 @@ test_expect_success 'remote-object-info does not die on missing oid like info' '
|
|||
)
|
||||
'
|
||||
|
||||
# This tests depends on %(objecttype) not being supported yet, once supported
|
||||
# it needs to be updated.
|
||||
test_expect_success 'unsupported placeholder on remote returns empty string' '
|
||||
(
|
||||
set_transport_variables "$daemon_parent" &&
|
||||
cd "$daemon_parent/daemon_client_empty" &&
|
||||
|
||||
echo "" >expect &&
|
||||
git cat-file --batch-command="%(objecttype)" >actual <<-EOF &&
|
||||
remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid
|
||||
EOF
|
||||
test_cmp expect actual
|
||||
)
|
||||
'
|
||||
|
||||
# Test --batch-command remote-object-info with 'git://' and
|
||||
# transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability
|
||||
test_expect_success 'batch-command remote-object-info git:// fails when transfer.advertiseobjectinfo=false' '
|
||||
|
|
@ -575,10 +590,12 @@ test_expect_success 'remote-object-info fails on unsupported filter option (obje
|
|||
set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
|
||||
cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
|
||||
|
||||
test_must_fail git cat-file --batch-command="%(objectsize:disk)" 2>err <<-EOF &&
|
||||
echo "$hello_oid " >expect &&
|
||||
|
||||
git cat-file --batch-command="%(objectname) %(objectsize:disk)" >actual <<-EOF &&
|
||||
remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
|
||||
EOF
|
||||
test_grep "%(objectsize:disk) is currently not supported with remote-object-info" err
|
||||
test_cmp expect actual
|
||||
)
|
||||
'
|
||||
|
||||
|
|
@ -587,10 +604,12 @@ test_expect_success 'remote-object-info fails on unsupported filter option (delt
|
|||
set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
|
||||
cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
|
||||
|
||||
test_must_fail git cat-file --batch-command="%(deltabase)" 2>err <<-EOF &&
|
||||
echo "" >expect &&
|
||||
|
||||
git cat-file --batch-command="%(deltabase)" >actual <<-EOF &&
|
||||
remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
|
||||
EOF
|
||||
test_grep "%(deltabase) is currently not supported with remote-object-info" err
|
||||
test_cmp expect actual
|
||||
)
|
||||
'
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue