Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses

From: Torsten Bögershausen <tboegi@web.de>
Date: Feb 19, 2015, 16:42 UTC
Message-ID: <54E61285.5070600@web.de>
In-Reply-To: <xmqqtwyj2hdk.fsf@gitster.dls.corp.google.com>

On 02/18/2015 07:40 PM, Junio C Hamano wrote:

Show 22 quoted lines

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> On Thu, Jan 22, 2015 at 11:05:29PM +0100, Torsten Bögershausen wrote:
>>> We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/
>>>   because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of
>>> other installations) supports it.
>> I understand that this used to work, but it probably shouldn't have
>> ever been accepted.  It's nonstandard, and if we accept it for ssh,
>> people will want it to work for https, and due to libcurl, it simply
>> won't.
>>
>> I prefer to see our past acceptance of this format as a bug.  This is
>> the first that I've heard of anyone noticing this (since 2013), so it
>> can't be in common usage.
>>
>> If we accept it, we should explicitly document it as being deprecated
>> and note that it's inconsistent with the way everything else works.
> I was reviewing my Undecided pile today, and I think your objection
> makes sense.
>
> Either of you care to update documentation, please, before I drop
> this series and forget about it?

The URL RFC is much stricter regarding which characters that are allowed in which part of the URL, as least as I read it.

The "problem" started when /usr/bin/ssh excepted things like /usr/bin/ssh fe80:x:y:z%eth0 and Git simply passed the hostname to ssh.

And when the [] was there, it was stripped because ssh doesn't like them. URLs like

ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/

simply worked, and nobody ever complained about this, (until now), Git never rejected IPV6 URLs without [], please correct me if I'm wrong.

Git never cared about the exact URL, so that IPV6 URL's without [] where allowed from "day one".

On top of that, we support the short form, user@host:~ or other variants. But we never claimed to be compatible to RFC 1738, even if it makes sense to do so.

What exactly should we write in the documentation ?

Git supports RFC1738 (but is not as strict in parsing the URL, because we assume that the host name resolver will do some checking for us.

Git currently does not support user@[fe80::x:y:z], even if RFC suggests it

Git never claimed to be 100% compatible to RFC 1738, and will probably never be, (as we have old code that is as it is).

We (at least I) don't want to break existing repos, rejecting URL's that had been working before and stopped working because the Git version is updated or so)

This patch series is attempting to be backwards compatible to what old, older. and oldest versions of Git accepted.

At the price that we accept URL's which do not conform to the RFC are accepted. It fixes the long standing issue that user@[fe80:] did not work.

I'm somewhat unsure what to write in the documentation, I must admit.

Unfortunately URL parsing is a tricky thing, this patch tries to do improvements.
Especially it adds test cases, which are good to prevent further breakage.
 
Updating the documentation was never part of the patch series,
and if the documentation is updated, this is done in a separate commit anyway.

How much does this series qualify for the "we didn't update the docs",
but fixed the code, let's drop it ?