From cf5e530ed98684535a0f79de05aac9c6f320a81b Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Mon, 19 Aug 2019 16:29:58 +0200 Subject: [PATCH] special case handling: if one file has been sent and it is empty, return a correct invalid HTTP code AND remove the resource folder --- internal/app/xhttp/middleware.go | 101 ++++++++++++---------- internal/app/xhttp/pkg/context/context.go | 10 +++ 2 files changed, 65 insertions(+), 46 deletions(-) diff --git a/internal/app/xhttp/middleware.go b/internal/app/xhttp/middleware.go index 72552a88..456a7cd2 100644 --- a/internal/app/xhttp/middleware.go +++ b/internal/app/xhttp/middleware.go @@ -34,9 +34,10 @@ func contextMiddleware(config conf.Config, processes ...pm2.Process) echo.Middle // if the endpoint is not for healthcheck, create a // Resource. if err := ctx.WithResource(trace); err != nil { - // required to have a correct status code. - ctx.Error(err) + err = doCleanup(ctx, err) + err = doErr(ctx, err) return ctx.LogRequestResult(err, false) + } return next(ctx) } @@ -62,27 +63,9 @@ func loggerMiddleware() echo.MiddlewareFunc { func cleanupMiddleware() echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { - const op string = "xhttp.cleanupMiddleware" err := next(c) ctx := context.MustCastFromEchoContext(c) - if !ctx.HasResource() { - // nothing to remove. - return err - } - r := ctx.MustResource() - // if a webhook URL has been given, - // do not remove the resource.Resource here because - // we don't know if the result file has been - // generated or sent. - if r.HasArg(resource.WebhookURLArgKey) { - return err - } - // a resource.Resource is associated with our custom context. - if resourceErr := r.Close(); resourceErr != nil { - xerr := xerror.New(op, resourceErr) - ctx.XLogger().ErrorOp(xerror.Op(xerr), xerr) - } - return err + return doCleanup(ctx, err) } } } @@ -97,31 +80,57 @@ func errorMiddleware() echo.MiddlewareFunc { // so far so good! return nil } - // if it's an error from echo - // like 404 not found and so on. - if echoHTTPErr, ok := err.(*echo.HTTPError); ok { - return echoHTTPErr - } - // we log the initial error before returning - // the HTTP error. - errOp := xerror.Op(err) - logger := ctx.XLogger() - logger.ErrorOp(errOp, err) - // handle our custom HTTP error. - var httpErr error - errCode := xerror.Code(err) - errMessage := xerror.Message(err) - switch errCode { - case xerror.InvalidCode: - httpErr = echo.NewHTTPError(http.StatusBadRequest, errMessage) - case xerror.TimeoutCode: - httpErr = echo.NewHTTPError(http.StatusGatewayTimeout, errMessage) - default: - httpErr = echo.NewHTTPError(http.StatusInternalServerError, errMessage) - } - // required to have a correct status code. - ctx.Error(httpErr) - return httpErr + return doErr(ctx, err) } } } + +func doCleanup(ctx context.Context, err error) error { + const op string = "xhttp.cleanup" + if !ctx.HasResource() { + // nothing to remove. + return err + } + r := ctx.MustResource() + // if a webhook URL has been given, + // do not remove the resource.Resource here because + // we don't know if the result file has been + // generated or sent. + if r.HasArg(resource.WebhookURLArgKey) { + return err + } + // a resource.Resource is associated with our custom context. + if resourceErr := r.Close(); resourceErr != nil { + xerr := xerror.New(op, resourceErr) + ctx.XLogger().ErrorOp(xerror.Op(xerr), xerr) + } + return err +} + +func doErr(ctx context.Context, err error) error { + // if it's an error from echo + // like 404 not found and so on. + if echoHTTPErr, ok := err.(*echo.HTTPError); ok { + return echoHTTPErr + } + // we log the initial error before returning + // the HTTP error. + errOp := xerror.Op(err) + logger := ctx.XLogger() + logger.ErrorOp(errOp, err) + // handle our custom HTTP error. + var httpErr error + errCode := xerror.Code(err) + errMessage := xerror.Message(err) + switch errCode { + case xerror.InvalidCode: + httpErr = echo.NewHTTPError(http.StatusBadRequest, errMessage) + case xerror.TimeoutCode: + httpErr = echo.NewHTTPError(http.StatusGatewayTimeout, errMessage) + default: + httpErr = echo.NewHTTPError(http.StatusInternalServerError, errMessage) + } + // required to have a correct status code. + ctx.Error(httpErr) + return httpErr +} diff --git a/internal/app/xhttp/pkg/context/context.go b/internal/app/xhttp/pkg/context/context.go index d29b9321..d702e9d8 100644 --- a/internal/app/xhttp/pkg/context/context.go +++ b/internal/app/xhttp/pkg/context/context.go @@ -2,9 +2,11 @@ package context import ( "fmt" + "io" "net/http" "reflect" "strconv" + "strings" "time" "github.com/labstack/echo/v4" @@ -102,6 +104,14 @@ func (ctx *Context) WithResource(directoryName string) error { // write form files from request. form, err := ctx.MultipartForm() if err != nil { + /* + (very) special case: one and + only one file has been sent + and it is empty. + */ + if strings.Contains(err.Error(), io.EOF.Error()) { + return r, xerror.Invalid(op, "one file has been sent but it is empty: does it exist?", err) + } return r, err } for _, files := range form.File {