Skip to content

Conversation

@t-ibayashi-safie
Copy link
Contributor

@t-ibayashi-safie t-ibayashi-safie commented Apr 2, 2025

This PR fixes the issue below.
#2200

Detail

  • middleware/proxy.go
    • I change proxyRaw method to be able to proxy tls connection
  • middleware/proxy_test.go

About websocket package in testing

I used an external package to easily implement web socket testing.

We considered the following packages, but we thought it would be better not to add too many third-party package dependencies for echo, so we adopted the official golang.org/x/net/websocket.

https://pkg.go.dev/github.com/gorilla/websocket
https://pkg.go.dev/github.com/coder/websocket

Copy link
Contributor

@aldas aldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this would be better approach

func proxyRaw(t *ProxyTarget, c echo.Context, config ProxyConfig) http.Handler { var dialFunc func(ctx context.Context, network, addr string) (net.Conn, error) if transport, ok := config.Transport.(*http.Transport); ok { if transport.TLSClientConfig != nil { d := tls.Dialer{ Config: transport.TLSClientConfig,	} dialFunc = d.DialContext	}	} if dialFunc == nil { var d net.Dialer dialFunc = d.DialContext	} return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { in, _, err := c.Response().Hijack() if err != nil { c.Set("_error", fmt.Errorf("proxy raw, hijack error=%w, url=%s", err, t.URL)) return	} defer in.Close() out, err := dialFunc(c.Request().Context(), "tcp", t.URL.Host) if err != nil { c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("proxy raw, dial error=%v, url=%s", err, t.URL))) return	}
@t-ibayashi-safie t-ibayashi-safie requested a review from aldas April 4, 2025 02:55
Copy link
Contributor

@aldas aldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

p.s. huge thanks for the raw proxy tests! I have never found motivation to add these myself and always felt bad when reviewing proxy related PRs for that.

@aldas aldas merged commit de44c53 into labstack:master Apr 4, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants