diff --git a/core/proxy.go b/core/proxy.go index fb668d266..4e662816e 100644 --- a/core/proxy.go +++ b/core/proxy.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/base64" "io" + "net" "net/http" "net/url" "regexp" @@ -22,6 +23,8 @@ import ( var ProxyAuthorizationHeader string +const errProxyAuthRequired = "407 Proxy authentication required" + // Creates goproxy.ProxyHttpServer and configures it to be used as a proxy for Hoverfly // goproxy is given handlers that use the Hoverfly request processing func NewProxy(hoverfly *Hoverfly) *goproxy.ProxyHttpServer { @@ -30,6 +33,27 @@ func NewProxy(hoverfly *Hoverfly) *goproxy.ProxyHttpServer { // creating proxy proxy := goproxy.NewProxyHttpServer() + // Fix #774: HTTP/1.1 clients may send requests with a relative URL (path only) and a Host header. + // goproxy's default NonproxyHandler returns 500 for these. Reconstruct the absolute URL from the + // Host header so the request is processed normally, matching the behaviour of NewWebserverProxy. + // + // Guard: if the Host header targets the proxy's own port the client is connecting directly to + // Hoverfly (not using it as a proxy). In that case keep the original 500 "is a proxy server" + // response — forwarding to ourselves would cause infinite recursion or a panic. + proxy.NonproxyHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Host == "" { + http.Error(w, "Cannot handle requests without Host header, e.g., HTTP 1.0", http.StatusBadRequest) + return + } + if _, port, err := net.SplitHostPort(r.Host); err == nil && port == hoverfly.Cfg.ProxyPort { + http.Error(w, "This is a proxy server. Does not respond to non-proxy requests.", http.StatusInternalServerError) + return + } + r.URL.Scheme = "http" + r.URL.Host = r.Host + proxy.ServeHTTP(w, r) + }) + if hoverfly.Cfg.AuthEnabled { log.Info("Enabling proxy authentication") proxyBasicAndBearer(proxy, "hoverfly", func(user, password string) bool { @@ -201,25 +225,25 @@ func authFromHeader(req *http.Request, basicFunc func(user, passwd string) bool, authheader := strings.SplitN(headerValue, " ", 2) req.Header.Del(ProxyAuthorizationHeader) if len(authheader) != 2 { - return fmt.Errorf("407 Proxy authentication required") + return fmt.Errorf(errProxyAuthRequired) } if authheader[0] == "Basic" { userpassraw, err := base64.StdEncoding.DecodeString(authheader[1]) if err != nil { - return fmt.Errorf("407 Proxy authentication required") + return fmt.Errorf(errProxyAuthRequired) } userpass := strings.SplitN(string(userpassraw), ":", 2) if len(userpass) != 2 { - return fmt.Errorf("407 Proxy authentication required") + return fmt.Errorf(errProxyAuthRequired) } result := basicFunc(userpass[0], userpass[1]) if result == false { - return fmt.Errorf("407 Proxy authentication required") + return fmt.Errorf(errProxyAuthRequired) } } else if authheader[0] == "Bearer" { result := bearerFunc(authheader[1]) if result == false { - return fmt.Errorf("407 Proxy authentication required") + return fmt.Errorf(errProxyAuthRequired) } } else { return fmt.Errorf("407 Unknown authentication type `%v`, only `Basic` or `Bearer` are supported", authheader[0]) diff --git a/core/proxy_test.go b/core/proxy_test.go index 5a9f459ba..c72cb6682 100644 --- a/core/proxy_test.go +++ b/core/proxy_test.go @@ -1,30 +1,38 @@ package hoverfly import ( + "bufio" + "fmt" + "net" "net/http" + "net/http/httptest" "net/url" "testing" - "bufio" - "net" - . "github.com/onsi/gomega" +) - "net/http/httptest" +const ( + testURL = "localhost:8888" + testProxyAuthHeader = "Proxy-Authorization" + testHost = "test.com" + testPath = "/testing" + testHostSSL = "test.com:443" + testSchemeHTTPS = "https://" ) func Test_authFromHeader_ShouldRemoveProxyAuthorizationHeader(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) - req.Header.Add("Proxy-Authorization", "something") + req, _ := http.NewRequest(http.MethodGet, testURL, nil) + req.Header.Add(testProxyAuthHeader, "something") authFromHeader(req, nil, nil) - Expect(req.Header).ToNot(HaveKey("Proxy-Authorization")) + Expect(req.Header).ToNot(HaveKey(testProxyAuthHeader)) } func Test_authFromHeader_ShouldRemoveXHoverflyAuthorizationHeader(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) + req, _ := http.NewRequest(http.MethodGet, testURL, nil) req.Header.Add("X-HOVERFLY-AUTHORIZATION", "something") authFromHeader(req, nil, nil) @@ -33,8 +41,8 @@ func Test_authFromHeader_ShouldRemoveXHoverflyAuthorizationHeader(t *testing.T) func Test_authFromHeader_ShouldReturnErrorIfNotBasicOrBearer(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) - req.Header.Add("Proxy-Authorization", "Something YmVuamloOlBhc3N3b3JkMTIz") + req, _ := http.NewRequest(http.MethodGet, testURL, nil) + req.Header.Add(testProxyAuthHeader, "Something YmVuamloOlBhc3N3b3JkMTIz") err := authFromHeader(req, nil, nil) @@ -44,8 +52,8 @@ func Test_authFromHeader_ShouldReturnErrorIfNotBasicOrBearer(t *testing.T) { func Test_authFromHeader_Basic_ShouldBase64DecodeUsernameAndPassword(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) - req.Header.Add("Proxy-Authorization", "Basic YmVuamloOlBhc3N3b3JkMTIz") + req, _ := http.NewRequest(http.MethodGet, testURL, nil) + req.Header.Add(testProxyAuthHeader, "Basic YmVuamloOlBhc3N3b3JkMTIz") var basicUsername, basicPassword string @@ -61,24 +69,24 @@ func Test_authFromHeader_Basic_ShouldBase64DecodeUsernameAndPassword(t *testing. func Test_authFromHeader_Basic_ShouldReturnFalseIfNotBase64Encoded(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) - req.Header.Add("Proxy-Authorization", "Basic benjih:Password123") + req, _ := http.NewRequest(http.MethodGet, testURL, nil) + req.Header.Add(testProxyAuthHeader, "Basic benjih:Password123") Expect(authFromHeader(req, nil, nil)).ToNot(BeNil()) } func Test_authFromHeader_Basic_ShouldReturnFalseIfDecodedBasicCredentialsArentFormattedCorrectly(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) - req.Header.Add("Proxy-Authorization", "Basic YmVuamlo") + req, _ := http.NewRequest(http.MethodGet, testURL, nil) + req.Header.Add(testProxyAuthHeader, "Basic YmVuamlo") Expect(authFromHeader(req, nil, nil)).ToNot(BeNil()) } func Test_authFromHeader_Bearer_ShouldPassJwtTokenOntoFunction(t *testing.T) { RegisterTestingT(t) - req, _ := http.NewRequest(http.MethodGet, "localhost:8888", nil) - req.Header.Add("Proxy-Authorization", "Bearer gregg.EEewGREQ.GDSG") + req, _ := http.NewRequest(http.MethodGet, testURL, nil) + req.Header.Add(testProxyAuthHeader, "Bearer gregg.EEewGREQ.GDSG") var bearerToken string @@ -124,22 +132,22 @@ func shouldHandleConnect(t *testing.T, hoverfly *Hoverfly, url string) { func Test_matchesFilter_ShouldMatchHostDestination(t *testing.T) { RegisterTestingT(t) - httpResult := matchesFilter("test.com")(&http.Request{ - Host: "test.com", + httpResult := matchesFilter(testHost)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "http", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpResult).To(BeTrue()) - httpsResult := matchesFilter("test.com")(&http.Request{ - Host: "test.com", + httpsResult := matchesFilter(testHost)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "https", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) @@ -147,22 +155,22 @@ func Test_matchesFilter_ShouldMatchHostDestination(t *testing.T) { func Test_matchesFilter_ShouldMatchPathDestination(t *testing.T) { RegisterTestingT(t) - httpResult := matchesFilter("/testing")(&http.Request{ - Host: "test.com", + httpResult := matchesFilter(testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "http", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpResult).To(BeTrue()) - httpsResult := matchesFilter("/testing")(&http.Request{ - Host: "test.com", + httpsResult := matchesFilter(testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "https", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) @@ -170,22 +178,22 @@ func Test_matchesFilter_ShouldMatchPathDestination(t *testing.T) { func Test_matchesFilter_ShouldMatchHostAndPathDestination(t *testing.T) { RegisterTestingT(t) - httpResult := matchesFilter("test.com/testing")(&http.Request{ - Host: "test.com", + httpResult := matchesFilter(testHost+testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "http", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpResult).To(BeTrue()) - httpsResult := matchesFilter("test.com/testing")(&http.Request{ - Host: "test.com", + httpsResult := matchesFilter(testHost+testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "https", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) @@ -194,21 +202,21 @@ func Test_matchesFilter_ShouldMatchHostAndPathDestination(t *testing.T) { func Test_matchesFilter_ShouldMatchSchemeDestination(t *testing.T) { RegisterTestingT(t) httpResult := matchesFilter("https")(&http.Request{ - Host: "test.com", + Host: testHost, URL: &url.URL{ Scheme: "http", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpResult).To(BeFalse()) - httpsResult := matchesFilter("https://")(&http.Request{ - Host: "test.com", + httpsResult := matchesFilter(testSchemeHTTPS)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "https", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) @@ -216,22 +224,22 @@ func Test_matchesFilter_ShouldMatchSchemeDestination(t *testing.T) { func Test_matchesFilter_ShouldMatchSchemeAndHostDestination(t *testing.T) { RegisterTestingT(t) - httpResult := matchesFilter("https://test.com")(&http.Request{ - Host: "test.com", + httpResult := matchesFilter(testSchemeHTTPS+testHost)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "http", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpResult).To(BeFalse()) - httpsResult := matchesFilter("https://test.com")(&http.Request{ - Host: "test.com", + httpsResult := matchesFilter(testSchemeHTTPS+testHost)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "https", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) @@ -239,22 +247,22 @@ func Test_matchesFilter_ShouldMatchSchemeAndHostDestination(t *testing.T) { func Test_matchesFilter_ShouldMatchSchemeAndHostAndPathDestination(t *testing.T) { RegisterTestingT(t) - httpResult := matchesFilter("https://test.com/testing")(&http.Request{ - Host: "test.com", + httpResult := matchesFilter(testSchemeHTTPS+testHost+testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "http", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpResult).To(BeFalse()) - httpsResult := matchesFilter("https://test.com/testing")(&http.Request{ - Host: "test.com", + httpsResult := matchesFilter(testSchemeHTTPS+testHost+testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "https", - Host: "test.com", - Path: "/testing", + Host: testHost, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) @@ -263,22 +271,22 @@ func Test_matchesFilter_ShouldMatchSchemeAndHostAndPathDestination(t *testing.T) func Test_matchesFilter_ShoulRemoveSslPortFromHostToMatch(t *testing.T) { RegisterTestingT(t) - httpsResult := matchesFilter("https://test.com/testing")(&http.Request{ - Host: "test.com:443", + httpsResult := matchesFilter(testSchemeHTTPS+testHost+testPath)(&http.Request{ + Host: testHostSSL, URL: &url.URL{ Scheme: "https", - Host: "test.com:443", - Path: "/testing", + Host: testHostSSL, + Path: testPath, }, }, nil) Expect(httpsResult).To(BeTrue()) - noSchemeResult := matchesFilter("https://test.com/testing")(&http.Request{ - Host: "test.com:443", + noSchemeResult := matchesFilter(testSchemeHTTPS+testHost+testPath)(&http.Request{ + Host: testHostSSL, URL: &url.URL{ Scheme: "", - Host: "test.com:443", - Path: "/testing", + Host: testHostSSL, + Path: testPath, }, }, nil) Expect(noSchemeResult).To(BeTrue()) @@ -287,26 +295,99 @@ func Test_matchesFilter_ShoulRemoveSslPortFromHostToMatch(t *testing.T) { func Test_matchesFilter_ShoulRemovePathFromFilterIfConnectRequest(t *testing.T) { RegisterTestingT(t) - removedPathResult := matchesFilter("https://test.com/testing")(&http.Request{ - Host: "test.com:443", + removedPathResult := matchesFilter(testSchemeHTTPS+testHost+testPath)(&http.Request{ + Host: testHostSSL, Method: http.MethodConnect, URL: &url.URL{ Scheme: "https", - Host: "test.com:443", + Host: testHostSSL, Path: "", }, }, nil) Expect(removedPathResult).To(BeTrue()) } +func Test_NewProxy_ShouldReturn400ForRelativeURLWithNoHostHeader(t *testing.T) { + RegisterTestingT(t) + testHoverfly := NewHoverfly() + proxy := NewProxy(testHoverfly) + proxyServer := httptest.NewServer(proxy) + defer proxyServer.Close() + + // Use raw TCP to send a relative URL with no Host header — http.DefaultClient + // always produces an absolute URL, so raw TCP is required to exercise NonproxyHandler. + conn, err := net.Dial("tcp", proxyServer.Listener.Addr().String()) + Expect(err).To(BeNil()) + defer conn.Close() + + fmt.Fprintf(conn, "GET /some-path HTTP/1.1\r\nConnection: close\r\n\r\n") + + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + Expect(err).To(BeNil()) + Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) +} + +func Test_NewProxy_ShouldHandleRelativeURLWithHostHeader(t *testing.T) { + RegisterTestingT(t) + + // Upstream target that the proxy will forward to + targetServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer targetServer.Close() + + testHoverfly := NewHoverfly() + proxy := NewProxy(testHoverfly) + proxyServer := httptest.NewServer(proxy) + defer proxyServer.Close() + + targetURL, _ := url.Parse(targetServer.URL) + + // Use raw TCP to send a relative URL with a Host header — this is the HTTP/1.1 + // pattern that was previously returning 500. + conn, err := net.Dial("tcp", proxyServer.Listener.Addr().String()) + Expect(err).To(BeNil()) + defer conn.Close() + + fmt.Fprintf(conn, "GET / HTTP/1.1\r\nHost: %s\r\nConnection: close\r\n\r\n", targetURL.Host) + + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + Expect(err).To(BeNil()) + // Must not return the "This is a proxy server" 500 — any non-500 response is acceptable + Expect(resp.StatusCode).ToNot(Equal(http.StatusInternalServerError)) +} + +func Test_NewProxy_ShouldReturn500WhenRelativeURLHostIsProxyItself(t *testing.T) { + RegisterTestingT(t) + testHoverfly := NewHoverfly() + proxy := NewProxy(testHoverfly) + proxyServer := httptest.NewServer(proxy) + defer proxyServer.Close() + + // Set ProxyPort to match the test server's port so the self-address guard fires. + _, port, _ := net.SplitHostPort(proxyServer.Listener.Addr().String()) + testHoverfly.Cfg.ProxyPort = port + + // Send a relative URL with Host == the proxy's own address (direct browser-to-proxy hit). + conn, err := net.Dial("tcp", proxyServer.Listener.Addr().String()) + Expect(err).To(BeNil()) + defer conn.Close() + + fmt.Fprintf(conn, "GET / HTTP/1.1\r\nHost: localhost:%s\r\nConnection: close\r\n\r\n", port) + + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + Expect(err).To(BeNil()) + Expect(resp.StatusCode).To(Equal(http.StatusInternalServerError)) +} + func Test_matchesFilter_ShouldGetHostNameFromRequest(t *testing.T) { RegisterTestingT(t) - httpResult := matchesFilter("/testing")(&http.Request{ - Host: "test.com", + httpResult := matchesFilter(testPath)(&http.Request{ + Host: testHost, URL: &url.URL{ Scheme: "http", - Path: "/testing", + Path: testPath, }, }, nil) Expect(httpResult).To(BeTrue())