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 <mack...@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 majord...@vger.kernel.org > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html