Skip to content

Commit 233af2c

Browse files
Mzack9999Mzack9999
authored
fix(websocket): path handling when merging template & target URLs (#7290)
1 parent c2f3397 commit 233af2c

2 files changed

Lines changed: 264 additions & 2 deletions

File tree

‎pkg/protocols/websocket/websocket.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"net"
99
"net/http"
1010
"net/url"
11-
"path"
1211
"strings"
1312
"time"
1413

@@ -227,7 +226,9 @@ func (request *Request) executeRequestWithPayloads(target *contextargs.Context,
227226
requestOptions.Progress.IncrementFailedRequestsBy(1)
228227
return errors.Wrap(err, parseUrlErrorMessage)
229228
}
230-
parsedAddress.Path = path.Join(parsedAddress.Path, parsed.Path)
229+
if parsedAddress.Path == "" || parsedAddress.Path == "/" {
230+
parsedAddress.Path = parsed.Path
231+
}
231232
addressToDial = parsedAddress.String()
232233

233234
conn, readBuffer, _, err := websocketDialer.Dial(target.Context(), addressToDial)

‎pkg/protocols/websocket/websocket_test.go‎

Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
package websocket
2+
3+
import (
4+
"net/url"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
urlutil "github.com/projectdiscovery/utils/url"
9+
)
10+
11+
// resolveAddress mirrors the path resolution logic in executeRequestWithPayloads.
12+
// it parses the template address and the input URL then applies the path rule.
13+
func resolveAddress(templateAddress, inputURL string) (string, error) {
14+
parsedAddress, err := url.Parse(templateAddress)
15+
if err != nil {
16+
return "", err
17+
}
18+
parsed, err := urlutil.Parse(inputURL)
19+
if err != nil {
20+
return "", err
21+
}
22+
if parsedAddress.Path == "" || parsedAddress.Path == "/" {
23+
parsedAddress.Path = parsed.Path
24+
}
25+
return parsedAddress.String(), nil
26+
}
27+
28+
func TestAddressResolution(t *testing.T) {
29+
tests := []struct {
30+
name string
31+
templateAddress string
32+
inputURL string
33+
expected string
34+
}{
35+
// template already has a path so we keep it and don't double
36+
{
37+
name: "same path in both - no doubling",
38+
templateAddress: "wss://jenkins.cloud/cli/ws",
39+
inputURL: "https://jenkins.cloud/cli/ws",
40+
expected: "wss://jenkins.cloud/cli/ws",
41+
},
42+
{
43+
name: "different paths - template path preserved",
44+
templateAddress: "wss://example.com/ws/connect",
45+
inputURL: "https://example.com/api/v1",
46+
expected: "wss://example.com/ws/connect",
47+
},
48+
{
49+
name: "deep template path preserved",
50+
templateAddress: "wss://example.com/a/b/c/d",
51+
inputURL: "https://example.com/x/y",
52+
expected: "wss://example.com/a/b/c/d",
53+
},
54+
{
55+
name: "template path with trailing slash preserved",
56+
templateAddress: "wss://example.com/ws/",
57+
inputURL: "https://example.com/other",
58+
expected: "wss://example.com/ws/",
59+
},
60+
61+
// when the template has no path we fall back to the input path
62+
{
63+
name: "no template path - input path used",
64+
templateAddress: "wss://example.com",
65+
inputURL: "https://example.com/api/ws",
66+
expected: "wss://example.com/api/ws",
67+
},
68+
{
69+
name: "root template path - input path used",
70+
templateAddress: "wss://example.com/",
71+
inputURL: "https://example.com/chat/ws",
72+
expected: "wss://example.com/chat/ws",
73+
},
74+
{
75+
name: "no paths on either side",
76+
templateAddress: "wss://example.com",
77+
inputURL: "https://example.com",
78+
expected: "wss://example.com",
79+
},
80+
{
81+
name: "root template, root input",
82+
templateAddress: "wss://example.com/",
83+
inputURL: "https://example.com/",
84+
expected: "wss://example.com/",
85+
},
86+
87+
// ports should not affect path resolution
88+
{
89+
name: "template with port and path",
90+
templateAddress: "wss://example.com:8443/ws",
91+
inputURL: "https://example.com:8443/api",
92+
expected: "wss://example.com:8443/ws",
93+
},
94+
{
95+
name: "template with port, no path - input path used",
96+
templateAddress: "ws://example.com:9090",
97+
inputURL: "http://example.com:9090/stream",
98+
expected: "ws://example.com:9090/stream",
99+
},
100+
{
101+
name: "ws scheme with port and deep input path",
102+
templateAddress: "ws://example.com:8080",
103+
inputURL: "http://example.com:8080/api/v2/ws",
104+
expected: "ws://example.com:8080/api/v2/ws",
105+
},
106+
107+
// query strings should stay with their respective URLs
108+
{
109+
name: "template with query string preserved",
110+
templateAddress: "wss://example.com/ws?token=abc",
111+
inputURL: "https://example.com/other",
112+
expected: "wss://example.com/ws?token=abc",
113+
},
114+
{
115+
name: "input query string not leaked when template has path",
116+
templateAddress: "wss://example.com/ws",
117+
inputURL: "https://example.com/api?key=secret",
118+
expected: "wss://example.com/ws",
119+
},
120+
{
121+
name: "no template path - input path used but not query",
122+
templateAddress: "wss://example.com",
123+
inputURL: "https://example.com/stream?v=1",
124+
expected: "wss://example.com/stream",
125+
},
126+
127+
// both ws and wss schemes should behave the same way
128+
{
129+
name: "ws scheme template path preserved",
130+
templateAddress: "ws://example.com/plain",
131+
inputURL: "http://example.com/other",
132+
expected: "ws://example.com/plain",
133+
},
134+
{
135+
name: "wss scheme no path - input path used",
136+
templateAddress: "wss://secure.example.com",
137+
inputURL: "https://secure.example.com/endpoint",
138+
expected: "wss://secure.example.com/endpoint",
139+
},
140+
141+
// same logic applies to IP-based targets
142+
{
143+
name: "IPv4 template with path",
144+
templateAddress: "ws://192.168.1.1/ws",
145+
inputURL: "http://192.168.1.1/api",
146+
expected: "ws://192.168.1.1/ws",
147+
},
148+
{
149+
name: "IPv4 template no path - input path used",
150+
templateAddress: "ws://192.168.1.1",
151+
inputURL: "http://192.168.1.1/metrics/ws",
152+
expected: "ws://192.168.1.1/metrics/ws",
153+
},
154+
{
155+
name: "IPv4 with port, no path",
156+
templateAddress: "ws://10.0.0.1:3000",
157+
inputURL: "http://10.0.0.1:3000/graphql/ws",
158+
expected: "ws://10.0.0.1:3000/graphql/ws",
159+
},
160+
161+
// patterns from actual templates and bug reports
162+
{
163+
name: "jenkins websocket - the original bug report",
164+
templateAddress: "wss://jenkins-ci.corp.cloud/cli/ws",
165+
inputURL: "https://jenkins-ci.corp.cloud/cli/ws",
166+
expected: "wss://jenkins-ci.corp.cloud/cli/ws",
167+
},
168+
{
169+
name: "grafana live websocket",
170+
templateAddress: "wss://grafana.local/api/live/ws",
171+
inputURL: "https://grafana.local/api/live/ws",
172+
expected: "wss://grafana.local/api/live/ws",
173+
},
174+
{
175+
name: "generic host-only template with target path",
176+
templateAddress: "wss://target.example.com",
177+
inputURL: "https://target.example.com/socket.io/ws",
178+
expected: "wss://target.example.com/socket.io/ws",
179+
},
180+
}
181+
182+
for _, tt := range tests {
183+
t.Run(tt.name, func(t *testing.T) {
184+
result, err := resolveAddress(tt.templateAddress, tt.inputURL)
185+
require.NoError(t, err)
186+
require.Equal(t, tt.expected, result)
187+
})
188+
}
189+
}
190+
191+
func TestGetAddress(t *testing.T) {
192+
tests := []struct {
193+
name string
194+
input string
195+
want string
196+
wantErr bool
197+
}{
198+
{
199+
name: "ws scheme returns host",
200+
input: "ws://example.com/path",
201+
want: "example.com",
202+
},
203+
{
204+
name: "wss scheme returns host with port",
205+
input: "wss://example.com:8443/path",
206+
want: "example.com:8443",
207+
},
208+
{
209+
name: "ws with non-standard port",
210+
input: "ws://example.com:9090",
211+
want: "example.com:9090",
212+
},
213+
{
214+
name: "wss with standard port",
215+
input: "wss://example.com:443/ws",
216+
want: "example.com:443",
217+
},
218+
{
219+
name: "ws IPv4",
220+
input: "ws://192.168.1.1/ws",
221+
want: "192.168.1.1",
222+
},
223+
{
224+
name: "ws IPv4 with port",
225+
input: "ws://192.168.1.1:8080/ws",
226+
want: "192.168.1.1:8080",
227+
},
228+
{
229+
name: "http scheme rejected",
230+
input: "http://example.com",
231+
wantErr: true,
232+
},
233+
{
234+
name: "https scheme rejected",
235+
input: "https://example.com",
236+
wantErr: true,
237+
},
238+
{
239+
name: "ftp scheme rejected",
240+
input: "ftp://example.com",
241+
wantErr: true,
242+
},
243+
{
244+
name: "invalid URL",
245+
input: "://broken",
246+
wantErr: true,
247+
},
248+
}
249+
250+
for _, tt := range tests {
251+
t.Run(tt.name, func(t *testing.T) {
252+
got, err := getAddress(tt.input)
253+
if tt.wantErr {
254+
require.Error(t, err)
255+
return
256+
}
257+
require.NoError(t, err)
258+
require.Equal(t, tt.want, got)
259+
})
260+
}
261+
}

0 commit comments

Comments
 (0)