testing and linting and linter fixes

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
+}