Skip to content

Commit df956a6

Browse files
andyfellerandyfeller
committed
Initial fix for browsing URLs
This commit enhances `Browser` logic within `pkg/browser` to detect supported and possibly supported URL schemes / protocols. If a URL is presented that doesn't match a known supported scheme/protocol, then `Browse(string) error` will check if the provided URL matches a file or directory on the filesystem. Erring if this matches anything we can find on the filesystem.
1 parent 61bf393 commit df956a6

2 files changed

Lines changed: 188 additions & 8 deletions

File tree

‎pkg/browser/browser.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
package browser
33

44
import (
5+
"fmt"
56
"io"
67
"os"
78
"os/exec"
9+
"strings"
810

911
cliBrowser "github.com/cli/browser"
1012
"github.com/cli/go-gh/v2/pkg/config"
@@ -45,6 +47,10 @@ func (b *Browser) Browse(url string) error {
4547
}
4648

4749
func (b *Browser) browse(url string, env []string) error {
50+
if !isPossibleProtocol(url) {
51+
return fmt.Errorf("cannot browse due to unsupported protocol: %s", url)
52+
}
53+
4854
if b.launcher == "" {
4955
return cliBrowser.OpenURL(url)
5056
}
@@ -78,3 +84,32 @@ func resolveLauncher() string {
7884
}
7985
return os.Getenv("BROWSER")
8086
}
87+
88+
func (b *Browser) IsURL(u string) bool {
89+
return isPossibleProtocol(u)
90+
}
91+
92+
func isSupportedProtocol(u string) bool {
93+
return strings.HasPrefix(u, "http:") ||
94+
strings.HasPrefix(u, "https:") ||
95+
strings.HasPrefix(u, "vscode:") ||
96+
strings.HasPrefix(u, "vscode-insiders:")
97+
}
98+
99+
func isPossibleProtocol(u string) bool {
100+
if isSupportedProtocol(u) {
101+
return true
102+
}
103+
104+
// Disallow URLs that match existing files or directorys on the filesystem
105+
if fileInfo, _ := os.Lstat(u); fileInfo != nil {
106+
return false
107+
}
108+
109+
// Disallow URLs using alternative `file:` protocol not located previously
110+
if strings.HasPrefix(u, "file:") {
111+
return false
112+
}
113+
114+
return true
115+
}

‎pkg/browser/browser_test.go‎

Lines changed: 153 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import (
44
"bytes"
55
"fmt"
66
"os"
7+
"path/filepath"
78
"testing"
89

910
"github.com/cli/go-gh/v2/pkg/config"
1011
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1113
)
1214

1315
func TestHelperProcess(t *testing.T) {
@@ -19,14 +21,157 @@ func TestHelperProcess(t *testing.T) {
1921
}
2022

2123
func TestBrowse(t *testing.T) {
22-
launcher := fmt.Sprintf("%q -test.run=TestHelperProcess -- chrome", os.Args[0])
23-
stdout := &bytes.Buffer{}
24-
stderr := &bytes.Buffer{}
25-
b := Browser{launcher: launcher, stdout: stdout, stderr: stderr}
26-
err := b.browse("github.com", []string{"GH_WANT_HELPER_PROCESS=1"})
27-
assert.NoError(t, err)
28-
assert.Equal(t, "[chrome github.com]", stdout.String())
29-
assert.Equal(t, "", stderr.String())
24+
tests := []struct {
25+
name string
26+
url string
27+
launcher string
28+
expected string
29+
setup func(*testing.T) error
30+
wantErr bool
31+
}{
32+
{
33+
name: "Explicit `http` URL works",
34+
url: "http://github.com",
35+
launcher: fmt.Sprintf("%q -test.run=TestHelperProcess -- explicit http", os.Args[0]),
36+
expected: "[explicit http http://github.com]",
37+
},
38+
{
39+
name: "Explicit `https` URL works",
40+
url: "https://github.com",
41+
launcher: fmt.Sprintf("%q -test.run=TestHelperProcess -- explicit https", os.Args[0]),
42+
expected: "[explicit https https://github.com]",
43+
},
44+
{
45+
name: "Explicit `vscode` URL works",
46+
url: "vscode:extension/GitHub.copilot",
47+
launcher: fmt.Sprintf("%q -test.run=TestHelperProcess -- explicit vscode", os.Args[0]),
48+
expected: "[explicit vscode vscode:extension/GitHub.copilot]",
49+
},
50+
{
51+
name: "Explicit `vscode-insiders` URL works",
52+
url: "vscode-insiders:extension/GitHub.copilot",
53+
launcher: fmt.Sprintf("%q -test.run=TestHelperProcess -- explicit vscode-insiders", os.Args[0]),
54+
expected: "[explicit vscode-insiders vscode-insiders:extension/GitHub.copilot]",
55+
},
56+
{
57+
name: "Implicit `https` URL works",
58+
url: "github.com",
59+
launcher: fmt.Sprintf("%q -test.run=TestHelperProcess -- implicit https", os.Args[0]),
60+
expected: "[implicit https github.com]",
61+
},
62+
{
63+
name: "Explicit absolute `file://` URL errors",
64+
url: "file:///System/Applications/Calculator.app",
65+
wantErr: true,
66+
},
67+
{
68+
name: "Implicit absolute file URL errors",
69+
url: "/bin/sh",
70+
wantErr: true,
71+
},
72+
{
73+
name: "Implicit absolute directory URL errors",
74+
url: "/System/Applications/Calculator.app",
75+
wantErr: true,
76+
},
77+
{
78+
name: "Explicit absolute Windows file URL errors",
79+
url: `C:\Windows\System32\calc.exe`,
80+
wantErr: true,
81+
},
82+
{
83+
name: "Implicit relative file URL errors",
84+
url: "poc.command",
85+
setup: func(t *testing.T) error {
86+
// Capture current working directory for test cleanup
87+
cwd, err := os.Getwd()
88+
if err != nil {
89+
return err
90+
}
91+
92+
// Create temporary directory containing relative executable for testing
93+
tempDir := t.TempDir()
94+
err = os.Chdir(tempDir)
95+
if err != nil {
96+
return err
97+
}
98+
99+
path := filepath.Join(tempDir, "poc.command")
100+
err = os.WriteFile(path, []byte("#!/bin/bash\necho hello"), 0755)
101+
if err != nil {
102+
return err
103+
}
104+
105+
// Restore original working directory after test
106+
t.Cleanup(func() {
107+
_ = os.Chdir(cwd)
108+
})
109+
110+
return nil
111+
},
112+
wantErr: true,
113+
},
114+
{
115+
name: "Implicit relative directory URL errors",
116+
url: "poc.command",
117+
setup: func(t *testing.T) error {
118+
// Capture current working directory for test cleanup
119+
cwd, err := os.Getwd()
120+
if err != nil {
121+
return err
122+
}
123+
124+
// Create temporary directory containing relative executable for testing
125+
tempDir := t.TempDir()
126+
err = os.Chdir(tempDir)
127+
if err != nil {
128+
return err
129+
}
130+
131+
path := filepath.Join(tempDir, "Fake.app")
132+
err = os.Mkdir(path, 0755)
133+
if err != nil {
134+
return err
135+
}
136+
137+
path = filepath.Join(path, "poc.command")
138+
err = os.WriteFile(path, []byte("#!/bin/bash\necho hello"), 0755)
139+
if err != nil {
140+
return err
141+
}
142+
143+
// Restore original working directory after test
144+
t.Cleanup(func() {
145+
_ = os.Chdir(cwd)
146+
})
147+
148+
return nil
149+
},
150+
wantErr: true,
151+
},
152+
}
153+
154+
for _, tt := range tests {
155+
t.Run(tt.name, func(t *testing.T) {
156+
if tt.setup != nil {
157+
err := tt.setup(t)
158+
require.NoError(t, err)
159+
}
160+
161+
stdout := &bytes.Buffer{}
162+
stderr := &bytes.Buffer{}
163+
b := Browser{launcher: tt.launcher, stdout: stdout, stderr: stderr}
164+
err := b.browse(tt.url, []string{"GH_WANT_HELPER_PROCESS=1"})
165+
166+
if tt.wantErr {
167+
require.Error(t, err)
168+
} else {
169+
require.NoError(t, err)
170+
assert.Equal(t, tt.expected, stdout.String())
171+
assert.Equal(t, "", stderr.String())
172+
}
173+
})
174+
}
30175
}
31176

32177
func TestResolveLauncher(t *testing.T) {

0 commit comments

Comments
 (0)