undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'jk/reduce-malloc-in-v2-servers'

Browse Source 
Code cleanup to limit memory consumption and tighten protocol
message parsing.

* jk/reduce-malloc-in-v2-servers:
  ls-refs: reject unknown arguments
  serve: reject commands used as capabilities
  serve: reject bogus v2 "command=ls-refs=foo"
  docs/protocol-v2: clarify some ls-refs ref-prefix details
  ls-refs: ignore very long ref-prefix counts
  serve: drop "keys" strvec
  serve: provide "receive" function for session-id capability
  serve: provide "receive" function for object-format capability
  serve: add "receive" method for v2 capabilities table
  serve: return capability "value" from get_capability()
  serve: rename is_command() to parse_command()
This commit is contained in:
Junio C Hamano 2021-09-28 13:06:53 -07:00
commit bb1677fc29
4 changed files with 164 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Documentation/technical/protocol-v2.txt
View File

@ -199,7 +199,11 @@ ls-refs takes in the following arguments:
Show peeled tags.
ref-prefix <prefix>
When specified, only references having a prefix matching one of
the provided prefixes are displayed.
the provided prefixes are displayed. Multiple instances may be
given, in which case references matching any prefix will be
shown. Note that this is purely for optimization; a server MAY
show refs not matching the prefix if it chooses, and clients
should filter the result themselves.
If the 'unborn' feature is advertised the following argument can be
included in the client's request.

22
ls-refs.c
View File

@ -40,6 +40,12 @@ static void ensure_config_read(void)
config_read = 1;
}
/*
* If we see this many or more "ref-prefix" lines from the client, we consider
* it "too many" and will avoid using the prefix feature entirely.
*/
#define TOO_MANY_PREFIXES 65536
/*
* Check if one of the prefixes is a prefix of the ref.
* If no prefixes were provided, all refs match.
@ -158,15 +164,27 @@ int ls_refs(struct repository *r, struct packet_reader *request)
data.peel = 1;
else if (!strcmp("symrefs", arg))
data.symrefs = 1;
else if (skip_prefix(arg, "ref-prefix ", &out))
strvec_push(&data.prefixes, out);
else if (skip_prefix(arg, "ref-prefix ", &out)) {
if (data.prefixes.nr < TOO_MANY_PREFIXES)
strvec_push(&data.prefixes, out);
}
else if (!strcmp("unborn", arg))
data.unborn = allow_unborn;
else
die(_("unexpected line: '%s'"), arg);
}
if (request->status != PACKET_READ_FLUSH)
die(_("expected flush after ls-refs arguments"));
/*
* If we saw too many prefixes, we must avoid using them at all; as
* soon as we have any prefix, they are meant to form a comprehensive
* list.
*/
if (data.prefixes.nr >= TOO_MANY_PREFIXES)
strvec_clear(&data.prefixes);
send_possibly_unborn_head(&data);
if (!data.prefixes.nr)
strvec_push(&data.prefixes, "");

120
serve.c
View File

@ -10,6 +10,7 @@
#include "upload-pack.h"
static int advertise_sid = -1;
static int client_hash_algo = GIT_HASH_SHA1;
static int always_advertise(struct repository *r,
struct strbuf *value)
@ -33,6 +34,17 @@ static int object_format_advertise(struct repository *r,
return 1;
}
static void object_format_receive(struct repository *r,
const char *algo_name)
{
if (!algo_name)
die("object-format capability requires an argument");
client_hash_algo = hash_algo_by_name(algo_name);
if (client_hash_algo == GIT_HASH_UNKNOWN)
die("unknown object format '%s'", algo_name);
}
static int session_id_advertise(struct repository *r, struct strbuf *value)
{
if (advertise_sid == -1 &&
@ -45,6 +57,14 @@ static int session_id_advertise(struct repository *r, struct strbuf *value)
return 1;
}
static void session_id_receive(struct repository *r,
const char *client_sid)
{
if (!client_sid)
client_sid = "";
trace2_data_string("transfer", NULL, "client-sid", client_sid);
}
struct protocol_capability {
/*
* The name of the capability. The server uses this name when
@ -70,6 +90,16 @@ struct protocol_capability {
* This field should be NULL for capabilities which are not commands.
*/
int (*command)(struct repository *r, struct packet_reader *request);
/*
* Function called when a client requests the capability as a
* non-command. This may be NULL if the capability does nothing.
*
* For a capability of the form "foo=bar", the value string points to
* the content after the "=" (i.e., "bar"). For simple capabilities
* (just "foo"), it is NULL.
*/
void (*receive)(struct repository *r, const char *value);
};
static struct protocol_capability capabilities[] = {
@ -94,10 +124,12 @@ static struct protocol_capability capabilities[] = {
{
.name = "object-format",
.advertise = object_format_advertise,
.receive = object_format_receive,
},
{
.name = "session-id",
.advertise = session_id_advertise,
.receive = session_id_receive,
},
{
.name = "object-info",
@ -139,7 +171,7 @@ void protocol_v2_advertise_capabilities(void)
strbuf_release(&value);
}
static struct protocol_capability *get_capability(const char *key)
static struct protocol_capability *get_capability(const char *key, const char **value)
{
int i;
@ -149,31 +181,46 @@ static struct protocol_capability *get_capability(const char *key)
for (i = 0; i < ARRAY_SIZE(capabilities); i++) {
struct protocol_capability *c = &capabilities[i];
const char *out;
if (skip_prefix(key, c->name, &out) && (!*out || *out == '='))
if (!skip_prefix(key, c->name, &out))
continue;
if (!*out) {
*value = NULL;
return c;
}
if (*out++ == '=') {
*value = out;
return c;
}
}
return NULL;
}
static int is_valid_capability(const char *key)
static int receive_client_capability(const char *key)
{
const struct protocol_capability *c = get_capability(key);
const char *value;
const struct protocol_capability *c = get_capability(key, &value);
return c && c->advertise(the_repository, NULL);
if (!c || c->command || !c->advertise(the_repository, NULL))
return 0;
if (c->receive)
c->receive(the_repository, value);
return 1;
}
static int is_command(const char *key, struct protocol_capability **command)
static int parse_command(const char *key, struct protocol_capability **command)
{
const char *out;
if (skip_prefix(key, "command=", &out)) {
struct protocol_capability *cmd = get_capability(out);
const char *value;
struct protocol_capability *cmd = get_capability(out, &value);
if (*command)
die("command '%s' requested after already requesting command '%s'",
out, (*command)->name);
if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command)
if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command || value)
die("invalid command '%s'", out);
*command = cmd;
@ -183,42 +230,6 @@ static int is_command(const char *key, struct protocol_capability **command)
return 0;
}
static int has_capability(const struct strvec *keys, const char *capability,
const char **value)
{
int i;
for (i = 0; i < keys->nr; i++) {
const char *out;
if (skip_prefix(keys->v[i], capability, &out) &&
(!*out || *out == '=')) {
if (value) {
if (*out == '=')
out++;
*value = out;
}
return 1;
}
}
return 0;
}
static void check_algorithm(struct repository *r, struct strvec *keys)
{
int client = GIT_HASH_SHA1, server = hash_algo_by_ptr(r->hash_algo);
const char *algo_name;
if (has_capability(keys, "object-format", &algo_name)) {
client = hash_algo_by_name(algo_name);
if (client == GIT_HASH_UNKNOWN)
die("unknown object format '%s'", algo_name);
}
if (client != server)
die("mismatched object format: server %s; client %s\n",
r->hash_algo->name, hash_algos[client].name);
}
enum request_state {
PROCESS_REQUEST_KEYS,
PROCESS_REQUEST_DONE,
@ -228,9 +239,8 @@ static int process_request(void)
{
enum request_state state = PROCESS_REQUEST_KEYS;
struct packet_reader reader;
struct strvec keys = STRVEC_INIT;
int seen_capability_or_command = 0;
struct protocol_capability *command = NULL;
const char *client_sid;
packet_reader_init(&reader, 0, NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
@ -250,10 +260,9 @@ static int process_request(void)
case PACKET_READ_EOF:
BUG("Should have already died when seeing EOF");
case PACKET_READ_NORMAL:
/* collect request; a sequence of keys and values */
if (is_command(reader.line, &command) ||
is_valid_capability(reader.line))
strvec_push(&keys, reader.line);
if (parse_command(reader.line, &command) ||
receive_client_capability(reader.line))
seen_capability_or_command = 1;
else
die("unknown capability '%s'", reader.line);
@ -265,7 +274,7 @@ static int process_request(void)
* If no command and no keys were given then the client
* wanted to terminate the connection.
*/
if (!keys.nr)
if (!seen_capability_or_command)
return 1;
/*
@ -292,14 +301,13 @@ static int process_request(void)
if (!command)
die("no command requested");
check_algorithm(the_repository, &keys);
if (has_capability(&keys, "session-id", &client_sid))
trace2_data_string("transfer", NULL, "client-sid", client_sid);
if (client_hash_algo != hash_algo_by_ptr(the_repository->hash_algo))
die("mismatched object format: server %s; client %s\n",
the_repository->hash_algo->name,
hash_algos[client_hash_algo].name);
command->command(the_repository, &reader);
strvec_clear(&keys);
return 0;
}

75
t/t5701-git-serve.sh
View File

@ -72,6 +72,37 @@ test_expect_success 'request invalid command' '
test_i18ngrep "invalid command" err
'
test_expect_success 'request capability as command' '
test-tool pkt-line pack >in <<-EOF &&
command=agent
object-format=$(test_oid algo)
0000
EOF
test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
grep invalid.command.*agent err
'
test_expect_success 'request command as capability' '
test-tool pkt-line pack >in <<-EOF &&
command=ls-refs
object-format=$(test_oid algo)
fetch
0000
EOF
test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
grep unknown.capability err
'
test_expect_success 'requested command is command=value' '
test-tool pkt-line pack >in <<-EOF &&
command=ls-refs=whatever
object-format=$(test_oid algo)
0000
EOF
test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
grep invalid.command.*ls-refs=whatever err
'
test_expect_success 'wrong object-format' '
test-tool pkt-line pack >in <<-EOF &&
command=fetch
@ -116,6 +147,19 @@ test_expect_success 'basics of ls-refs' '
test_cmp expect actual
'
test_expect_success 'ls-refs complains about unknown options' '
test-tool pkt-line pack >in <<-EOF &&
command=ls-refs
object-format=$(test_oid algo)
0001
no-such-arg
0000
EOF
test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
grep unexpected.line.*no-such-arg err
'
test_expect_success 'basic ref-prefixes' '
test-tool pkt-line pack >in <<-EOF &&
command=ls-refs
@ -158,6 +202,37 @@ test_expect_success 'refs/heads prefix' '
test_cmp expect actual
'
test_expect_success 'ignore very large set of prefixes' '
# generate a large number of ref-prefixes that we expect
# to match nothing; the value here exceeds TOO_MANY_PREFIXES
# from ls-refs.c.
{
echo command=ls-refs &&
echo object-format=$(test_oid algo) &&
echo 0001 &&
perl -le "print \"ref-prefix refs/heads/\$_\" for (1..65536)" &&
echo 0000
} |
test-tool pkt-line pack >in &&
# and then confirm that we see unmatched prefixes anyway (i.e.,
# that the prefix was not applied).
cat >expect <<-EOF &&
$(git rev-parse HEAD) HEAD
$(git rev-parse refs/heads/dev) refs/heads/dev
$(git rev-parse refs/heads/main) refs/heads/main
$(git rev-parse refs/heads/release) refs/heads/release
$(git rev-parse refs/tags/annotated-tag) refs/tags/annotated-tag
$(git rev-parse refs/tags/one) refs/tags/one
$(git rev-parse refs/tags/two) refs/tags/two
0000
EOF
test-tool serve-v2 --stateless-rpc <in >out &&
test-tool pkt-line unpack <out >actual &&
test_cmp expect actual
'
test_expect_success 'peel parameter' '
test-tool pkt-line pack >in <<-EOF &&
command=ls-refs