Merge pull request #610 from gabemontero/s2i-bump

Bug 1884270: bypass golang url parsing with scp styled ssh git URLs; refactor URL for older git clients
openshift · Oct 9, 2020 · e053f7b · e053f7b
2 parents 3a37e92 + 1149c70
commit e053f7b
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 5 deletions.
diff --git a/pkg/helpers/source-to-image/git/url.go b/pkg/helpers/source-to-image/git/url.go
@@ -86,6 +86,8 @@ func splitOnByte(s string, c byte) (string, string) {
 // Parse parses a "Git URL"
 func Parse(rawurl string) (*URL, error) {
 	if urlSchemeRegexp.MatchString(rawurl) &&
+		// at least through golang 1.14.9, url.Parse cannot handle ssh://git@github.com:sclorg/nodejs-ex
+		!strings.HasPrefix(rawurl, "ssh://") &&
 		(runtime.GOOS != "windows" || !dosDriveRegexp.MatchString(rawurl)) {
 		u, err := url.Parse(rawurl)
 		if err != nil {
@@ -106,6 +108,12 @@ func Parse(rawurl string) (*URL, error) {
 		}, nil
 	}
 
+	// if ssh://git@github.com:sclorg/nodejs-ex then strip ssh:// a la what we
+	// see in other upstream git parsing handling of scp styled git URLs;
+	if strings.HasPrefix(rawurl, "ssh://") {
+		rawurl = strings.Trim(rawurl, "ssh://")
+	}
+
 	s, fragment := splitOnByte(rawurl, '#')
 
 	if m := scpRegexp.FindStringSubmatch(s); m != nil &&

diff --git a/pkg/helpers/source-to-image/git/url_test.go b/pkg/helpers/source-to-image/git/url_test.go
@@ -10,6 +10,7 @@ import (
 
 type parseTest struct {
 	rawurl         string
+	ammendedRawURL string
 	expectedGitURL *URL
 	expectedError  bool
 }
@@ -184,6 +185,19 @@ func TestParse(t *testing.T) {
 				Type: URLTypeSCP,
 			},
 		},
+		parseTest{
+			rawurl:         "ssh://git@github.com:sclorg/nodejs-ex",
+			ammendedRawURL: "git@github.com:sclorg/nodejs-ex",
+			expectedGitURL: &URL{
+				URL: url.URL{
+					User: url.User("git"),
+					Host: "github.com",
+					Path: "sclorg/nodejs-ex",
+				},
+				Type: URLTypeSCP,
+			},
+			expectedError: false,
+		},
 
 		// path ...
 		parseTest{
@@ -221,12 +235,23 @@ func TestParse(t *testing.T) {
 			t.Errorf("%s: Parse() returned\n\t%#v\nWanted\n\t%#v", test.rawurl, parsedURL, test.expectedGitURL)
 		}
 
-		if parsedURL.String() != test.rawurl {
-			t.Errorf("%s: String() returned %s", test.rawurl, parsedURL.String())
-		}
+		if len(test.ammendedRawURL) > 0 {
+			if parsedURL.String() != test.ammendedRawURL {
+				t.Errorf("%s: String() returned %s", test.ammendedRawURL, parsedURL.String())
+			}
+
+			if parsedURL.StringNoFragment() != strings.SplitN(test.ammendedRawURL, "#", 2)[0] {
+				t.Errorf("%s: StringNoFragment() returned %s", test.ammendedRawURL, parsedURL.StringNoFragment())
+			}
+
+		} else {
+			if parsedURL.String() != test.rawurl {
+				t.Errorf("%s: String() returned %s", test.rawurl, parsedURL.String())
+			}
 
-		if parsedURL.StringNoFragment() != strings.SplitN(test.rawurl, "#", 2)[0] {
-			t.Errorf("%s: StringNoFragment() returned %s", test.rawurl, parsedURL.StringNoFragment())
+			if parsedURL.StringNoFragment() != strings.SplitN(test.rawurl, "#", 2)[0] {
+				t.Errorf("%s: StringNoFragment() returned %s", test.rawurl, parsedURL.StringNoFragment())
+			}
 		}
 	}
 }