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 <[email protected]> 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://[email protected]:/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 [email protected]
> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
