Re: What's cooking in git.git (Jun 2016, #01; Thu, 2)

On Fri, Jun 03, 2016 at 08:59:46AM -0700, Junio C Hamano wrote:

Torsten Bögershausen [off-list ref] writes:

quoted

There where 2 comments in the review.
The most important thing is that now
git://[example.com:123]/path/to/repo is valid, but it shouldn't.
This patch fixes it:

@@ -673,7 +669,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_user,
         * "host:port" and NULL.
         * To support this undocumented legacy we still need to split the port.
         */
-       if (!port)
+       if (!port && protocol == PROTO_SSH)

Hmph, which one of these (if any) is valid, which ones aren't and
why?

    git://[example.com:123]/path/to/repo
    ssh://[example.com:123]/path/to/repo
    [example.com:123]:/path/to/repo

I am wondering about the latter two, because both of them would
become PROTO_SSH at some point in the codepath.  And I am wondering
about the first two, because they look the same at the syntactic
level and if one is allowed the users would expect the other would
also be (or vice versa).

In fact, the parser doesn't even reject the one that is considered
invalid (the first).

It just happens to not work because example.com:123 is not a valid
hostname, and can't be resolved, which is then the error presented to
the user.

Which brings me to:

On Fri, Jun 03, 2016 at 07:07:18AM +0200, Torsten Bögershausen wrote:

The other thing is that I asked for a test case for
git://[example.com:123]/path/to/repo
which shouldn't be hard to do.

Since the parser doesn't error out, the only way to test this is to
check that the parser thinks the output for it is host=example.com:123
port=NONE. Which sounds like an awful way to check for this.

(Also, the discussion back then was about
git://[example.com:123]:/path/to/repo, not
git://[example.com:123]/path/to/repo)

Mike