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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html