Re: [PATCH v2 4/9] daemon: recognize hidden request arguments

[Brandon Williams <bmwill@google.com>]

On 09/27, Brandon Williams wrote:
> On 09/27, Junio C Hamano wrote:
> > Brandon Williams <bmwill@google.com> 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?
>
> 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