From 2a44c0f65ae6b1fe564bbeef8f42eb70980cfcb9 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Tue, 8 Oct 2019 10:50:15 +0200 Subject: [PATCH] improving HTTP error code and fixing #67 --- internal/app/xhttp/handler.go | 26 ++++++++++++++++++++ internal/app/xhttp/handler_test.go | 38 ++++++++++++++++++++++++++++++ internal/app/xhttp/middleware.go | 25 ++++++++++++++++---- internal/app/xhttp/xhttp_test.go | 11 ++++++++- 4 files changed, 95 insertions(+), 5 deletions(-) diff --git a/internal/app/xhttp/handler.go b/internal/app/xhttp/handler.go index dda5d68e..7c819bd2 100644 --- a/internal/app/xhttp/handler.go +++ b/internal/app/xhttp/handler.go @@ -8,6 +8,7 @@ import ( "github.com/labstack/echo/v4" "github.com/thecodingmachine/gotenberg/internal/app/xhttp/pkg/context" "github.com/thecodingmachine/gotenberg/internal/app/xhttp/pkg/resource" + "github.com/thecodingmachine/gotenberg/internal/pkg/conf" "github.com/thecodingmachine/gotenberg/internal/pkg/printer" "github.com/thecodingmachine/gotenberg/internal/pkg/xerror" "github.com/thecodingmachine/gotenberg/internal/pkg/xrand" @@ -24,6 +25,31 @@ const ( officeEndpoint string = "/office" ) +func isMultipartFormDataEndpoint(config conf.Config, path string) bool { + var multipartFormDataEndpoints []string + multipartFormDataEndpoints = append(multipartFormDataEndpoints, mergeEndpoint) + if !config.DisableGoogleChrome() { + multipartFormDataEndpoints = append( + multipartFormDataEndpoints, + fmt.Sprintf("%s%s", convertGroupEndpoint, htmlEndpoint), + fmt.Sprintf("%s%s", convertGroupEndpoint, urlEndpoint), + fmt.Sprintf("%s%s", convertGroupEndpoint, markdownEndpoint), + ) + } + if !config.DisableUnoconv() { + multipartFormDataEndpoints = append( + multipartFormDataEndpoints, + fmt.Sprintf("%s%s", convertGroupEndpoint, officeEndpoint), + ) + } + for _, endpoint := range multipartFormDataEndpoints { + if endpoint == path { + return true + } + } + return false +} + // pingHandler is the handler for healthcheck. func pingHandler(c echo.Context) error { const op string = "xhttp.pingHandler" diff --git a/internal/app/xhttp/handler_test.go b/internal/app/xhttp/handler_test.go index df63d2a8..5602d90b 100644 --- a/internal/app/xhttp/handler_test.go +++ b/internal/app/xhttp/handler_test.go @@ -21,6 +21,9 @@ func TestPingHandler(t *testing.T) { srv := New(config) req := httptest.NewRequest(http.MethodGet, pingEndpoint, nil) test.AssertStatusCode(t, http.StatusOK, srv, req) + // should return 405 as Method is wrong. + req = httptest.NewRequest(http.MethodPost, pingEndpoint, nil) + test.AssertStatusCode(t, http.StatusMethodNotAllowed, srv, req) } func TestMergeHandler(t *testing.T) { @@ -31,6 +34,13 @@ func TestMergeHandler(t *testing.T) { req := httptest.NewRequest(http.MethodPost, mergeEndpoint, body) req.Header.Set(echo.HeaderContentType, contentType) test.AssertStatusCode(t, http.StatusOK, srv, req) + // should return 405 as Method is wrong. + req = httptest.NewRequest(http.MethodGet, mergeEndpoint, nil) + test.AssertStatusCode(t, http.StatusMethodNotAllowed, srv, req) + // should return 415 as Content-Type is wrong. + body, _ = test.MergeMultipartForm(t, nil) + req = httptest.NewRequest(http.MethodPost, mergeEndpoint, body) + test.AssertStatusCode(t, http.StatusUnsupportedMediaType, srv, req) // should return 400 as "waitTimeout" form field // value is < 0. body, contentType = test.MergeMultipartForm(t, map[string]string{string(resource.WaitTimeoutArgKey): "-1"}) @@ -65,6 +75,13 @@ func TestHTMLHandler(t *testing.T) { req := httptest.NewRequest(http.MethodPost, endpoint, body) req.Header.Set(echo.HeaderContentType, contentType) test.AssertStatusCode(t, http.StatusOK, srv, req) + // should return 405 as Method is wrong. + req = httptest.NewRequest(http.MethodGet, endpoint, nil) + test.AssertStatusCode(t, http.StatusMethodNotAllowed, srv, req) + // should return 415 as Content-Type is wrong. + body, _ = test.HTMLMultipartForm(t, nil) + req = httptest.NewRequest(http.MethodPost, endpoint, body) + test.AssertStatusCode(t, http.StatusUnsupportedMediaType, srv, req) // should return 400 as "waitTimeout" form field // value is < 0. body, contentType = test.HTMLMultipartForm(t, map[string]string{string(resource.WaitTimeoutArgKey): "-1"}) @@ -213,6 +230,13 @@ func TestURLHandler(t *testing.T) { req := httptest.NewRequest(http.MethodPost, endpoint, body) req.Header.Set(echo.HeaderContentType, contentType) test.AssertStatusCode(t, http.StatusOK, srv, req) + // should return 405 as Method is wrong. + req = httptest.NewRequest(http.MethodGet, endpoint, nil) + test.AssertStatusCode(t, http.StatusMethodNotAllowed, srv, req) + // should return 415 as Content-Type is wrong. + body, _ = test.URLMultipartForm(t, nil) + req = httptest.NewRequest(http.MethodPost, endpoint, body) + test.AssertStatusCode(t, http.StatusUnsupportedMediaType, srv, req) // should return 400 as "waitTimeout" form field // value is < 0. body, contentType = test.URLMultipartForm(t, map[string]string{string(resource.WaitTimeoutArgKey): "-1"}) @@ -361,6 +385,13 @@ func TestMarkdownHandler(t *testing.T) { req := httptest.NewRequest(http.MethodPost, endpoint, body) req.Header.Set(echo.HeaderContentType, contentType) test.AssertStatusCode(t, http.StatusOK, srv, req) + // should return 405 as Method is wrong. + req = httptest.NewRequest(http.MethodGet, endpoint, nil) + test.AssertStatusCode(t, http.StatusMethodNotAllowed, srv, req) + // should return 415 as Content-Type is wrong. + body, _ = test.MarkdownMultipartForm(t, nil) + req = httptest.NewRequest(http.MethodPost, endpoint, body) + test.AssertStatusCode(t, http.StatusUnsupportedMediaType, srv, req) // should return 400 as "waitTimeout" form field // value is < 0. body, contentType = test.MarkdownMultipartForm(t, map[string]string{string(resource.WaitTimeoutArgKey): "-1"}) @@ -509,6 +540,13 @@ func TestOfficeHandler(t *testing.T) { req := httptest.NewRequest(http.MethodPost, endpoint, body) req.Header.Set(echo.HeaderContentType, contentType) test.AssertStatusCode(t, http.StatusOK, srv, req) + // should return 405 as Method is wrong. + req = httptest.NewRequest(http.MethodGet, endpoint, nil) + test.AssertStatusCode(t, http.StatusMethodNotAllowed, srv, req) + // should return 415 as Content-Type is wrong. + body, _ = test.OfficeMultipartForm(t, nil) + req = httptest.NewRequest(http.MethodPost, endpoint, body) + test.AssertStatusCode(t, http.StatusUnsupportedMediaType, srv, req) // should return 400 as "waitTimeout" form field // value is < 0. body, contentType = test.OfficeMultipartForm(t, map[string]string{string(resource.WaitTimeoutArgKey): "-1"}) diff --git a/internal/app/xhttp/middleware.go b/internal/app/xhttp/middleware.go index 5e5ca0b3..ff722c6a 100644 --- a/internal/app/xhttp/middleware.go +++ b/internal/app/xhttp/middleware.go @@ -2,6 +2,7 @@ package xhttp import ( "net/http" + "strings" "github.com/labstack/echo/v4" "github.com/thecodingmachine/gotenberg/internal/app/xhttp/pkg/context" @@ -25,12 +26,28 @@ func contextMiddleware(config conf.Config) echo.MiddlewareFunc { // extend the current echo context with our custom // context. ctx := context.New(c, logger, config) - // if its an healthcheck request, there - // is no need to create a Resource. - if ctx.Path() == pingEndpoint { + // if it's not a multipart/form-data request, + // there is no need to create a Resource. + if !isMultipartFormDataEndpoint(config, ctx.Path()) { + // validate method for healthcheck endpoint. + if ctx.Path() == pingEndpoint && ctx.Request().Method != http.MethodGet { + err := doErr(ctx, echo.NewHTTPError(http.StatusMethodNotAllowed)) + return ctx.LogRequestResult(err, false) + } return next(ctx) } - // if the endpoint is not for healthcheck, create a + // validate method. + if ctx.Request().Method != http.MethodPost { + err := doErr(ctx, echo.NewHTTPError(http.StatusMethodNotAllowed)) + return ctx.LogRequestResult(err, false) + } + // validate Content-Type. + contentType := ctx.Request().Header.Get("Content-Type") + if !strings.Contains(contentType, "multipart/form-data") { + err := doErr(ctx, echo.NewHTTPError(http.StatusUnsupportedMediaType)) + return ctx.LogRequestResult(err, false) + } + // it's a multipart/form-data request, create a // Resource. if err := ctx.WithResource(trace); err != nil { err = doCleanup(ctx, err) diff --git a/internal/app/xhttp/xhttp_test.go b/internal/app/xhttp/xhttp_test.go index 685fb0a1..801407ec 100644 --- a/internal/app/xhttp/xhttp_test.go +++ b/internal/app/xhttp/xhttp_test.go @@ -13,6 +13,15 @@ import ( "github.com/thecodingmachine/gotenberg/test" ) +func TestNonExistingEndpoint(t *testing.T) { + config, err := conf.FromEnv() + assert.Nil(t, err) + srv := New(config) + // "/" endpoint should return 404. + req := httptest.NewRequest(http.MethodGet, "/", nil) + test.AssertStatusCode(t, http.StatusNotFound, srv, req) +} + func TestDisableChromeEndpoints(t *testing.T) { os.Setenv(conf.DisableGoogleChromeEnvVar, "1") config, err := conf.FromEnv() @@ -73,7 +82,7 @@ func TestDisableUnoconvEndpoints(t *testing.T) { req = httptest.NewRequest(http.MethodPost, fmt.Sprintf("%s%s", convertGroupEndpoint, urlEndpoint), body) req.Header.Set(echo.HeaderContentType, contentType) test.AssertStatusCode(t, http.StatusOK, srv, req) - // Markdown endpoint should return 404. + // Markdown endpoint should return 200. body, contentType = test.MarkdownMultipartForm(t, nil) req = httptest.NewRequest(http.MethodPost, fmt.Sprintf("%s%s", convertGroupEndpoint, markdownEndpoint), body) req.Header.Set(echo.HeaderContentType, contentType)