On 01/03, Stefan Beller wrote:
> On Tue, Jan 2, 2018 at 4:18 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> > Introduce the ls-refs server command. In protocol v2, the ls-refs
> > command is used to request the ref advertisement from the server. Since
> > it is a command which can be requested (as opposed to mandatory in v1),
> > a client can sent a number of parameters in its request to limit the ref
> > advertisement based on provided ref-patterns.
> >
> > Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> > ---
> > Documentation/technical/protocol-v2.txt | 26 +++++++++
> > Makefile | 1 +
> > ls-refs.c | 97 +++++++++++++++++++++++++++++++++
> > ls-refs.h | 9 +++
>
> Maybe consider putting any served command into a sub directory?
>
> For example the code in builtin/ has laxer rules w.r.t. die()ing
> as it is a user facing command, whereas some devs want to see
> code at the root of the repo to not die() at all as the eventual goal
> is to have a library there.
> All this code is on the remote side, which also has different traits than
> the code at the root of the git.git repo; non-localisation comes to mind,
> but there might be other aspects as well (security?).
Well if we were to do this then we should move upload-pack and
receive-pack into this same "server code" directory.
>
>
> > serve.c | 2 +
> > 5 files changed, 135 insertions(+)
> > create mode 100644 ls-refs.c
> > create mode 100644 ls-refs.h
> >
> > diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt
> > index b87ba3816..5f4d0e719 100644
> > --- a/Documentation/technical/protocol-v2.txt
> > +++ b/Documentation/technical/protocol-v2.txt
> > @@ -89,3 +89,29 @@ terminate the connection.
> > Commands are the core actions that a client wants to perform (fetch, push,
> > etc). Each command will be provided with a list capabilities and
> > arguments as requested by a client.
> > +
> > + Ls-refs
>
> So is it ls-refs or Ls-refs or is any capitalization valid?
"ls-refs" I'll make sure to change this.
>
> > +---------
> > +
> > +Ls-refs is the command used to request a reference advertisement in v2.
> > +Unlike the current reference advertisement, ls-refs takes in parameters
> > +which can be used to limit the refs sent from the server.
> > +
> > +Ls-ref takes in the following parameters wraped in packet-lines:
> > +
> > + symrefs: In addition to the object pointed by it, show the underlying
> > + ref pointed by it when showing a symbolic ref.
> > + peel: Show peeled tags.
> > + ref-pattern <pattern>: When specified, only references matching the
> > + given patterns are displayed.
>
> What kind of pattern matching is allowed here?
> strictly prefix only, or globbing, regexes?
> Is there a given grammar to follow? Maybe a link to the git
> glossary is or somewhere else might be fine.
>
> Seeing that we do wildmatch() down there (as opposed to regexes),
> I wonder if it provides an entry for a denial of service attack, by crafting
> a pattern that is very expensive for the server to compute but cheap to
> ask for from a client. (c.f. 94da9193a6 (grep: add support for PCRE v2,
> 2017-06-01, but that is regexes!)
>
> > +The output of ls-refs is as follows:
> > +
> > + output = *ref
> > + flush-pkt
> > + ref = PKT-LINE((tip | peeled) LF)
> > + tip = obj-id SP refname (SP symref-target)
> > + peeled = obj-id SP refname "^{}"
> > +
> > + symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
> > + shallow = PKT-LINE("shallow" SP obj-id LF)
> > diff --git a/Makefile b/Makefile
> > index 5f3b5fe8b..152a73bec 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -820,6 +820,7 @@ LIB_OBJS += list-objects-filter-options.o
> > LIB_OBJS += ll-merge.o
> > LIB_OBJS += lockfile.o
> > LIB_OBJS += log-tree.o
> > +LIB_OBJS += ls-refs.o
> > LIB_OBJS += mailinfo.o
> > LIB_OBJS += mailmap.o
> > LIB_OBJS += match-trees.o
> > diff --git a/ls-refs.c b/ls-refs.c
> > new file mode 100644
> > index 000000000..ac4904a40
> > --- /dev/null
> > +++ b/ls-refs.c
> > @@ -0,0 +1,97 @@
> > +#include "cache.h"
> > +#include "repository.h"
> > +#include "refs.h"
> > +#include "remote.h"
> > +#include "argv-array.h"
> > +#include "ls-refs.h"
> > +#include "pkt-line.h"
> > +
> > +struct ls_refs_data {
> > + unsigned peel;
> > + unsigned symrefs;
> > + struct argv_array patterns;
> > +};
> > +
> > +/*
> > + * Check if one of the patterns matches the tail part of the ref.
> > + * If no patterns were provided, all refs match.
> > + */
> > +static int ref_match(const struct argv_array *patterns, const char *refname)
> > +{
> > + char *pathbuf;
> > + int i;
> > +
> > + if (!patterns->argc)
> > + return 1; /* no restriction */
> > +
> > + pathbuf = xstrfmt("/%s", refname);
> > + for (i = 0; i < patterns->argc; i++) {
> > + if (!wildmatch(patterns->argv[i], pathbuf, 0)) {
> > + free(pathbuf);
> > + return 1;
> > + }
> > + }
> > + free(pathbuf);
> > + return 0;
> > +}
> > +
> > +static int send_ref(const char *refname, const struct object_id *oid,
> > + int flag, void *cb_data)
> > +{
> > + struct ls_refs_data *data = cb_data;
> > + const char *refname_nons = strip_namespace(refname);
> > + struct strbuf refline = STRBUF_INIT;
> > +
> > + if (!ref_match(&data->patterns, refname))
> > + return 0;
> > +
> > + strbuf_addf(&refline, "%s %s", oid_to_hex(oid), refname_nons);
> > + if (data->symrefs && flag & REF_ISSYMREF) {
> > + struct object_id unused;
> > + const char *symref_target = resolve_ref_unsafe(refname, 0,
> > + &unused,
> > + &flag);
> > +
> > + if (!symref_target)
> > + die("'%s' is a symref but it is not?", refname);
> > +
> > + strbuf_addf(&refline, " %s", symref_target);
> > + }
> > +
> > + strbuf_addch(&refline, '\n');
> > +
> > + packet_write(1, refline.buf, refline.len);
> > + if (data->peel) {
> > + struct object_id peeled;
> > + if (!peel_ref(refname, &peeled))
> > + packet_write_fmt(1, "%s %s^{}\n", oid_to_hex(&peeled),
> > + refname_nons);
> > + }
> > +
> > + strbuf_release(&refline);
> > + return 0;
> > +}
> > +
> > +int ls_refs(struct repository *r, struct argv_array *keys, struct argv_array *args)
> > +{
> > + int i;
> > + struct ls_refs_data data = { 0, 0, ARGV_ARRAY_INIT };
> > +
> > + for (i = 0; i < args->argc; i++) {
> > + const char *arg = args->argv[i];
> > + const char *out;
> > +
> > + if (!strcmp("peel", arg))
> > + data.peel = 1;
> > + else if (!strcmp("symrefs", arg))
> > + data.symrefs = 1;
> > + else if (skip_prefix(arg, "ref-pattern ", &out))
> > + argv_array_pushf(&data.patterns, "*/%s", out);
> > + }
> > +
> > + head_ref_namespaced(send_ref, &data);
> > + for_each_namespaced_ref(send_ref, &data);
> > + packet_flush(1);
> > + argv_array_clear(&data.patterns);
> > + return 0;
> > +}
> > diff --git a/ls-refs.h b/ls-refs.h
> > new file mode 100644
> > index 000000000..9e4c57bfe
> > --- /dev/null
> > +++ b/ls-refs.h
> > @@ -0,0 +1,9 @@
> > +#ifndef LS_REFS_H
> > +#define LS_REFS_H
> > +
> > +struct repository;
> > +struct argv_array;
> > +extern int ls_refs(struct repository *r, struct argv_array *keys,
> > + struct argv_array *args);
> > +
> > +#endif /* LS_REFS_H */
> > diff --git a/serve.c b/serve.c
> > index da8127775..88d548410 100644
> > --- a/serve.c
> > +++ b/serve.c
> > @@ -4,6 +4,7 @@
> > #include "pkt-line.h"
> > #include "version.h"
> > #include "argv-array.h"
> > +#include "ls-refs.h"
> > #include "serve.h"
> >
> > static int always_advertise(struct repository *r,
> > @@ -44,6 +45,7 @@ struct protocol_capability {
> > static struct protocol_capability capabilities[] = {
> > { "agent", agent_advertise, NULL },
> > { "stateless-rpc", always_advertise, NULL },
> > + { "ls-refs", always_advertise, ls_refs },
> > };
> >
> > static void advertise_capabilities(void)
> > --
> > 2.15.1.620.gb9897f4670-goog
> >
--
Brandon Williams
