From 997514292a8492d2291643e91081f3e790eefbaf Mon Sep 17 00:00:00 2001 From: tjpcc Date: Tue, 24 Jan 2023 19:59:47 -0700 Subject: [PATCH] testing and linting and linter fixes --- .drone.yml | 4 ++- contrib/cgi/cgi.go | 4 +-- contrib/tlsauth/auth_test.go | 12 ++++++--- contrib/tlsauth/gemini_test.go | 8 ++++-- gemini/roundtrip_test.go | 8 ++++-- gemini/serve.go | 21 +++++++++++---- logging/middleware.go | 47 +++++++++++++++------------------- 7 files changed, 61 insertions(+), 43 deletions(-) diff --git a/.drone.yml b/.drone.yml index f2790c6..b5c0f6c 100644 --- a/.drone.yml +++ b/.drone.yml @@ -6,4 +6,6 @@ steps: - name: test image: golang commands: - - go test -v ./... + - go test -race -v ./... + - name: lint + image: rancher/drone-golangci-lint:latest diff --git a/contrib/cgi/cgi.go b/contrib/cgi/cgi.go index ab10622..71743a0 100644 --- a/contrib/cgi/cgi.go +++ b/contrib/cgi/cgi.go @@ -116,9 +116,7 @@ func RunCGI( } scriptName := req.Path[:len(req.Path)-infoLen] - if strings.HasSuffix(scriptName, "/") { - scriptName = scriptName[:len(scriptName)-1] - } + scriptName = strings.TrimSuffix(scriptName, "/") cmd := exec.CommandContext(ctx, "./"+basename) cmd.Env = prepareCGIEnv(ctx, req, scriptName, pathInfo) diff --git a/contrib/tlsauth/auth_test.go b/contrib/tlsauth/auth_test.go index 41292b4..fc39359 100644 --- a/contrib/tlsauth/auth_test.go +++ b/contrib/tlsauth/auth_test.go @@ -38,7 +38,9 @@ func TestIdentify(t *testing.T) { leafCert, err := x509.ParseCertificate(clientCert.Certificate[0]) require.Nil(t, err) - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() requestPath(t, client, server, "/") @@ -75,7 +77,9 @@ func TestRequiredAuth(t *testing.T) { gus.FallthroughHandler(handler1, handler2), ) - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() requestPath(t, client, server, "/one") @@ -113,7 +117,9 @@ func TestOptionalAuth(t *testing.T) { handler, ) - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() requestPath(t, client, server, "/one") diff --git a/contrib/tlsauth/gemini_test.go b/contrib/tlsauth/gemini_test.go index bc87958..8f1efda 100644 --- a/contrib/tlsauth/gemini_test.go +++ b/contrib/tlsauth/gemini_test.go @@ -54,7 +54,9 @@ func TestGeminiAuth(t *testing.T) { authlessClient, _ := clientFor(t, server, "", "") - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() resp := requestPath(t, authClient, server, "/one") @@ -94,7 +96,9 @@ func TestGeminiOptionalAuth(t *testing.T) { ) authlessClient, _ := clientFor(t, server, "", "") - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() resp := requestPath(t, authClient, server, "/one") diff --git a/gemini/roundtrip_test.go b/gemini/roundtrip_test.go index 4f48e47..ab7baa4 100644 --- a/gemini/roundtrip_test.go +++ b/gemini/roundtrip_test.go @@ -27,7 +27,9 @@ func TestRoundTrip(t *testing.T) { server, err := gemini.NewServer(context.Background(), nil, tlsConf, "tcp", "127.0.0.1:0", handler) require.Nil(t, err) - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() u, err := url.Parse(fmt.Sprintf("gemini://%s/test", server.Address())) @@ -70,7 +72,9 @@ func TestTitanRequest(t *testing.T) { server, err := gemini.NewServer(context.Background(), nil, tlsConf, "tcp", "127.0.0.1:0", handler) require.Nil(t, err) - go server.Serve() + go func() { + _ = server.Serve() + }() defer server.Close() conn, err := tls.Dial(server.Network(), server.Address(), testClientTLS()) diff --git a/gemini/serve.go b/gemini/serve.go index dd7ad52..abed257 100644 --- a/gemini/serve.go +++ b/gemini/serve.go @@ -5,6 +5,7 @@ import ( "context" "crypto/tls" "errors" + "fmt" "io" "net" "strconv" @@ -15,11 +16,13 @@ import ( "tildegit.org/tjp/gus/logging" ) +type titanRequestBodyKey struct{} + // TitanRequestBody is the key set in a handler's context for titan requests. // // When this key is present in the context (request.URL.Scheme will be "titan"), the // corresponding value is a *bufio.Reader from which the request body can be read. -const TitanRequestBody = "titan_request_body" +var TitanRequestBody = titanRequestBodyKey{} type server struct { ctx context.Context @@ -70,7 +73,7 @@ func NewServer( // but be aware that Close() must still be called in that case to avoid // dangling goroutines. // -// On titan protocol requests, it sets a key/value pair in the context. The +// On titan protocol requests it sets a key/value pair in the context. The // key is TitanRequestBody, and the value is a *bufio.Reader from which the // request body can be read. func (s *server) Serve() error { @@ -88,7 +91,7 @@ func (s *server) Serve() error { if s.Closed() { err = nil } else { - s.errorLog.Log("msg", "accept_error", "error", err) + _ = s.errorLog.Log("msg", "accept error", "error", err) } return err @@ -135,7 +138,7 @@ func (s *server) handleConn(conn net.Conn) { } else { req.Server = s req.RemoteAddr = conn.RemoteAddr() - if tlsconn, ok := conn.(*tls.Conn); req != nil && ok { + if tlsconn, ok := conn.(*tls.Conn); ok { state := tlsconn.ConnectionState() req.TLSState = &state } @@ -146,12 +149,20 @@ func (s *server) handleConn(conn net.Conn) { if err == nil { ctx = context.WithValue( ctx, - "titan_request_body", + TitanRequestBody, io.LimitReader(buf, int64(len)), ) } } + defer func() { + if r := recover(); r != nil { + err := fmt.Errorf("%s", r) + _ = s.errorLog.Log("msg", "panic in handler", "err", err) + _, _ = io.Copy(conn, NewResponseReader(Failure(err))) + } + }() + response = s.handler(ctx, req) if response == nil { response = NotFound("Resource does not exist.") diff --git a/logging/middleware.go b/logging/middleware.go index cbb345a..01278e1 100644 --- a/logging/middleware.go +++ b/logging/middleware.go @@ -3,6 +3,7 @@ package logging import ( "context" "errors" + "fmt" "io" "time" @@ -12,18 +13,9 @@ import ( func LogRequests(logger Logger) gus.Middleware { return func(inner gus.Handler) gus.Handler { return func(ctx context.Context, request *gus.Request) *gus.Response { - start := time.Now() - response := inner(ctx, request) if response != nil { - body := loggedResponseBody{ - request: request, - response: response, - body: response.Body, - start: start, - logger: logger, - } - response.Body = &body + response.Body = loggingBody(logger, request, response) } return response @@ -42,23 +34,6 @@ type loggedResponseBody struct { logger Logger } -func loggingBody(logger Logger, request *gus.Request, response *gus.Response) io.Reader { - body := &loggedResponseBody{ - request: request, - response: response, - body: response.Body, - start: time.Now(), - written: 0, - logger: logger, - } - - if _, ok := response.Body.(io.WriterTo); ok { - return loggedWriteToResponseBody{body} - } - - return body -} - func (lr *loggedResponseBody) log() { end := time.Now() _ = lr.logger.Log( @@ -98,6 +73,7 @@ type loggedWriteToResponseBody struct { } func (lwtr loggedWriteToResponseBody) WriteTo(dst io.Writer) (int64, error) { + fmt.Println("lwtrb.WriteTo()") n, err := lwtr.body.(io.WriterTo).WriteTo(dst) if err == nil { lwtr.written += int(n) @@ -105,3 +81,20 @@ func (lwtr loggedWriteToResponseBody) WriteTo(dst io.Writer) (int64, error) { } return n, err } + +func loggingBody(logger Logger, request *gus.Request, response *gus.Response) io.Reader { + body := &loggedResponseBody{ + request: request, + response: response, + body: response.Body, + start: time.Now(), + written: 0, + logger: logger, + } + + if _, ok := response.Body.(io.WriterTo); ok { + return loggedWriteToResponseBody{body} + } + + return body +}