diff --git a/DESIGN.md b/DESIGN.md index 4f4e692..90452fc 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -17,6 +17,4 @@ A post request to an endpoint with the matching jason notification type should s ## Dependency injection -The web package has some weird dependency injection. - -For now, take a look at it until you understand it. It looks like it does for a good reason. \ No newline at end of file +The web router uses net/http.ServeMux. Each request receives a mail service from the supplied factory, authenticates before routing, and destroys the service when the request finishes, including when sending panics. The request context carries this service to the mail handler. diff --git a/README.md b/README.md index 663b5a7..9943e09 100644 --- a/README.md +++ b/README.md @@ -142,3 +142,9 @@ services: ``` Other services would then be able to reach this service on `http://gotify:8080/...` with `123abc` as the preshared key + +### HTTP routing + +The router uses Go's standard `net/http` package. `POST /mail` and `POST /mail/` send mail; authenticated `OPTIONS` requests to these paths advertise `POST`. Unsupported methods and unknown paths return 404 after authentication. Each request releases its mail service, including after a panic. + +Requests are logged with method, path, status, and duration. Panics return a generic 500 response; debug mode includes a plain-text stack trace instead of the former framework's HTML error page. Standard ServeMux path normalization applies. diff --git a/go.mod b/go.mod index cfbdb59..c6148e7 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/cthit/gotify go 1.27.1 require ( - github.com/gocraft/web v0.0.0-20190207150652-9707327fb69b github.com/spf13/viper v1.20.0 github.com/stretchr/testify v1.10.0 golang.org/x/oauth2 v0.28.0 diff --git a/go.sum b/go.sum index 0038de1..d65564d 100644 --- a/go.sum +++ b/go.sum @@ -20,8 +20,6 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-viper/mapstructure/v2 v2.2.1 h1:ZAaOCxANMuZx5RCeg0mBdEZk7DZasvvZIxtHqx8aGss= github.com/go-viper/mapstructure/v2 v2.2.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= -github.com/gocraft/web v0.0.0-20190207150652-9707327fb69b h1:g2Qcs0B+vOQE1L3a7WQ/JUUSzJnHbTz14qkJSqEWcF4= -github.com/gocraft/web v0.0.0-20190207150652-9707327fb69b/go.mod h1:Ag7UMbZNGrnHwaXPJOUKJIVgx4QOWMOWZngrvsN6qak= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= diff --git a/web/auth.go b/web/auth.go index f6af08a..8f14abd 100644 --- a/web/auth.go +++ b/web/auth.go @@ -1,15 +1,13 @@ package web -import ( - "fmt" - "github.com/gocraft/web" - "net/http" -) +import "net/http" -func (c *Context) Auth(rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) { - if req.Header.Get("Authorization") == fmt.Sprintf("pre-shared: %s", c.AuthKey) { - next(rw, req) - } else { - rw.WriteHeader(http.StatusUnauthorized) - } +func (c *Context) Auth(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + if req.Header.Get("Authorization") != "pre-shared: "+c.AuthKey { + rw.WriteHeader(http.StatusUnauthorized) + return + } + next.ServeHTTP(rw, req) + }) } diff --git a/web/mail.go b/web/mail.go index 1a61fc2..a34efb9 100644 --- a/web/mail.go +++ b/web/mail.go @@ -6,11 +6,10 @@ import ( "net/http" "github.com/cthit/gotify" - "github.com/gocraft/web" "github.com/spf13/viper" ) -func (c *Context) SendMail(rw web.ResponseWriter, req *web.Request) { +func (c *Context) SendMail(rw http.ResponseWriter, req *http.Request) { var mail gotify.Mail // Ensure that the request is not too large diff --git a/web/router.go b/web/router.go index 3282972..59ede93 100644 --- a/web/router.go +++ b/web/router.go @@ -1,9 +1,14 @@ package web import ( - "github.com/cthit/gotify" - "github.com/gocraft/web" + "context" + "fmt" + "log" "net/http" + "runtime/debug" + "time" + + "github.com/cthit/gotify" ) type Context struct { @@ -12,46 +17,78 @@ type Context struct { Debug bool } -func Router(authKey string, mailServiceCreator func() gotify.MailService, debug bool) http.Handler { +type requestContextKey struct{} - router := web.NewWithPrefix( - Context{}, - "") - - router.Middleware(web.LoggerMiddleware) - if debug { - router.Middleware(web.ShowErrorsMiddleware) +func Router(authKey string, mailServiceCreator func() gotify.MailService, debugMode bool) http.Handler { + mux := http.NewServeMux() + send := func(rw http.ResponseWriter, req *http.Request) { + c := req.Context().Value(requestContextKey{}).(*Context) + c.SendMail(rw, req) } + options := func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Access-Control-Allow-Methods", "POST") + rw.WriteHeader(http.StatusOK) + } + mux.HandleFunc("POST /mail", send) + mux.HandleFunc("POST /mail/{$}", send) + mux.HandleFunc("OPTIONS /mail", options) + mux.HandleFunc("OPTIONS /mail/{$}", options) + // Preserve the existing 404 response for unsupported methods and paths. + mux.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusNotFound) + if _, err := fmt.Fprint(rw, "Not Found"); err != nil { + log.Print(err) + } + }) - router.Middleware(setDebugMode(debug)) - router.Middleware(setMailServiceProvider(mailServiceCreator)) - router.Middleware(setAuthKey(authKey)) - router.Middleware((*Context).Auth) - - router.Post("/mail", (*Context).SendMail) - return router + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + started := time.Now() + response := &statusWriter{ResponseWriter: rw} + defer func() { + if recovered := recover(); recovered != nil { + log.Printf("request panic: %v\n%s", recovered, debug.Stack()) + if response.status == 0 { + message := "Application Error" + if debugMode { + message = fmt.Sprintf("%v\n%s", recovered, debug.Stack()) + } + http.Error(response, message, http.StatusInternalServerError) + } + } + status := response.status + if status == 0 { + status = http.StatusOK + } + log.Printf("%s %s %d %s", req.Method, req.URL.Path, status, time.Since(started)) + }() + c := &Context{MailService: mailServiceCreator(), AuthKey: authKey, Debug: debugMode} + defer func() { + if err := c.MailService.Destroy(); err != nil { + c.printError(err) + } + }() + req = req.WithContext(context.WithValue(req.Context(), requestContextKey{}, c)) + c.Auth(mux).ServeHTTP(response, req) + }) } -func setMailServiceProvider(mailServiceProvider func() gotify.MailService) func(*Context, web.ResponseWriter, *web.Request, web.NextMiddlewareFunc) { - return func(c *Context, rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) { - c.MailService = mailServiceProvider() - next(rw, req) - if err := c.MailService.Destroy(); err != nil { - c.printError(err) - } - } +type statusWriter struct { + http.ResponseWriter + status int } -func setAuthKey(authKey string) func(*Context, web.ResponseWriter, *web.Request, web.NextMiddlewareFunc) { - return func(c *Context, rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) { - c.AuthKey = authKey - next(rw, req) +func (w *statusWriter) WriteHeader(status int) { + if w.status == 0 { + w.status = status + w.ResponseWriter.WriteHeader(status) } } -func setDebugMode(debug bool) func(*Context, web.ResponseWriter, *web.Request, web.NextMiddlewareFunc) { - return func(c *Context, rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) { - c.Debug = debug - next(rw, req) +func (w *statusWriter) Write(body []byte) (int, error) { + if w.status == 0 { + w.WriteHeader(http.StatusOK) } + return w.ResponseWriter.Write(body) } + +func (w *statusWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter } diff --git a/web/router_test.go b/web/router_test.go index c723465..8beb627 100644 --- a/web/router_test.go +++ b/web/router_test.go @@ -1,8 +1,10 @@ package web import ( + "github.com/spf13/viper" "net/http" "net/http/httptest" + "strings" "testing" "github.com/cthit/gotify" @@ -19,6 +21,8 @@ func TestRoutingContract(t *testing.T) { {http.MethodPut, "/mail", 404}, {http.MethodPost, "/missing", 404}, {http.MethodPost, "/mail/", 400}, + {http.MethodOptions, "/mail", 200}, + {http.MethodOptions, "/mail/", 200}, } { t.Run(tc.method+" "+tc.path, func(t *testing.T) { server := httptest.NewServer(Router("secret", func() gotify.MailService { return &fakeMailService{} }, false)) @@ -30,6 +34,51 @@ func TestRoutingContract(t *testing.T) { require.NoError(t, err) defer func() { assert.NoError(t, response.Body.Close()) }() assert.Equal(t, tc.status, response.StatusCode) + if tc.method == http.MethodOptions { + assert.Equal(t, "POST", response.Header.Get("Access-Control-Allow-Methods")) + } }) } } + +func TestAuthenticationPrecedesRouting(t *testing.T) { + for _, path := range []string{"/mail", "/mail/", "/missing"} { + t.Run(path, func(t *testing.T) { + service := &fakeMailService{} + handler := Router("secret", func() gotify.MailService { return service }, false) + response := httptest.NewRecorder() + handler.ServeHTTP(response, httptest.NewRequest(http.MethodGet, path, nil)) + assert.Equal(t, http.StatusUnauthorized, response.Code) + assert.Empty(t, response.Body.String()) + assert.Empty(t, service.sent) + assert.Equal(t, 1, service.destroyed) + }) + } +} + +type panickingMailService struct{ destroyed bool } + +func (s *panickingMailService) SendMail(mail gotify.Mail) (gotify.Mail, error) { + panic("test panic detail") +} +func (s *panickingMailService) Destroy() error { s.destroyed = true; return nil } + +func TestPanicRecoveryReleasesService(t *testing.T) { + viper.Set("max-mail-size", 256) + t.Cleanup(viper.Reset) + for _, debugMode := range []bool{false, true} { + service := &panickingMailService{} + handler := Router("secret", func() gotify.MailService { return service }, debugMode) + req := httptest.NewRequest(http.MethodPost, "/mail", strings.NewReader(`{}`)) + req.Header.Set("Authorization", "pre-shared: secret") + response := httptest.NewRecorder() + handler.ServeHTTP(response, req) + assert.Equal(t, http.StatusInternalServerError, response.Code) + assert.True(t, service.destroyed) + if debugMode { + assert.Contains(t, response.Body.String(), "test panic detail") + } else { + assert.Equal(t, "Application Error\n", response.Body.String()) + } + } +}