fix: extract folder correctly from repo URLs with trailing slash (#381)

Closes #380 Closes coder/customers#690
coder · Oct 4, 2024 · f50ec7a · f50ec7a
1 parent 46c97a4
commit f50ec7a
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 7 deletions.
diff --git a/integration/integration_test.go b/integration/integration_test.go
@@ -844,15 +844,17 @@ func TestContainerEnv(t *testing.T) {
 	require.NoError(t, err)
 
 	output := execContainer(t, ctr, "cat /env")
-	require.Contains(t, strings.TrimSpace(output),
-		`DEVCONTAINER=true
+	want := `DEVCONTAINER=true
 DEVCONTAINER_CONFIG=/workspaces/empty/.devcontainer/devcontainer.json
 ENVBUILDER=true
 FROM_CONTAINER_ENV=bar
 FROM_DOCKERFILE=foo
 FROM_REMOTE_ENV=baz
 PATH=/usr/local/bin:/bin:/go/bin:/opt
-REMOTE_BAR=bar`)
+REMOTE_BAR=bar`
+	if diff := cmp.Diff(want, strings.TrimSpace(output)); diff != "" {
+		require.Failf(t, "env mismatch", "diff (-want +got):\n%s", diff)
+	}
 }
 
 func TestUnsetOptionsEnv(t *testing.T) {

diff --git a/options/defaults.go b/options/defaults.go
@@ -2,6 +2,7 @@ package options
 
 import (
 	"fmt"
+	"path"
 	"strings"
 
 	"github.com/go-git/go-billy/v5/osfs"
@@ -25,12 +26,14 @@ func DefaultWorkspaceFolder(repoURL string) string {
 	if err != nil {
 		return EmptyWorkspaceDir
 	}
-	name := strings.Split(parsed.Path, "/")
-	hasOwnerAndRepo := len(name) >= 2
-	if !hasOwnerAndRepo {
+	repo := path.Base(parsed.Path)
+	// Giturls parsing never actually fails since ParseLocal never
+	// errors and places the entire URL in the Path field. This check
+	// ensures it's at least a Unix path containing forwardslash.
+	if repo == repoURL || repo == "/" || repo == "." || repo == "" {
 		return EmptyWorkspaceDir
 	}
-	repo := strings.TrimSuffix(name[len(name)-1], ".git")
+	repo = strings.TrimSuffix(repo, ".git")
 	return fmt.Sprintf("/workspaces/%s", repo)
 }
 

diff --git a/options/defaults_test.go b/options/defaults_test.go
@@ -35,11 +35,56 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
 			gitURL:   "https://username:password@github.com/coder/envbuilder.git",
 			expected: "/workspaces/envbuilder",
 		},
+		{
+			name:     "trailing",
+			gitURL:   "https://github.com/coder/envbuilder.git/",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "trailing-x2",
+			gitURL:   "https://github.com/coder/envbuilder.git//",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "no .git",
+			gitURL:   "https://github.com/coder/envbuilder",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "trailing no .git",
+			gitURL:   "https://github.com/coder/envbuilder/",
+			expected: "/workspaces/envbuilder",
+		},
 		{
 			name:     "fragment",
 			gitURL:   "https://github.com/coder/envbuilder.git#feature-branch",
 			expected: "/workspaces/envbuilder",
 		},
+		{
+			name:     "fragment-trailing",
+			gitURL:   "https://github.com/coder/envbuilder.git/#refs/heads/feature-branch",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "fragment-trailing no .git",
+			gitURL:   "https://github.com/coder/envbuilder/#refs/heads/feature-branch",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "space",
+			gitURL:   "https://github.com/coder/env%20builder.git",
+			expected: "/workspaces/env builder",
+		},
+		{
+			name:     "Unix path",
+			gitURL:   "/repo",
+			expected: "/workspaces/repo",
+		},
+		{
+			name:     "Unix subpath",
+			gitURL:   "/path/to/repo",
+			expected: "/workspaces/repo",
+		},
 		{
 			name:     "empty",
 			gitURL:   "",
@@ -65,6 +110,18 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
 			name:       "website URL",
 			invalidURL: "www.google.com",
 		},
+		{
+			name:       "Unix root",
+			invalidURL: "/",
+		},
+		{
+			name:       "Path consists entirely of slash",
+			invalidURL: "//",
+		},
+		{
+			name:       "Git URL with no path",
+			invalidURL: "http://127.0.0.1:41073",
+		},
 	}
 	for _, tt := range invalidTests {
 		t.Run(tt.name, func(t *testing.T) {