All of lore.kernel.org
 help / color / mirror / Atom feed
* git 2.3.4, ssh: Could not resolve hostname
@ 2015-04-02 17:18 Reid Woodbury Jr.
  2015-04-02 18:09 ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Reid Woodbury Jr. @ 2015-04-02 17:18 UTC (permalink / raw)
  To: git

Dear Sirs

After upgrading from GIT 2.3.3 to 2.3.4 (on Mac OS X 10.10.2, installed with MacPorts) I received this error message when doing a push:

$ git push
ssh: Could not resolve hostname xxxx:: nodename nor servname provided, or not known
fatal: Could not read from remote repository.


It was working previously and nothing in ~/.gitconfig nor .git/config was changed. I rolled back to 2.3.3 and it is working again.

Reid Woodbury
https://github.com/diskerror

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 17:18 git 2.3.4, ssh: Could not resolve hostname Reid Woodbury Jr.
@ 2015-04-02 18:09 ` Jeff King
  2015-04-02 18:58   ` Reid Woodbury Jr.
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2015-04-02 18:09 UTC (permalink / raw)
  To: Reid Woodbury Jr.; +Cc: Torsten Bögershausen, git

On Thu, Apr 02, 2015 at 10:18:33AM -0700, Reid Woodbury Jr. wrote:

> After upgrading from GIT 2.3.3 to 2.3.4 (on Mac OS X 10.10.2,
> installed with MacPorts) I received this error message when doing a
> push:
> 
> $ git push
> ssh: Could not resolve hostname xxxx:: nodename nor servname provided, or not known
> fatal: Could not read from remote repository.

It is hard to tell from the obfuscated output, but perhaps the problem
is the two colons (i.e., git is feeding a hostname like "foo:" when it
should be just "foo"). There were some changes in v2.3.4 related to
parsing ssh URLs. +cc Torsten, who worked on that code.

Can you show us your git config (presumably the host is defined in
remote.origin.url in .git/config of the repository)?

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 18:09 ` Jeff King
@ 2015-04-02 18:58   ` Reid Woodbury Jr.
  2015-04-02 19:14     ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Reid Woodbury Jr. @ 2015-04-02 18:58 UTC (permalink / raw)
  To: Jeff King; +Cc: Torsten Bögershausen, git

Peff
The colons were part of the output. The 'xxxx' replaces the domain in the response. The domain is an internal one that my client would rather keep private. But this got me to think that this might be an important detail: I am using GIT from a remote node on a Cisco AnyConnect VPN with DNS served by ActiveDirectory.
Reid


> On Apr 2, 2015, at 11:09 AM, Jeff King <peff@peff.net> wrote:
> 
> On Thu, Apr 02, 2015 at 10:18:33AM -0700, Reid Woodbury Jr. wrote:
> 
>> After upgrading from GIT 2.3.3 to 2.3.4 (on Mac OS X 10.10.2,
>> installed with MacPorts) I received this error message when doing a
>> push:
>> 
>> $ git push
>> ssh: Could not resolve hostname xxxx:: nodename nor servname provided, or not known
>> fatal: Could not read from remote repository.
> 
> It is hard to tell from the obfuscated output, but perhaps the problem
> is the two colons (i.e., git is feeding a hostname like "foo:" when it
> should be just "foo"). There were some changes in v2.3.4 related to
> parsing ssh URLs. +cc Torsten, who worked on that code.
> 
> Can you show us your git config (presumably the host is defined in
> remote.origin.url in .git/config of the repository)?
> 
> -Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 18:58   ` Reid Woodbury Jr.
@ 2015-04-02 19:14     ` Jeff King
  2015-04-02 19:24       ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2015-04-02 19:14 UTC (permalink / raw)
  To: Reid Woodbury Jr.; +Cc: Torsten Bögershausen, git

On Thu, Apr 02, 2015 at 11:58:13AM -0700, Reid Woodbury Jr. wrote:

> The colons were part of the output. The 'xxxx' replaces the domain in
> the response.

OK, if the double colons are correct, then that is almost certainly the
problem:

  $ ssh does-not-exist
  ssh: Could not resolve hostname does-not-exist: No address associated with hostname
  $ ssh does-not-exist:
  ssh: Could not resolve hostname does-not-exist:: No address associated with hostname

> The domain is an internal one that my client would rather keep private.

Can you give us a hint as to the format of your remote URL? This "works":

  $ git push does-not-exist:repo.git
  ssh: Could not resolve hostname does-not-exist: No address associated with hostname

in the sense that it looks up the right hostname (which is of course
nonsense, but note the single colon in the error message). So does:

  $ git push ssh://does-not-exist/repo.git
  ssh: Could not resolve hostname does-not-exist: No address associated with hostname

but this does not:

  $ git push ssh://does-not-exist:/repo.git
  ssh: Could not resolve hostname does-not-exist:: No address associated with hostname

(note the doubled colon). v2.3.3 did strip off that extra colon, but I
am not sure the URL above (i.e., a colon with no hostname) is actually
sane. IOW, it may have happened to work in older versions, but I'm not
sure we would want to promise to keep it working.

Can you show us what your URL looks like, obfuscating the names but
keeping the syntax the same? Also, are you using the "insteadOf" config
syntax at all (which could easily lead to funny splicing, I imagine).

> But this got me to think that this might be an
> important detail: I am using GIT from a remote node on a Cisco
> AnyConnect VPN with DNS served by ActiveDirectory.

If the extra colon is indeed the problem, I don't think the DNS setup is
relevant. The name git is feeding to ssh is bogus.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 19:14     ` Jeff King
@ 2015-04-02 19:24       ` Junio C Hamano
  2015-04-02 19:31         ` Reid Woodbury Jr.
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2015-04-02 19:24 UTC (permalink / raw)
  To: Reid Woodbury Jr.; +Cc: Jeff King, Torsten Bögershausen, git

Jeff King <peff@peff.net> writes:

> but this does not:
>
>   $ git push ssh://does-not-exist:/repo.git
>   ssh: Could not resolve hostname does-not-exist:: No address associated with hostname
>
> (note the doubled colon). v2.3.3 did strip off that extra colon, but I
> am not sure the URL above (i.e., a colon with no hostname) is actually
> sane. IOW, it may have happened to work in older versions, but I'm not
> sure we would want to promise to keep it working.
>
> Can you show us what your URL looks like, obfuscating the names but
> keeping the syntax the same? Also, are you using the "insteadOf" config
> syntax at all (which could easily lead to funny splicing, I imagine).

Everything Jeff said ;-)

Depending on the nature of 'xxxx' in the original, Torsten's
response may be different.  'xxxx' could stand for [9999:9999::9999],
a.host.in.domain.xz, 127.0.0.1, or all the other things and it is a
bit too vague to help us tell which codepath will pick up what and
possibly screw it up.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 19:24       ` Junio C Hamano
@ 2015-04-02 19:31         ` Reid Woodbury Jr.
  2015-04-02 19:35           ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Reid Woodbury Jr. @ 2015-04-02 19:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Torsten Bögershausen, git

Ah, understand. Here's my project URL for 'remote "origin"' with a more meaningful representation of their internal FQDN:

	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git

The "online" is their literal internal TLD.

Reid

> On Apr 2, 2015, at 12:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Jeff King <peff@peff.net> writes:
> 
>> but this does not:
>> 
>>  $ git push ssh://does-not-exist:/repo.git
>>  ssh: Could not resolve hostname does-not-exist:: No address associated with hostname
>> 
>> (note the doubled colon). v2.3.3 did strip off that extra colon, but I
>> am not sure the URL above (i.e., a colon with no hostname) is actually
>> sane. IOW, it may have happened to work in older versions, but I'm not
>> sure we would want to promise to keep it working.
>> 
>> Can you show us what your URL looks like, obfuscating the names but
>> keeping the syntax the same? Also, are you using the "insteadOf" config
>> syntax at all (which could easily lead to funny splicing, I imagine).
> 
> Everything Jeff said ;-)
> 
> Depending on the nature of 'xxxx' in the original, Torsten's
> response may be different.  'xxxx' could stand for [9999:9999::9999],
> a.host.in.domain.xz, 127.0.0.1, or all the other things and it is a
> bit too vague to help us tell which codepath will pick up what and
> possibly screw it up.
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 19:31         ` Reid Woodbury Jr.
@ 2015-04-02 19:35           ` Jeff King
  2015-04-02 20:06             ` Reid Woodbury Jr.
  2015-04-03  0:02             ` Torsten Bögershausen
  0 siblings, 2 replies; 16+ messages in thread
From: Jeff King @ 2015-04-02 19:35 UTC (permalink / raw)
  To: Reid Woodbury Jr.; +Cc: Junio C Hamano, Torsten Bögershausen, git

On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote:

> Ah, understand. Here's my project URL for 'remote "origin"' with a
> more meaningful representation of their internal FQDN:
> 
> 	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git
> 
> The "online" is their literal internal TLD.

Thanks. The problem is the extra ":" after "online"; your URL is
malformed. You can just drop that colon entirely.

I do not think we need to support this syntax going forward (the colon
is meaningless here, and our documentation is clear that it should go
with a port number), but on the other hand, it might be nice to be more
liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see
whether supporting that would hurt some of the other cases, or whether
it would make the code too awkward.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 19:35           ` Jeff King
@ 2015-04-02 20:06             ` Reid Woodbury Jr.
  2015-04-02 20:15               ` Thomas Schneider
  2015-04-03  0:02             ` Torsten Bögershausen
  1 sibling, 1 reply; 16+ messages in thread
From: Reid Woodbury Jr. @ 2015-04-02 20:06 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Torsten Bögershausen, git

Yup, removing the colon works in both 2.3.3 and 2.3.4. And I see that the manual doesn't use the colon! (eg. $ git clone ssh://user@server/project.git). The usage of the colon looks normal to my eyes because, for instance, SFTP uses it to set the path on login so this wasn't something I would have even considered. I'm sure I've seen it other places but I can't remember right now.

Thanks all for your time.


> On Apr 2, 2015, at 12:35 PM, Jeff King <peff@peff.net> wrote:
> 
> On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote:
> 
>> Ah, understand. Here's my project URL for 'remote "origin"' with a
>> more meaningful representation of their internal FQDN:
>> 
>> 	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git
>> 
>> The "online" is their literal internal TLD.
> 
> Thanks. The problem is the extra ":" after "online"; your URL is
> malformed. You can just drop that colon entirely.
> 
> I do not think we need to support this syntax going forward (the colon
> is meaningless here, and our documentation is clear that it should go
> with a port number), but on the other hand, it might be nice to be more
> liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see
> whether supporting that would hurt some of the other cases, or whether
> it would make the code too awkward.
> 
> -Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 20:06             ` Reid Woodbury Jr.
@ 2015-04-02 20:15               ` Thomas Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Schneider @ 2015-04-02 20:15 UTC (permalink / raw)
  To: Reid Woodbury Jr.
  Cc: Jeff King, Junio C Hamano, Torsten Bögershausen, git

2015-04-02 22:06 GMT+02:00 Reid Woodbury Jr. <reidw@rawsound.com>:
> I'm sure I've seen it other places but I can't remember right now.
What you mean is the scp-like syntax: user@host:path/relative/to/home
– but if you write user@host:/path/to/something, it’s relative to /.
You can also achieve paths relative to the home directory with the
other syntax: ssh://user@host/~/path/relative/to/home.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-02 19:35           ` Jeff King
  2015-04-02 20:06             ` Reid Woodbury Jr.
@ 2015-04-03  0:02             ` Torsten Bögershausen
  2015-04-03  1:30               ` brian m. carlson
                                 ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Torsten Bögershausen @ 2015-04-03  0:02 UTC (permalink / raw)
  To: Jeff King, Reid Woodbury Jr.
  Cc: Junio C Hamano, Torsten Bögershausen, git

On 2015-04-02 21.35, Jeff King wrote:
> On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote:
> 
>> Ah, understand. Here's my project URL for 'remote "origin"' with a
>> more meaningful representation of their internal FQDN:
>>
>> 	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git
>>
>> The "online" is their literal internal TLD.
> 
> Thanks. The problem is the extra ":" after "online"; your URL is
> malformed. You can just drop that colon entirely.
> 
> I do not think we need to support this syntax going forward (the colon
> is meaningless here, and our documentation is clear that it should go
> with a port number), but on the other hand, it might be nice to be more
> liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see
> whether supporting that would hurt some of the other cases, or whether
> it would make the code too awkward.
> 
> -Peff

Thanks for digging.

This makes my think that it is
a) non-standard to have the extra colon
b) The error message could be better
c) We don't have a test case
d) This reminds my of an improvement from Linus:
608d48b2207a61528
......
    So when somebody passes me a "please pull" request pointing to something
    like the following
    
    	git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
    
    (note the extraneous colon at the end of the host name), git would happily
    try to connect to port 0, which would generally just cause the remote to
    not even answer, and the "connect()" will take a long time to time out.
.....

Sorry guys for the regression, the old parser handled the extra colon as "port 0",
the new one looks for the "/" as the end of the hostname (and the beginning of the path) 

Either we accept the extra colon as before, or the parser puts out a better error message,
(because the OS doesn't seem to do so):

./git clone git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Cloning into 'v4l-dvb'...
fatal: unable to connect to git.kernel.org::
git.kernel.org:[0: 62.157.140.133]: errno=Connection refused
git.kernel.org:[1: 80.156.86.78]: errno=Connection refused

(Especially the "::" is a little bit funny: the first ':' is the extra one,
the second one comes from the error message:
"unable to connect to %s:\n%s"

That is not really user-friendly, so I put it onto my TODO-list
It seems as if it comes from the repair of another regression, which re-allows
the usage of IPV6 addresses without []:
./git fetch-pack  --diag-url  ssh://::1/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Diag: url=ssh://::1/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Diag: protocol=ssh
Diag: userandhost=::1
Diag: port=NONE
Diag: path=/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git


And this makes sense too:
./git fetch-pack  --diag-url  ssh://git.kernel.org:1/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Diag: url=ssh://git.kernel.org:1/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Diag: protocol=ssh
Diag: userandhost=git.kernel.org
Diag: port=1
Diag: path=/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git


But not this one:
 ./git fetch-pack  --diag-url  ssh://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Diag: url=ssh://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
Diag: protocol=ssh
Diag: userandhost=git.kernel.org:
Diag: port=NONE


Spontaneously I would say that a trailing ':' at the end of a hostname in the ssh:// scheme
can be safely ignored, what do you think ?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-03  0:02             ` Torsten Bögershausen
@ 2015-04-03  1:30               ` brian m. carlson
  2015-04-03 21:01               ` Junio C Hamano
  2015-04-03 21:32               ` Kyle J. McKay
  2 siblings, 0 replies; 16+ messages in thread
From: brian m. carlson @ 2015-04-03  1:30 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Reid Woodbury Jr., Junio C Hamano, git

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

On Fri, Apr 03, 2015 at 02:02:15AM +0200, Torsten Bögershausen wrote:
> But not this one:
>  ./git fetch-pack  --diag-url  ssh://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
> Diag: url=ssh://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
> Diag: protocol=ssh
> Diag: userandhost=git.kernel.org:
> Diag: port=NONE
> 
> 
> Spontaneously I would say that a trailing ':' at the end of a hostname in the ssh:// scheme
> can be safely ignored, what do you think ?

I think instead of ignoring it we can just produce an error.  The user
might have meant to specify a port, but forgotten.  I've done similar
things before.

I generally prefer being a bit stricter and giving helpful error
messages rather than trying to intuit what the user meant.  The user is
always going to come up with a new way to break the code.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-03  0:02             ` Torsten Bögershausen
  2015-04-03  1:30               ` brian m. carlson
@ 2015-04-03 21:01               ` Junio C Hamano
  2015-04-03 21:05                 ` Jeff King
  2015-04-03 21:32               ` Kyle J. McKay
  2 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2015-04-03 21:01 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Jeff King, Reid Woodbury Jr., git

Torsten Bögershausen <tboegi@web.de> writes:

> This makes my think that it is
> a) non-standard to have the extra colon
> b) The error message could be better

For that, perhaps

-ssh: Could not resolve hostname xxxx:: nodename nor servname provided, or not known
+ssh: Could not resolve hostname "xxxx:": nodename nor servname provided, or not known

would be something we would want to do, no matter what other fixes
we would apply.

> Spontaneously I would say that a trailing ':' at the end of a
> hostname in the ssh:// scheme can be safely ignored, what do you
> think?

If it is not too much hassle to make the current code do so, I'd say
that is a good way forward.  Giving a warning that lets the user
know that the input has an extra and unwanted colon in it may be a
plus, too.

Thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-03 21:01               ` Junio C Hamano
@ 2015-04-03 21:05                 ` Jeff King
  0 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2015-04-03 21:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, Reid Woodbury Jr., git

On Fri, Apr 03, 2015 at 02:01:55PM -0700, Junio C Hamano wrote:

> Torsten Bögershausen <tboegi@web.de> writes:
> 
> > This makes my think that it is
> > a) non-standard to have the extra colon
> > b) The error message could be better
> 
> For that, perhaps
> 
> -ssh: Could not resolve hostname xxxx:: nodename nor servname provided, or not known
> +ssh: Could not resolve hostname "xxxx:": nodename nor servname provided, or not known
> 
> would be something we would want to do, no matter what other fixes
> we would apply.

That message comes from the ssh client. So the "we" here would have to submit a
patch to OpenSSH".

The easier way to diagnose inside git is to set GIT_TRACE, which makes
it more clear:

  $ GIT_TRACE=1 git clone ssh://bogosity:/repo.git
  ...
  17:05:00.734019 run-command.c:347       trace: run_command: 'ssh' 'bogosity:' 'git-upload-pack '\''/repo.git'\'''

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-03  0:02             ` Torsten Bögershausen
  2015-04-03  1:30               ` brian m. carlson
  2015-04-03 21:01               ` Junio C Hamano
@ 2015-04-03 21:32               ` Kyle J. McKay
  2015-04-04  0:19                 ` Reid Woodbury Jr.
  2 siblings, 1 reply; 16+ messages in thread
From: Kyle J. McKay @ 2015-04-03 21:32 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Reid Woodbury Jr.,
	brian m. carlson, Junio C Hamano, git@vger.kernel.org list

On Apr 2, 2015, at 17:02, Torsten Bögershausen wrote:

> On 2015-04-02 21.35, Jeff King wrote:
>> On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote:
>>
>>> Ah, understand. Here's my project URL for 'remote "origin"' with a
>>> more meaningful representation of their internal FQDN:
>>>
>>> 	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git
>>>
>>> The "online" is their literal internal TLD.
>>
>> Thanks. The problem is the extra ":" after "online"; your URL is
>> malformed. You can just drop that colon entirely.
>>
>> I do not think we need to support this syntax going forward (the  
>> colon
>> is meaningless here, and our documentation is clear that it should go
>> with a port number), but on the other hand, it might be nice to be  
>> more
>> liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten  
>> to see
>> whether supporting that would hurt some of the other cases, or  
>> whether
>> it would make the code too awkward.
>>
>> -Peff
>
> Thanks for digging.
>
> This makes my think that it is
> a) non-standard to have the extra colon

It's not.  See RFC 3986 appendix A:

   authority = [ userinfo "@" ] host [ ":" port ]

   port = *DIGIT

"*DIGIT" means (see RFC 2234 section 3.6) zero or more digits.

> b) The error message could be better
> c) We don't have a test case
> d) This reminds my of an improvement from Linus:
> 608d48b2207a61528
> ......
>    So when somebody passes me a "please pull" request pointing to  
> something
>    like the following
>
>    	git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
>
>    (note the extraneous colon at the end of the host name), git  
> would happily
>    try to connect to port 0, which would generally just cause the  
> remote to
>    not even answer, and the "connect()" will take a long time to  
> time out.
> .....
>
> Sorry guys for the regression, the old parser handled the extra  
> colon as "port 0",
> the new one looks for the "/" as the end of the hostname (and the  
> beginning of the path)
>
> Either we accept the extra colon as before, or the parser puts out a  
> better error message,

[...]

> Spontaneously I would say that a trailing ':' at the end of a  
> hostname in the ssh:// scheme
> can be safely ignored, what do you think ?

You know, there is a "url_normalize" routine in urlmatch.h/urlmatch.c  
that checks for a lot of these things and provides a translated error  
message if there's a problem as well as normalizing and separating out  
the various parts of the URL.  It does not currently handle default  
ports for anything other than http[s] but it would be simple enough to  
add support for ssh, git, ftp[s] and rsync default ports too.

-Kyle

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-03 21:32               ` Kyle J. McKay
@ 2015-04-04  0:19                 ` Reid Woodbury Jr.
  2015-04-04  7:21                   ` Torsten Bögershausen
  0 siblings, 1 reply; 16+ messages in thread
From: Reid Woodbury Jr. @ 2015-04-04  0:19 UTC (permalink / raw)
  To: Kyle J. McKay
  Cc: Torsten Bögershausen, Jeff King, brian m. carlson,
	Junio C Hamano, git@vger.kernel.org list

Thanks for keeping me in the loop!

I have two thoughts on handling input:

As a coder I want to know exactly what's going on in my code. If I've given erroneous input I'd like to know about it in the most useful and quickest way, never glossed over, liberally accepted, nor fixed for me even if the input is non-ambigous. I won't learn the right way unless I'm told. I enjoy that when I've typo'd a command in GIT it gives useful suggestions to what I might have meant.

But, most of the coding *I* do is for the non-coder or the general end user. These might be people that would reasonably yell at their computer screen "you know what I meant!" So I try to be more liberal in the input I write code to accept by filtering it, cleaning it, etc. I'll even filter input by keystroke when possible. I have the philosophy: don't tell the user that they input something bad, just prevent them from inputting it to begin with. I know, this is appropriate when building a GUI and not for CLI.

thanks for listening
Reid Woodbury


> On Apr 3, 2015, at 2:32 PM, Kyle J. McKay <mackyle@gmail.com> wrote:
> 
> On Apr 2, 2015, at 17:02, Torsten Bögershausen wrote:
> 
>> On 2015-04-02 21.35, Jeff King wrote:
>>> On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote:
>>> 
>>>> Ah, understand. Here's my project URL for 'remote "origin"' with a
>>>> more meaningful representation of their internal FQDN:
>>>> 
>>>> 	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git
>>>> 
>>>> The "online" is their literal internal TLD.
>>> 
>>> Thanks. The problem is the extra ":" after "online"; your URL is
>>> malformed. You can just drop that colon entirely.
>>> 
>>> I do not think we need to support this syntax going forward (the colon
>>> is meaningless here, and our documentation is clear that it should go
>>> with a port number), but on the other hand, it might be nice to be more
>>> liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see
>>> whether supporting that would hurt some of the other cases, or whether
>>> it would make the code too awkward.
>>> 
>>> -Peff
>> 
>> Thanks for digging.
>> 
>> This makes my think that it is
>> a) non-standard to have the extra colon
> 
> It's not.  See RFC 3986 appendix A:
> 
>  authority = [ userinfo "@" ] host [ ":" port ]
> 
>  port = *DIGIT
> 
> "*DIGIT" means (see RFC 2234 section 3.6) zero or more digits.
> 
>> b) The error message could be better
>> c) We don't have a test case
>> d) This reminds my of an improvement from Linus:
>> 608d48b2207a61528
>> ......
>>   So when somebody passes me a "please pull" request pointing to something
>>   like the following
>> 
>>   	git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
>> 
>>   (note the extraneous colon at the end of the host name), git would happily
>>   try to connect to port 0, which would generally just cause the remote to
>>   not even answer, and the "connect()" will take a long time to time out.
>> .....
>> 
>> Sorry guys for the regression, the old parser handled the extra colon as "port 0",
>> the new one looks for the "/" as the end of the hostname (and the beginning of the path)
>> 
>> Either we accept the extra colon as before, or the parser puts out a better error message,
> 
> [...]
> 
>> Spontaneously I would say that a trailing ':' at the end of a hostname in the ssh:// scheme
>> can be safely ignored, what do you think ?
> 
> You know, there is a "url_normalize" routine in urlmatch.h/urlmatch.c that checks for a lot of these things and provides a translated error message if there's a problem as well as normalizing and separating out the various parts of the URL.  It does not currently handle default ports for anything other than http[s] but it would be simple enough to add support for ssh, git, ftp[s] and rsync default ports too.
> 
> -Kyle

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git 2.3.4, ssh: Could not resolve hostname
  2015-04-04  0:19                 ` Reid Woodbury Jr.
@ 2015-04-04  7:21                   ` Torsten Bögershausen
  0 siblings, 0 replies; 16+ messages in thread
From: Torsten Bögershausen @ 2015-04-04  7:21 UTC (permalink / raw)
  To: Reid Woodbury Jr., Kyle J. McKay
  Cc: Torsten Bögershausen, Jeff King, brian m. carlson,
	Junio C Hamano, git@vger.kernel.org list

On 2015-04-04 02.19, Reid Woodbury Jr. wrote:
> Thanks for keeping me in the loop!
> 
> I have two thoughts on handling input:
> 
> As a coder I want to know exactly what's going on in my code. If I've given erroneous input I'd like to know about it in the most useful and quickest way, never glossed over, liberally accepted, nor fixed for me even if the input is non-ambigous. I won't learn the right way unless I'm told. I enjoy that when I've typo'd a command in GIT it gives useful suggestions to what I might have meant.
> 
> But, most of the coding *I* do is for the non-coder or the general end user. These might be people that would reasonably yell at their computer screen "you know what I meant!" So I try to be more liberal in the input I write code to accept by filtering it, cleaning it, etc. I'll even filter input by keystroke when possible. I have the philosophy: don't tell the user that they input something bad, just prevent them from inputting it to begin with. I know, this is appropriate when building a GUI and not for CLI.
> 
> thanks for listening
> Reid Woodbury
> 
Thanks for the report.
(And please try to avoid top-posting to this list in the future ;-)

The basic fix will look like below, but I need to update the test-suite as well.


diff --git a/connect.c b/connect.c
index ce0e121..c8744f3 100644
--- a/connect.c
+++ b/connect.c
@@ -310,6 +310,8 @@ static void get_host_and_port(char **host, const char **port)
                if (end != colon + 1 && *end == '\0' && 0 <= portnr && portnr < 65536) {
                        *colon = 0;
                        *port = colon + 1;
+               } else if (!colon[1]) {
+                       *colon = 0;
                }
        }
 }
@@ -385,7 +387,7 @@ static int git_tcp_connect_sock(char *host, int flags)
        freeaddrinfo(ai0);
 
        if (sockfd < 0)
-               die("unable to connect to %s:\n%s", host, error_message.buf);
+               die("unable to connect to '%s' :\n%s", host, error_message.buf);
 
        enable_keepalive(sockfd);

> 
>> On Apr 3, 2015, at 2:32 PM, Kyle J. McKay <mackyle@gmail.com> wrote:
>>
>> On Apr 2, 2015, at 17:02, Torsten Bögershausen wrote:
>>
>>> On 2015-04-02 21.35, Jeff King wrote:
>>>> On Thu, Apr 02, 2015 at 12:31:14PM -0700, Reid Woodbury Jr. wrote:
>>>>
>>>>> Ah, understand. Here's my project URL for 'remote "origin"' with a
>>>>> more meaningful representation of their internal FQDN:
>>>>>
>>>>> 	url = ssh://rwoodbury@systemname.groupname.online:/opt/git/inventory.git
>>>>>
>>>>> The "online" is their literal internal TLD.
>>>>
>>>> Thanks. The problem is the extra ":" after "online"; your URL is
>>>> malformed. You can just drop that colon entirely.
>>>>
>>>> I do not think we need to support this syntax going forward (the colon
>>>> is meaningless here, and our documentation is clear that it should go
>>>> with a port number), but on the other hand, it might be nice to be more
>>>> liberal, as we were in v2.3.3 and prior. I'll leave it to Torsten to see
>>>> whether supporting that would hurt some of the other cases, or whether
>>>> it would make the code too awkward.
>>>>
>>>> -Peff
>>>
>>> Thanks for digging.
>>>
>>> This makes my think that it is
>>> a) non-standard to have the extra colon
>>
>> It's not.  See RFC 3986 appendix A:
>>
>>  authority = [ userinfo "@" ] host [ ":" port ]
>>
>>  port = *DIGIT
>>
>> "*DIGIT" means (see RFC 2234 section 3.6) zero or more digits.
>>
>>> b) The error message could be better
>>> c) We don't have a test case
>>> d) This reminds my of an improvement from Linus:
>>> 608d48b2207a61528
>>> ......
>>>   So when somebody passes me a "please pull" request pointing to something
>>>   like the following
>>>
>>>   	git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
>>>
>>>   (note the extraneous colon at the end of the host name), git would happily
>>>   try to connect to port 0, which would generally just cause the remote to
>>>   not even answer, and the "connect()" will take a long time to time out.
>>> .....
>>>
>>> Sorry guys for the regression, the old parser handled the extra colon as "port 0",
>>> the new one looks for the "/" as the end of the hostname (and the beginning of the path)
>>>
>>> Either we accept the extra colon as before, or the parser puts out a better error message,
>>
>> [...]
>>
>>> Spontaneously I would say that a trailing ':' at the end of a hostname in the ssh:// scheme
>>> can be safely ignored, what do you think ?
>>
>> You know, there is a "url_normalize" routine in urlmatch.h/urlmatch.c that checks for a lot of these things and provides a translated error message if there's a problem as well as normalizing and separating out the various parts of the URL.  It does not currently handle default ports for anything other than http[s] but it would be simple enough to add support for ssh, git, ftp[s] and rsync default ports too.
>>
>> -Kyle
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-04-04  7:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-02 17:18 git 2.3.4, ssh: Could not resolve hostname Reid Woodbury Jr.
2015-04-02 18:09 ` Jeff King
2015-04-02 18:58   ` Reid Woodbury Jr.
2015-04-02 19:14     ` Jeff King
2015-04-02 19:24       ` Junio C Hamano
2015-04-02 19:31         ` Reid Woodbury Jr.
2015-04-02 19:35           ` Jeff King
2015-04-02 20:06             ` Reid Woodbury Jr.
2015-04-02 20:15               ` Thomas Schneider
2015-04-03  0:02             ` Torsten Bögershausen
2015-04-03  1:30               ` brian m. carlson
2015-04-03 21:01               ` Junio C Hamano
2015-04-03 21:05                 ` Jeff King
2015-04-03 21:32               ` Kyle J. McKay
2015-04-04  0:19                 ` Reid Woodbury Jr.
2015-04-04  7:21                   ` Torsten Bögershausen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.