Merge pull request #124 from thecodingmachine/wrong_http_error_codes

improving HTTP error code and fixing #67

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"

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"})

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)

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)