Skip to content

Commit

Permalink
Record client disconnected errors as HTTP 499
Browse files Browse the repository at this point in the history
  • Loading branch information
Marcus Cobden committed Oct 1, 2018
1 parent 8e898d0 commit 52de3bc
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
19 changes: 17 additions & 2 deletions authfe/proxy.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -31,11 +32,13 @@ func newProxy(cfg proxyConfig) (http.Handler, error) {
case "https":
fallthrough
case "http":

// Make all transformations outside of the director since
// they are also required when proxying websockets
return &httpProxy{cfg, httputil.ReverseProxy{
Director: func(*http.Request) {},
Transport: proxyTransport,
Director: func(*http.Request) {},
Transport: proxyTransport,
ErrorHandler: handleError,
}}, nil
case "mock":
return &mockProxy{cfg}, nil
Expand Down Expand Up @@ -163,6 +166,18 @@ func (p *httpProxy) proxyWS(w http.ResponseWriter, r *http.Request) {
logger.Debugf("proxy: websocket: connection closed")
}

func handleError(rw http.ResponseWriter, outReq *http.Request, err error) {
ctx := outReq.Context()
var header int
if ctx.Err() == context.Canceled {
header = 499 // Client Closed Request (nginx convention)
} else {
user.LogWith(ctx, logging.Global()).WithField("err", err).Errorln("http proxy error")
header = http.StatusBadGateway
}
rw.WriteHeader(header)
}

type closeWriter interface {
CloseWrite() error
}
Expand Down
46 changes: 46 additions & 0 deletions authfe/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,49 @@ func TestProxyGRPCTracing(t *testing.T) {
assert.Equal(t, 200, resp.StatusCode)
assert.Equal(t, "world", string(body[:c]))
}

func TestProxyClientClosed(t *testing.T) {
serverCh := make(chan interface{})

// Set up a slow server
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(418)
serverCh <- nil // Synchonise with the test
}))
defer server.Close()

// Set up a proxy handler pointing at the server
serverURL, err := url.Parse(server.URL)
assert.NoError(t, err, "Cannot parse URL")
proxyHandler, _ := newProxy(proxyConfig{hostAndPort: serverURL.Host, protocol: "http"})

// Intercept the proxy response to check the response code
codeCh := make(chan int)
interceptedProxyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
recorder := httptest.NewRecorder()
proxyHandler.ServeHTTP(recorder, r)
codeCh <- recorder.Code
w.WriteHeader(recorder.Code)
})

// Set up the proxy server
proxyServer := httptest.NewServer(interceptedProxyHandler)
defer proxyServer.Close()

// Make a request which times out faster than the slow server
req, err := http.NewRequest("GET", proxyServer.URL, nil)
require.NoError(t, err)
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
req = req.WithContext(ctx)

_, err = http.DefaultClient.Do(req)
// Check that the request timed out early
assert.Error(t, context.DeadlineExceeded, err)
// Wait for the slow server to receive the request, and allow it to continue
assert.Nil(t, <-serverCh)
// Wait for the proxyHandler to finish handling the request, and allow it to continue
responseCode := <-codeCh
// Check that the proxy server set the response code to 499
assert.Equal(t, 499, responseCode)
}

0 comments on commit 52de3bc

Please sign in to comment.