Brandon Williams [off-list ref] writes:
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.
It's a bit unclear if that commit _introduced_ a bug, or just
noticed an old bug and documented it in its log message. How does
that commit impact the versons of Git that the updated code is
capable of interracting with?
+static void parse_extra_args(struct hostinfo *hi, struct argv_array *env,
+ char *extra_args, int buflen)
+{
+ const char *end = extra_args + buflen;
+ struct strbuf git_protocol = STRBUF_INIT;
+
+ /* First look for the host argument */
+ extra_args = parse_host_arg(hi, extra_args, buflen);
+
+ /* Look for additional arguments places after a second NUL byte */
+ 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);
}
On 09/27, Junio C Hamano wrote:
Brandon Williams [off-list ref] writes:
quoted
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.
It's a bit unclear if that commit _introduced_ a bug, or just
noticed an old bug and documented it in its log message. How does
that commit impact the versons of Git that the updated code is
capable of interracting with?
You're right, after reading it again it isn't clear. I'll change this
to indicate that the commit is a fix to a bug and that the fix doesn't
allow us to place any additional args.
quoted
+static void parse_extra_args(struct hostinfo *hi, struct argv_array *env,
+ char *extra_args, int buflen)
+{
+ const char *end = extra_args + buflen;
+ struct strbuf git_protocol = STRBUF_INIT;
+
+ /* First look for the host argument */
+ extra_args = parse_host_arg(hi, extra_args, buflen);
+
+ /* Look for additional arguments places after a second NUL byte */
+ 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);
}
--
Brandon Williams
On 09/27, Brandon Williams wrote:
On 09/27, Junio C Hamano wrote:
quoted
Brandon Williams [off-list ref] writes:
quoted
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.
It's a bit unclear if that commit _introduced_ a bug, or just
noticed an old bug and documented it in its log message. How does
that commit impact the versons of Git that the updated code is
capable of interracting with?
You're right, after reading it again it isn't clear. I'll change this
to indicate that the commit is a fix to a bug and that the fix doesn't
allow us to place any additional args.
Ok how about this wording for the commit msg:
daemon: recognize hidden request arguments
A normal request to git-daemon is structured as "command
path/to/repo\0host=..\0" and due to a bug introduced in
49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19)
we aren't able to place any extra arguments (separated by NULs)
besides the host otherwise the parsing of those arguments would
enter an infinite loop. This bug was fixed in 73bb33a94
(daemon: Strictly parse the "extra arg" part of the command,
2009-06-04) but a check was put in place to disallow extra
arguments so that new clients wouldn't trigger this bug in older
servers.
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.
By placing these extra arguments behind a second NUL byte we can
skirt around both the infinite loop bug in 49ba83fb6 (Add
virtualization support to git-daemon, 2006-09-19) as well as the
explicit disallowing of extra arguments introduced in 73bb33a94
(daemon: Strictly parse the "extra arg" part of the command,
2009-06-04) because both of these versions of git-daemon check
for a single NUL byte after the host argument before terminating
the argument parsing.
This way I'm citing when the bug was actually introduced as well as
describing why the 'fix' didn't completely resolve the issue. I also
explain a little bit about how this change should work even with very
old servers which still have the bug. (I tried to get the introp test
to work on a version of git that old but am having some difficulty even
getting the old version to launch git-daemon without hanging for some
unknown reason)
--
Brandon Williams