WIP-PROGRESS

git-lfs · Nov 14, 2022 · 6622d27 · 6622d27
1 parent a803f51
commit 6622d27
Show file tree

Hide file tree

Showing 8 changed files with 284 additions and 58 deletions.
diff --git a/commands/command_env.go b/commands/command_env.go
@@ -29,7 +29,7 @@ func envCommand(cmd *cobra.Command, args []string) {
 			access := getAPIClient().Endpoints.AccessFor(endpoint.Url)
 			Print("Endpoint=%s (auth=%s)", endpoint.Url, access.Mode())
 			if len(endpoint.SSHMetadata.UserAndHost) > 0 {
-				Print("  SSH=%s", ssh.URLFromMetadata(endpoint.SSHMetadata))
+				Print("  SSH=%s", ssh.GetURIOrGitSyntax(endpoint.SSHMetadata))
 			}
 		}
 	}
@@ -43,7 +43,7 @@ func envCommand(cmd *cobra.Command, args []string) {
 		Print("Endpoint (%s)=%s (auth=%s)", remote, remoteEndpoint.Url, remoteAccess.Mode())
 		if len(remoteEndpoint.SSHMetadata.UserAndHost) > 0 {
 
-			Print("  SSH=%s", ssh.URLFromMetadata(remoteEndpoint.SSHMetadata))
+			Print("  SSH=%s", ssh.GetURIOrGitSyntax(remoteEndpoint.SSHMetadata))
 		}
 	}
 

diff --git a/lfsapi/auth.go b/lfsapi/auth.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"os"
 	"strings"
 
 	"github.com/git-lfs/git-lfs/v3/creds"
@@ -284,7 +283,6 @@ func setRequestAuthFromURL(req *http.Request, u *url.URL) bool {
 	}
 
 	if pass, ok := u.User.Password(); ok {
-		fmt.Fprintln(os.Stderr, tr.Tr.Get("warning: current Git remote contains credentials"))
 		setRequestAuth(req, u.User.Username(), pass)
 		return true
 	}

diff --git a/lfsapi/endpoint_finder_test.go b/lfsapi/endpoint_finder_test.go
@@ -208,7 +208,7 @@ func TestGitSSHEndpointAddsLfsSuffix(t *testing.T) {
 	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
 	assert.Equal(t, "/foo/bar", e.SSHMetadata.Path)
 	assert.Equal(t, "", e.SSHMetadata.Port)
-	assert.Equal(t, "git+ssh", e.SSHMetadata.Scheme)
+	assert.Equal(t, "ssh", e.SSHMetadata.Scheme)
 }
 
 func TestGitSSHCustomPortEndpointAddsLfsSuffix(t *testing.T) {
@@ -221,7 +221,7 @@ func TestGitSSHCustomPortEndpointAddsLfsSuffix(t *testing.T) {
 	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
 	assert.Equal(t, "/foo/bar", e.SSHMetadata.Path)
 	assert.Equal(t, "9000", e.SSHMetadata.Port)
-	assert.Equal(t, "git+ssh", e.SSHMetadata.Scheme)
+	assert.Equal(t, "ssh", e.SSHMetadata.Scheme)
 }
 
 func TestSSHGitEndpointAddsLfsSuffix(t *testing.T) {
@@ -234,7 +234,7 @@ func TestSSHGitEndpointAddsLfsSuffix(t *testing.T) {
 	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
 	assert.Equal(t, "/foo/bar", e.SSHMetadata.Path)
 	assert.Equal(t, "", e.SSHMetadata.Port)
-	assert.Equal(t, "ssh+git", e.SSHMetadata.Scheme)
+	assert.Equal(t, "ssh", e.SSHMetadata.Scheme)
 }
 
 func TestSSHGitCustomPortEndpointAddsLfsSuffix(t *testing.T) {
@@ -247,7 +247,7 @@ func TestSSHGitCustomPortEndpointAddsLfsSuffix(t *testing.T) {
 	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
 	assert.Equal(t, "/foo/bar", e.SSHMetadata.Path)
 	assert.Equal(t, "9000", e.SSHMetadata.Port)
-	assert.Equal(t, "ssh+git", e.SSHMetadata.Scheme)
+	assert.Equal(t, "ssh", e.SSHMetadata.Scheme)
 }
 
 func TestBareSSHEndpointAddsLfsSuffix(t *testing.T) {
@@ -276,7 +276,6 @@ func TestBareSSSHEndpointWithCustomPortInBrackets(t *testing.T) {
 	assert.Equal(t, "", e.SSHMetadata.Scheme)
 }
 
-/**** DEBUG chrisd -
 func TestBareSSSHEndpointWithCustomPortInBracketsAfterUser(t *testing.T) {
 	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{
 		"remote.origin.url": "git@[example.com:2222]:foo/bar.git",
@@ -289,7 +288,71 @@ func TestBareSSSHEndpointWithCustomPortInBracketsAfterUser(t *testing.T) {
 	assert.Equal(t, "2222", e.SSHMetadata.Port)
 	assert.Equal(t, "", e.SSHMetadata.Scheme)
 }
-*/
+
+func TestBareSSSHEndpointWithCustomPortInBracketsAfterUserWithIgnoredCharsAfterBrackets(t *testing.T) {
+	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{
+		"remote.origin.url": "git@[example.com:2222]]xx:foo/bar.git",
+	}))
+
+	e := finder.Endpoint("download", "")
+	assert.Equal(t, "https://example.com/foo/bar.git/info/lfs", e.Url)
+	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
+	assert.Equal(t, "foo/bar.git", e.SSHMetadata.Path)
+	assert.Equal(t, "2222", e.SSHMetadata.Port)
+	assert.Equal(t, "", e.SSHMetadata.Scheme)
+}
+
+func TestBareSSSHEndpointInBracketsAfterUser(t *testing.T) {
+	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{
+		"remote.origin.url": "git@[example.com]:foo/bar.git",
+	}))
+
+	e := finder.Endpoint("download", "")
+	assert.Equal(t, "https://example.com/foo/bar.git/info/lfs", e.Url)
+	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
+	assert.Equal(t, "foo/bar.git", e.SSHMetadata.Path)
+	assert.Equal(t, "", e.SSHMetadata.Port)
+	assert.Equal(t, "", e.SSHMetadata.Scheme)
+}
+
+func TestBareSSSHEndpointInBrackets(t *testing.T) {
+	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{
+		"remote.origin.url": "[git@example.com]:foo/bar.git",
+	}))
+
+	e := finder.Endpoint("download", "")
+	assert.Equal(t, "https://example.com/foo/bar.git/info/lfs", e.Url)
+	assert.Equal(t, "git@example.com", e.SSHMetadata.UserAndHost)
+	assert.Equal(t, "foo/bar.git", e.SSHMetadata.Path)
+	assert.Equal(t, "", e.SSHMetadata.Port)
+	assert.Equal(t, "", e.SSHMetadata.Scheme)
+}
+
+func TestBareSSSHEndpointIncompleteBracketsAfterUser(t *testing.T) {
+	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{
+		"remote.origin.url": "git@[example.com:foo/bar.git",
+	}))
+
+	e := finder.Endpoint("download", "")
+	assert.Equal(t, "https://[example.com/foo/bar.git/info/lfs", e.Url)
+	assert.Equal(t, "git@[example.com", e.SSHMetadata.UserAndHost)
+	assert.Equal(t, "foo/bar.git", e.SSHMetadata.Path)
+	assert.Equal(t, "", e.SSHMetadata.Port)
+	assert.Equal(t, "", e.SSHMetadata.Scheme)
+}
+
+func TestBareSSSHEndpointIncompleteBrackets(t *testing.T) {
+	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{
+		"remote.origin.url": "exam[ple.com:foo/bar.git",
+	}))
+
+	e := finder.Endpoint("download", "")
+	assert.Equal(t, "https://exam[ple.com/foo/bar.git/info/lfs", e.Url)
+	assert.Equal(t, "exam[ple.com", e.SSHMetadata.UserAndHost)
+	assert.Equal(t, "foo/bar.git", e.SSHMetadata.Path)
+	assert.Equal(t, "", e.SSHMetadata.Port)
+	assert.Equal(t, "", e.SSHMetadata.Scheme)
+}
 
 func TestSSHEndpointFromGlobalLfsUrl(t *testing.T) {
 	finder := NewEndpointFinder(lfshttp.NewContext(nil, nil, map[string]string{

diff --git a/lfshttp/endpoint.go b/lfshttp/endpoint.go
@@ -32,13 +32,13 @@ func endpointOperation(e Endpoint, method string) string {
 	}
 }
 
-var sshURIHostPortRE = regexp.MustCompile(`^([^\:]+)(?:\:(\d+))?$`)
+var sshHostPortRE = regexp.MustCompile(`^([^\:]+)(?:\:(\d+))?$`)
 
 // EndpointFromSshUrl constructs a new endpoint from an ssh:// URL
 func EndpointFromSshUrl(u *url.URL) Endpoint {
 	var endpoint Endpoint
 	// Pull out port now, we need it separately for SSH
-	match := sshURIHostPortRE.FindStringSubmatch(u.Host)
+	match := sshHostPortRE.FindStringSubmatch(u.Host)
 	if match == nil || len(match) < 2 {
 		endpoint.Url = UrlUnknown
 		return endpoint
@@ -56,7 +56,9 @@ func EndpointFromSshUrl(u *url.URL) Endpoint {
 	}
 
 	endpoint.SSHMetadata.Path = u.Path
-	endpoint.SSHMetadata.Scheme = u.Scheme
+
+	// Always use ssh scheme instead of deprecated git+ssh or ssh+git.
+	endpoint.SSHMetadata.Scheme = "ssh"
 
 	// Fallback URL for using HTTPS while still using SSH for git
 	// u.Host includes host & port so can't use SSH port
@@ -70,47 +72,65 @@ func EndpointFromSshUrl(u *url.URL) Endpoint {
 //   user@host.com:path/to/repo.git or
 //   [user@host.com:port]:path/to/repo.git
 //
+// We emulate the relevant logic from Git's parse_connect_url() and
+// host_end() functions in connect.c:
+//   https://github.com/git/git/blob/0f828332d5ac36fc63b7d8202652efa152809856/connect.c#L673-L695
+//   https://github.com/git/git/blob/0f828332d5ac36fc63b7d8202652efa152809856/connect.c#L1051 
 func EndpointFromBareSshUrl(rawurl string) Endpoint {
-	parts := strings.SplitN(rawurl, ":", 3)
-	partsLen := len(parts)
-	if partsLen < 2 {
-		return Endpoint{Url: rawurl}
+	var userHostAndPort string
+	toParse := rawurl
+	if i := strings.Index(rawurl, "@["); i >= 0 {
+		userHostAndPort = rawurl[0:i + 1]
+		toParse = rawurl[i + 1:]
 	}
 
-	// Treat presence of ':' as a bare URL
-	var userHostAndPort string
-	var path string
-	if len(parts) > 2 { // port included; really should only ever be 3 parts
-		// Correctly handle [host:port]:path URLs
-//// DEBUG: user@[host:port]:... also OK
-//// if [ found ] must be found too (and last!)
-//// DEBUG: need tests for these
-		parts[0] = strings.TrimPrefix(parts[0], "[")
-		parts[1] = strings.TrimSuffix(parts[1], "]")
-		userHostAndPort = fmt.Sprintf("%v:%v", parts[0], parts[1])
-		path = parts[2]
-	} else {
-		userHostAndPort = parts[0]
-		path = parts[1]
+	var bracketed bool
+	if toParse[0] == '[' {
+		if i := strings.Index(toParse, "]"); i >= 0 {
+			userHostAndPort += toParse[1:i]
+			toParse = toParse[i + 1:]
+			bracketed = true
+		}
 	}
 
-	var absPath bool
-	if absPath = strings.HasPrefix(path, "/"); absPath {
-		path = strings.TrimLeft(path, "/")
+	i := strings.Index(toParse, ":")
+	if i < 0 {
+		return Endpoint{Url: rawurl}
+	}
+	path := toParse[i + 1:]
+	if !bracketed {
+		userHostAndPort += toParse[0:i]
 	}
 
-	newrawurl := fmt.Sprintf("ssh://%v/%v", userHostAndPort, path)
-	newu, err := url.Parse(newrawurl)
-	if err != nil {
+//// DEBUG: rename functions to use SSH, GitSyntax, etc.
+//// DEBUG: rename rawurl
+// https://github.com/golang/go/wiki/CodeReviewComments#initialisms
+
+//// DEBUG: endpoint_finder_test.go -- add tests
+//// DEBUG: t-env.sh - split into multiple tests to avoid false success
+////                 - also test ssh:// and git+ssh://, etc.
+
+//// note that IPv6 should be done via ssh:// only
+
+	match := sshHostPortRE.FindStringSubmatch(userHostAndPort)
+	if match == nil || len(match) < 2 {
 		return Endpoint{Url: UrlUnknown}
 	}
 
-	endpoint := EndpointFromSshUrl(newu)
-	if !absPath {
-		endpoint.SSHMetadata.Path = strings.TrimLeft(endpoint.SSHMetadata.Path, "/")
+	var endpoint Endpoint
+	endpoint.SSHMetadata.UserAndHost = match[1]
+	if len(match) > 2 {
+		endpoint.SSHMetadata.Port = match[2]
 	}
-//// DEBUG: skip Scheme and just canonicalize as bare SSH in env
-	endpoint.SSHMetadata.Scheme = ""
+	endpoint.SSHMetadata.Path = path
+
+	// Fallback URL for using HTTPS while still using SSH for git
+	host := endpoint.SSHMetadata.UserAndHost
+	if i = strings.Index(host, "@"); i >= 0 {
+		host = host[i + 1:]
+	}
+	endpoint.Url = fmt.Sprintf("https://%s/%s", host, strings.TrimLeft(path, "/"))
+
 	return endpoint
 }
 

diff --git a/ssh/ssh.go b/ssh/ssh.go
@@ -31,25 +31,19 @@ type SSHMetadata struct {
 	Scheme      string
 }
 
-//// DEBUG chrisd - use different name if canonicalizing as bare always
-// URLFromMetadata constructs a bare SSH URL or an SSH URI from metadata
-func URLFromMetadata(meta SSHMetadata) string {
+func GetURIOrGitSyntax(meta SSHMetadata) string {
 	userHostAndPort := meta.UserAndHost
-	port := meta.Port
-	if port != "" {
-		userHostAndPort = fmt.Sprintf("%s:%s", userHostAndPort, port)
+	if meta.Port != "" {
+		userHostAndPort = fmt.Sprintf("%s:%s", userHostAndPort, meta.Port)
 	}
-
-	var url string
 	if meta.Scheme == "" {
-		if port != "" {
+		if meta.Port != "" {
 			userHostAndPort = fmt.Sprintf("[%s]", userHostAndPort)
 		}
-		url = fmt.Sprintf("%s:%s", userHostAndPort, meta.Path)
+		return fmt.Sprintf("%s:%s", userHostAndPort, meta.Path)
 	} else {
-		url = fmt.Sprintf("%s://%s%s", meta.Scheme, userHostAndPort, meta.Path)
+		return fmt.Sprintf("%s://%s%s", meta.Scheme, userHostAndPort, meta.Path)
 	}
-	return url
 }
 
 func FormatArgs(cmd string, args []string, needShell bool) (string, []string) {

diff --git a/t/cmd/lfs-ssh-echo.go b/t/cmd/lfs-ssh-echo.go
@@ -53,12 +53,17 @@ func checkSufficientArgs(offset int) {
 
 func main() {
 	// expect args:
-	//   lfs-ssh-echo [-p PORT [--]] git@127.0.0.1 "git-lfs-authenticate REPO OPERATION"
-	//   lfs-ssh-echo [-p PORT [--]] git@127.0.0.1 "git-lfs-transfer REPO OPERATION"
+	//   lfs-ssh-echo [-o OPT] [-p PORT [--]] git@127.0.0.1 "git-lfs-authenticate REPO OPERATION"
+	//   lfs-ssh-echo [-o OPT] [-p PORT [--]] git@127.0.0.1 "git-lfs-transfer REPO OPERATION"
 	//   lfs-ssh-echo git@127.0.0.1 "git-upload-pack REPO"
 	//   lfs-ssh-echo git@127.0.0.1 "git-receive-pack REPO"
 	offset := 1
 
+	checkSufficientArgs(offset)
+	if os.Args[offset] == "-o" {
+		offset += 2
+	}
+
 	checkSufficientArgs(offset)
 	if os.Args[offset] == "-p" {
 		offset += 2