diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index 984babc1..9883a4e5 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -468,6 +468,8 @@ func listenForEventLoadingFailed(ctx context.Context, logger *slog.Logger, optio "net::ERR_CONNECTION_REFUSED", "net::ERR_CONNECTION_ABORTED", "net::ERR_CONNECTION_FAILED", + // What a pinning proxy CONNECT that cannot reach its target reports. + "net::ERR_TUNNEL_CONNECTION_FAILED", "net::ERR_NAME_NOT_RESOLVED", "net::ERR_INTERNET_DISCONNECTED", "net::ERR_ADDRESS_UNREACHABLE", diff --git a/pkg/modules/chromium/pinning_proxy.go b/pkg/modules/chromium/pinning_proxy.go index 4fcf2798..7e23ecf9 100644 --- a/pkg/modules/chromium/pinning_proxy.go +++ b/pkg/modules/chromium/pinning_proxy.go @@ -522,7 +522,7 @@ func (p *pinningProxy) handleForward(w http.ResponseWriter, req *http.Request) { } else { p.logger.WarnContext(req.Context(), fmt.Sprintf("forward RoundTrip failed for '%s': %s", req.URL, err)) } - http.Error(w, "upstream error", http.StatusBadGateway) + resetClient(w) return } defer resp.Body.Close() @@ -585,3 +585,23 @@ func isClientCancellation(ctx context.Context, err error) bool { func (p *pinningProxy) dialThroughUpstreamProxy(ctx context.Context, proxyURL *url.URL, target string) (net.Conn, error) { return gotenberg.DialThroughProxy(ctx, proxyURL, target, p.dialBypass) } + +// resetClient aborts the client connection with a TCP reset instead of +// answering. Chromium treats a proxy response as the origin's own, so a 502 +// would reach the page as an ordinary HTTP error; a reset surfaces as a +// network failure, as it would without the proxy. +func resetClient(w http.ResponseWriter) { + hj, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "upstream error", http.StatusBadGateway) + return + } + conn, _, err := hj.Hijack() + if err != nil { + return + } + if tcpConn, ok := conn.(*net.TCPConn); ok { + _ = tcpConn.SetLinger(0) + } + _ = conn.Close() +} diff --git a/pkg/modules/chromium/pinning_proxy_test.go b/pkg/modules/chromium/pinning_proxy_test.go index 089e6232..eeff7e89 100644 --- a/pkg/modules/chromium/pinning_proxy_test.go +++ b/pkg/modules/chromium/pinning_proxy_test.go @@ -15,6 +15,7 @@ import ( "strings" "sync" "sync/atomic" + "syscall" "testing" "time" @@ -764,6 +765,47 @@ func TestPinningProxy_CONNECT_DialFailure_LoggedAtWarn(t *testing.T) { } } +// A forward request whose target cannot be reached resets the client +// connection rather than answering 502. Chromium treats a proxy response as +// the origin's own, so a 502 would hide the failure from +// failOnResourceLoadingFailed. +func TestPinningProxy_Forward_DialFailure_ResetsClient(t *testing.T) { + p := newPinningProxy(nil, nil, false, false, false) + p.decide = func(_ context.Context, _ string, _, _ []*regexp2.Regexp, _ time.Time) (gotenberg.OutboundDecision, error) { + return gotenberg.OutboundDecision{Pinned: []netip.Addr{netip.MustParseAddr("127.0.0.1")}}, nil + } + p.dialPinned = func(_ context.Context, _ string, _ []netip.Addr, _ string) (net.Conn, error) { + return nil, errors.New("connection refused") + } + + logger := slog.New(slog.DiscardHandler) + err := p.Start(logger) + if err != nil { + t.Fatalf("start pinning proxy: %v", err) + } + t.Cleanup(func() { _ = p.Stop(logger) }) + + conn, err := net.Dial("tcp", strings.TrimPrefix(p.URL(), "http://")) + if err != nil { + t.Fatalf("dial proxy: %v", err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(5 * time.Second)) + + _, err = fmt.Fprintf(conn, "GET http://real.example/ HTTP/1.1\r\nHost: real.example\r\n\r\n") + if err != nil { + t.Fatalf("write request: %v", err) + } + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + if err == nil { + _ = resp.Body.Close() + t.Fatalf("got status %d, want the connection reset", resp.StatusCode) + } + if !errors.Is(err, syscall.ECONNRESET) && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + t.Fatalf("read response: %v, want a connection reset", err) + } +} + // TestPinningProxy_Forward_RoundTripCancellation_LoggedAtDebug verifies // the handleForward dial-cancellation path: when the inner Transport's // dial returns a canceled error (client hung up mid-dial), the proxy @@ -790,12 +832,9 @@ func TestPinningProxy_Forward_RoundTripCancellation_LoggedAtDebug(t *testing.T) Timeout: 5 * time.Second, } resp, err := client.Get("http://www.google.com/") - if err != nil { - t.Fatalf("GET via proxy: %v", err) - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusBadGateway { - t.Fatalf("status = %d, want 502", resp.StatusCode) + if err == nil { + _ = resp.Body.Close() + t.Fatalf("GET via proxy: got status %d, want the connection reset", resp.StatusCode) } records := rec.snapshot() diff --git a/test/integration/features/chromium_convert_html.feature b/test/integration/features/chromium_convert_html.feature index 5405b61a..a98a9d24 100644 --- a/test/integration/features/chromium_convert_html.feature +++ b/test/integration/features/chromium_convert_html.feature @@ -644,9 +644,23 @@ Feature: /forms/chromium/convert/html Then the response header "Content-Type" should be "application/pdf" Then there should be 1 PDF(s) in the response + # Through the pinning proxy, an unreachable resource must still fail the + # conversion. The proxy resets the connection instead of answering 502, which + # Chromium would treat as an ordinary HTTP error. + Scenario: POST /forms/chromium/convert/html (Fail On Resource Loading Failed through the pinning proxy) + Given I have a default Gotenberg container + When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): + | files | testdata/feature-rich-html/index.html | file | + | failOnResourceLoadingFailed | true | field | + Then the response status code should be 409 + Then the response body should contain string: + """ + resource Stylesheet: net::ERR_CONNECTION_RESET + """ + Scenario: POST /forms/chromium/convert/html (Fail On Resource Loading Failed) # A configured allow-list routes loopback through the pinning proxy, which - # reports a refused connection as a 502 response, not a loading failure. + # reports a refused connection as a reset. See the scenario below. Given I have a Gotenberg container with the following environment variable(s): | CHROMIUM_ALLOW_LIST | | When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): diff --git a/test/integration/features/chromium_convert_markdown.feature b/test/integration/features/chromium_convert_markdown.feature index 0f4b8e8a..8f83f328 100644 --- a/test/integration/features/chromium_convert_markdown.feature +++ b/test/integration/features/chromium_convert_markdown.feature @@ -411,7 +411,8 @@ Feature: /forms/chromium/convert/markdown Scenario: POST /forms/chromium/convert/markdown (Fail On Resource Loading Failed) # A configured allow-list routes loopback through the pinning proxy, which - # reports a refused connection as a 502 response, not a loading failure. + # reports a refused connection as a reset. See the convert/html + # scenario. Given I have a Gotenberg container with the following environment variable(s): | CHROMIUM_ALLOW_LIST | | When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/markdown" endpoint with the following form data and header(s): diff --git a/test/integration/features/chromium_convert_url.feature b/test/integration/features/chromium_convert_url.feature index b2d801e9..db278cce 100644 --- a/test/integration/features/chromium_convert_url.feature +++ b/test/integration/features/chromium_convert_url.feature @@ -712,7 +712,8 @@ Feature: /forms/chromium/convert/url Scenario: POST /forms/chromium/convert/url (Fail On Resource Loading Failed) # A configured allow-list routes loopback through the pinning proxy, which - # reports a refused connection as a 502 response, not a loading failure. + # reports a refused connection as a reset. See the convert/html + # scenario. Given I have a Gotenberg container with the following environment variable(s): | CHROMIUM_ALLOW_LIST | | Given I have a static server