Skip to content

Commit 879e387

Browse files
committed
fix(proxy): fix a case when URL is nil
1 parent 4fec245 commit 879e387

3 files changed

Lines changed: 53 additions & 1 deletion

File tree

internal/proxy/reqlog.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,20 @@ func (rl *RequestLogger) toLogEntry(req *RequestLog) *logging.Entry {
153153

154154
// LogRequest captures request details and returns a log entry
155155
func (rl *RequestLogger) LogRequest(req *http.Request) (*RequestLog, []byte) {
156+
// req.URL can be nil when goproxy's HTTPS handler fails to re-parse a
157+
// malformed request line - the parse error is swallowed and the request
158+
// is still dispatched. Fall back to RequestURI so logging never panics.
159+
// https://github.com/elazarl/goproxy/blob/v1.8.3/https.go#L272-L274
160+
urlStr := ""
161+
if req.URL != nil {
162+
urlStr = req.URL.String()
163+
} else {
164+
urlStr = req.RequestURI
165+
}
156166
entry := &RequestLog{
157167
Timestamp: time.Now(),
158168
Method: req.Method,
159-
URL: req.URL.String(),
169+
URL: urlStr,
160170
RequestHeaders: redactHeaders(cloneHeaders(req.Header)),
161171
}
162172

internal/proxy/reqlog_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,32 @@ func TestLogRequest_RedactsSensitiveHeaders(t *testing.T) {
5959
}
6060
}
6161

62+
// Regression: goproxy can dispatch HTTPS requests with a nil URL when its
63+
// internal url.Parse fallback fails. LogRequest must not panic in that case.
64+
// https://github.com/elazarl/goproxy/blob/v1.8.3/https.go#L272-L274
65+
func TestLogRequest_NilURL(t *testing.T) {
66+
dir := t.TempDir()
67+
rl, err := NewRequestLogger(dir, nil, false, nil)
68+
if err != nil {
69+
t.Fatal(err)
70+
}
71+
defer func() { _ = rl.Close() }()
72+
73+
req := &http.Request{
74+
Method: "GET",
75+
RequestURI: "/some/path",
76+
Header: http.Header{},
77+
}
78+
79+
entry, _ := rl.LogRequest(req)
80+
if entry == nil {
81+
t.Fatal("expected entry, got nil")
82+
}
83+
if entry.URL != "/some/path" {
84+
t.Errorf("URL = %q, want %q (falling back to RequestURI)", entry.URL, "/some/path")
85+
}
86+
}
87+
6288
func TestRequestLog_RedactionFields(t *testing.T) {
6389
entry := &RequestLog{
6490
Method: "POST",

internal/proxy/server.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,22 @@ func (s *Server) setupLogging() {
255255
entry, reqBody := s.reqLogger.LogRequest(req)
256256
ctx.UserData = entry
257257

258+
// goproxy can dispatch HTTPS requests with a nil URL when its own
259+
// url.Parse fallback fails. Every downstream step here (credential
260+
// injection, filtering, redaction, ask mode) dereferences req.URL,
261+
// so we reject with 403 rather than panic.
262+
// https://github.com/elazarl/goproxy/blob/v1.8.3/https.go#L272-L274
263+
if req.URL == nil {
264+
resp := BlockResponse(req, "malformed request: missing URL")
265+
if entry != nil {
266+
entry.FilterAction = string(FilterActionBlock)
267+
entry.FilterReason = "malformed request: missing URL"
268+
s.reqLogger.LogResponse(entry, resp, entry.Timestamp)
269+
_ = s.reqLogger.Log(entry)
270+
}
271+
return nil, resp
272+
}
273+
258274
// Inject credentials for matching domains
259275
for _, injector := range s.credentialInjectors {
260276
if injector.Match(req) {

0 commit comments

Comments
 (0)