On Wed, Sep 13, 2017 at 2:54 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> A normal request to git-daemon is structured as
> "command path/to/repo\0host=..\0" and due to a bug in an old version of
> git-daemon 73bb33a94 (daemon: Strictly parse the "extra arg" part of the
> command, 2009-06-04) we aren't able to place any extra args (separated
> by NULs) besides the host.
>
> In order to get around this limitation teach git-daemon to recognize
> additional request arguments hidden behind a second NUL byte. Requests
> can then be structured like:
> "command path/to/repo\0host=..\0\0version=1\0key=value\0". git-daemon
> can then parse out the extra arguments and set 'GIT_PROTOCOL'
> accordingly.
>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
> daemon.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 61 insertions(+), 10 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index 30747075f..250dbf82c 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -282,7 +282,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
> return NULL; /* Fallthrough. Deny by default */
> }
>
> -typedef int (*daemon_service_fn)(void);
> +typedef int (*daemon_service_fn)(const struct argv_array *env);
> struct daemon_service {
> const char *name;
> const char *config_name;
> @@ -363,7 +363,7 @@ static int run_access_hook(struct daemon_service *service, const char *dir,
> }
>
> static int run_service(const char *dir, struct daemon_service *service,
> - struct hostinfo *hi)
> + struct hostinfo *hi, const struct argv_array *env)
> {
> const char *path;
> int enabled = service->enabled;
> @@ -422,7 +422,7 @@ static int run_service(const char *dir, struct daemon_service *service,
> */
> signal(SIGTERM, SIG_IGN);
>
> - return service->fn();
> + return service->fn(env);
> }
>
> static void copy_to_log(int fd)
> @@ -462,25 +462,34 @@ static int run_service_command(struct child_process *cld)
> return finish_command(cld);
> }
>
> -static int upload_pack(void)
> +static int upload_pack(const struct argv_array *env)
> {
> struct child_process cld = CHILD_PROCESS_INIT;
> argv_array_pushl(&cld.args, "upload-pack", "--strict", NULL);
> argv_array_pushf(&cld.args, "--timeout=%u", timeout);
> +
> + argv_array_pushv(&cld.env_array, env->argv);
> +
> return run_service_command(&cld);
> }
>
> -static int upload_archive(void)
> +static int upload_archive(const struct argv_array *env)
> {
> struct child_process cld = CHILD_PROCESS_INIT;
> argv_array_push(&cld.args, "upload-archive");
> +
> + argv_array_pushv(&cld.env_array, env->argv);
> +
> return run_service_command(&cld);
> }
>
> -static int receive_pack(void)
> +static int receive_pack(const struct argv_array *env)
> {
> struct child_process cld = CHILD_PROCESS_INIT;
> argv_array_push(&cld.args, "receive-pack");
> +
> + argv_array_pushv(&cld.env_array, env->argv);
> +
> return run_service_command(&cld);
> }
>
> @@ -574,7 +583,7 @@ static void canonicalize_client(struct strbuf *out, const char *in)
> /*
> * Read the host as supplied by the client connection.
> */
> -static void parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
> +static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
> {
> char *val;
> int vallen;
> @@ -602,6 +611,39 @@ static void parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
> if (extra_args < end && *extra_args)
> die("Invalid request");
> }
> +
> + return extra_args;
> +}
> +
> +static void parse_extra_args(struct argv_array *env, const char *extra_args,
> + int buflen)
> +{
> + const char *end = extra_args + buflen;
> + struct strbuf git_protocol = STRBUF_INIT;
> +
> + for (; extra_args < end; extra_args += strlen(extra_args) + 1) {
> + const char *arg = extra_args;
> +
> + /*
> + * Parse the extra arguments, adding most to 'git_protocol'
> + * which will be used to set the 'GIT_PROTOCOL' envvar in the
> + * service that will be run.
> + *
> + * If there ends up being a particular arg in the future that
> + * git-daemon needs to parse specificly (like the 'host' arg)
> + * then it can be parsed here and not added to 'git_protocol'.
> + */
> + if (*arg) {
> + if (git_protocol.len > 0)
> + strbuf_addch(&git_protocol, ':');
> + strbuf_addstr(&git_protocol, arg);
> + }
> + }
> +
> + if (git_protocol.len > 0)
> + argv_array_pushf(env, GIT_PROTOCOL_ENVIRONMENT "=%s",
> + git_protocol.buf);
> + strbuf_release(&git_protocol);
> }
I wonder if this could be written as
begin = extra_args;
p = extra_args;
end = extra_args + buflen;
while (p < end) {
if (!*p)
*p = ':';
p++;
}
argv_array_pushf(env, GIT_PROTOCOL_ENVIRONMENT "=%s", begin);
to ease the load on the server side, as then we do not
have to copy the partial strings into strbufs and then
count the length individually? (maybe performance is no big deal here?)
>
> /*
> @@ -695,6 +737,7 @@ static int execute(void)
> int pktlen, len, i;
> char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
> struct hostinfo hi;
> + struct argv_array env = ARGV_ARRAY_INIT;
>
> hostinfo_init(&hi);
>
> @@ -716,8 +759,14 @@ static int execute(void)
> pktlen--;
> }
>
> - if (len != pktlen)
> - parse_host_arg(&hi, line + len + 1, pktlen - len - 1);
> + if (len != pktlen) {
> + const char *extra_args;
> + /* retrieve host */
> + extra_args = parse_host_arg(&hi, line + len + 1, pktlen - len - 1);
> +
> + /* parse additional args hidden behind a second NUL byte */
> + parse_extra_args(&env, extra_args + 1, pktlen - (extra_args - line) - 1);
> + }
>
> for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
> struct daemon_service *s = &(daemon_service[i]);
> @@ -730,13 +779,15 @@ static int execute(void)
> * Note: The directory here is probably context sensitive,
> * and might depend on the actual service being performed.
> */
> - int rc = run_service(arg, s, &hi);
> + int rc = run_service(arg, s, &hi, &env);
> hostinfo_clear(&hi);
> + argv_array_clear(&env);
> return rc;
> }
> }
>
> hostinfo_clear(&hi);
> + argv_array_clear(&env);
> logerror("Protocol error: '%s'", line);
> return -1;
> }
> --
> 2.14.1.690.gbb1197296e-goog
>
