git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/6] connect: uniformize and group CONNECT_DIAG_URL handling code

From
Mike Hommey <mh@glandium.org>
Date
May 2, 2016, 12:38 UTC
Message-ID
<20160502123858.GA26091@glandium.org>
In-Reply-To
<57273A0B.5050409@web.de>
On Mon, May 02, 2016 at 01:29:15PM +0200, Torsten Bögershausen wrote:
Show 46 quoted lines
> On 05/02/2016 10:31 AM, Mike Hommey wrote:
> > On Mon, May 02, 2016 at 06:56:54AM +0200, Torsten Bögershausen wrote:
> > > On 05/01/2016 08:02 AM, Mike Hommey wrote:
> > > > +	if (flags & CONNECT_DIAG_URL) {
> > > >    		printf("Diag: url=%s\n", url ? url : "NULL");
> > > >    		printf("Diag: protocol=%s\n", prot_name(protocol));
> > > >    		printf("Diag: hostandport=%s\n", hostandport ? hostandport : "NULL");
> > > > +		printf("Diag: userandhost=%s\n", host ? host : "NULL");
> > > > +		printf("Diag: port=%s\n", port ? port : "NONE");
> > > >    		printf("Diag: path=%s\n", path ? path : "NULL");
> > > >    		conn = NULL;
> > > Does it make sense  to write the host twice?
> > > If things are cleaned up, would something like this make sense ?
> > > 
> > > 		printf("Diag: url=%s\n", url ? url : "NULL");
> > >   		printf("Diag: protocol=%s\n", prot_name(protocol));
> > > 		printf("Diag: user=%s\n", user ? user : "NULL");
> > >   		printf("Diag: host=%s\n", host ? host : "NULL");
> > > 		printf("Diag: port=%s\n", port ? port : "NONE");
> > >   		printf("Diag: path=%s\n", path ? path : "NULL");
> > 
> > That's what I'm converging towards, in the end. There is one thing that
> > needs hostandport, though: the git protocol host header. But I don't
> > really like that parse_connect_url would return user, host, port, *and*
> > hostandport. How about "reconstructing" hostandport in that case, adding
> > square brackets when the host contains colons?
> > 
> > BTW, the git protocol currently doesn't reject urls with users and
> > doesn't seem to handle them properly either. My changes would fix this
> > by separating user and host at the parse_connect_url level, but the
> > question then is what to do when there is a user part? die()?
> > 
> > Mike
> > 
> That is what happening:
> git clone git://xx@git.kernel.org/pub/scm/git/git.git
> Cloning into 'git'...
> fatal: Unable to look up xx@git.kernel.org (port 9418) (Name or service not
> known)
> 
> And that may explain, why we have different handling of ssh vs git:
> The URL-scheme for git:// simply doesn't specify a user name at all.
> 
> git://host:[port]/path/to/repo
> Knowing that, the "@" will be feed into the name resolver,
> and that's OK.

But we might as well throw an error before, because @ is not going to appear in a valid hostname.

Mike
Previous: Torsten BögershausenNext: Junio C Hamano
Message 13 of 46 in “connect: various cleanups”
  1. 0/6 connect: various cleanupsMike Hommey, May 1, 2016
  2. 1/6 connect: remove get_port()Mike Hommey, May 1, 2016
  3. Torsten BögershausenMay 1, 2016
  4. Mike HommeyMay 1, 2016
  5. Jeff KingMay 3, 2016
  6. Mike HommeyMay 3, 2016
  7. 2/6 connect: uniformize and group CONNECT_DIAG_URL handling codeMike Hommey, May 1, 2016
  8. Torsten BögershausenMay 1, 2016
  9. Mike HommeyMay 1, 2016
  10. Torsten BögershausenMay 2, 2016
  11. Mike HommeyMay 2, 2016
  12. Torsten BögershausenMay 2, 2016
  13. Mike HommeyMay 2, 2016
  14. Junio C HamanoMay 2, 2016
  15. Junio C HamanoMay 2, 2016
  16. 3/6 connect: only match the host with core.gitProxyMike Hommey, May 1, 2016
  17. 4/6 connect: pass separate host and port to git_tcp_connect and git_proxy_connectMike Hommey, May 1, 2016
  18. 5/6 connect: don't xstrdup target_hostMike Hommey, May 1, 2016
  19. 6/6 connect: move ssh command line preparation to a separate functionMike Hommey, May 1, 2016
  20. 00/11 connect: various cleanupsMike Hommey, May 3, 2016
  21. 01/11 add fetch-pack --diag-url tests for some corner casesMike Hommey, May 3, 2016
  22. Torsten BögershausenMay 3, 2016
  23. Junio C HamanoMay 3, 2016
  24. Torsten BögershausenMay 3, 2016
  25. Mike HommeyMay 3, 2016
  26. Mike HommeyMay 5, 2016
  27. Torsten BögershausenMay 6, 2016
  28. Junio C HamanoMay 6, 2016
  29. 02/11 connect: call get_host_and_port() earlierMike Hommey, May 3, 2016
  30. 03/11 connect: only match the host with core.gitProxyMike Hommey, May 3, 2016
  31. 04/11 connect: fill the host header in the git protocol with the host and port variablesMike Hommey, May 3, 2016
  32. 05/11 connect: make parse_connect_url() return separated host and portMike Hommey, May 3, 2016
  33. 06/11 connect: group CONNECT_DIAG_URL handling codeMike Hommey, May 3, 2016
  34. 07/11 connect: make parse_connect_url() return the user part of the url as a separate valueMike Hommey, May 3, 2016
  35. 08/11 connect: change the --diag-url output to separate user and hostMike Hommey, May 3, 2016
  36. Torsten BögershausenMay 3, 2016
  37. Eric SunshineMay 3, 2016
  38. Mike HommeyMay 3, 2016
  39. 09/11 connect: use "-l user" instead of "user@" on ssh command lineMike Hommey, May 3, 2016
  40. Torsten BögershausenMay 3, 2016
  41. Junio C HamanoMay 3, 2016
  42. Eric SunshineMay 3, 2016
  43. Mike HommeyMay 3, 2016
  44. 10/11 connect: actively reject git:// urls with a user partMike Hommey, May 3, 2016
  45. 11/11 connect: move ssh command line preparation to a separate functionMike Hommey, May 3, 2016
  46. 11/11 connect: move ssh command line preparation to a separate functionMike Hommey, May 3, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.