diff --git a/middleware/context_timeout_test.go b/middleware/context_timeout_test.go index 605ca8e65..24c6203e7 100644 --- a/middleware/context_timeout_test.go +++ b/middleware/context_timeout_test.go @@ -148,7 +148,7 @@ func TestContextTimeoutWithDefaultErrorMessage(t *testing.T) { c := e.NewContext(req, rec) err := m(func(c echo.Context) error { - if err := sleepWithContext(c.Request().Context(), time.Duration(20*time.Millisecond)); err != nil { + if err := sleepWithContext(c.Request().Context(), time.Duration(80*time.Millisecond)); err != nil { return err } return c.String(http.StatusOK, "Hello, World!") @@ -176,7 +176,7 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { return nil } - timeout := 10 * time.Millisecond + timeout := 50 * time.Millisecond m := ContextTimeoutWithConfig(ContextTimeoutConfig{ Timeout: timeout, ErrorHandler: timeoutErrorHandler, @@ -189,11 +189,10 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { c := e.NewContext(req, rec) err := m(func(c echo.Context) error { - // NOTE: when difference between timeout duration and handler execution time is almost the same (in range of 100microseconds) - // the result of timeout does not seem to be reliable - could respond timeout, could respond handler output - // difference over 500microseconds (0.5millisecond) response seems to be reliable + // NOTE: Very short periods are not reliable for tests due to Go routine scheduling and the unpredictable order + // for 1) request and 2) time goroutine. For most OS this works as expected, but MacOS seems most flaky. - if err := sleepWithContext(c.Request().Context(), time.Duration(20*time.Millisecond)); err != nil { + if err := sleepWithContext(c.Request().Context(), 100*time.Millisecond); err != nil { return err } diff --git a/middleware/static.go b/middleware/static.go index 27ccf4117..24a5f59b9 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -8,7 +8,6 @@ import ( "net/url" "os" "path" - "path/filepath" "strings" "github.com/labstack/echo/v4" @@ -157,9 +156,9 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { } // Index template - t, err := template.New("index").Parse(html) - if err != nil { - panic(fmt.Sprintf("echo: %v", err)) + t, tErr := template.New("index").Parse(html) + if tErr != nil { + panic(fmt.Errorf("echo: %w", tErr)) } return func(next echo.HandlerFunc) echo.HandlerFunc { @@ -176,7 +175,7 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { if err != nil { return } - name := filepath.Join(config.Root, filepath.Clean("/"+p)) // "/"+ for security + name := path.Join(config.Root, path.Clean("/"+p)) // "/"+ for security if config.IgnoreBase { routePath := path.Base(strings.TrimRight(c.Path(), "/*")) @@ -187,12 +186,14 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { } } - file, err := openFile(config.Filesystem, name) + file, err := config.Filesystem.Open(name) if err != nil { - if !os.IsNotExist(err) { + if !isIgnorableOpenFileError(err) { return err } + // file with that path did not exist, so we continue down in middleware/handler chain, hoping that we end up in + // handler that is meant to handle this request if err = next(c); err == nil { return err } @@ -202,7 +203,7 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { return err } - file, err = openFile(config.Filesystem, filepath.Join(config.Root, config.Index)) + file, err = config.Filesystem.Open(path.Join(config.Root, config.Index)) if err != nil { return err } @@ -216,15 +217,13 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { } if info.IsDir() { - index, err := openFile(config.Filesystem, filepath.Join(name, config.Index)) + index, err := config.Filesystem.Open(path.Join(name, config.Index)) if err != nil { if config.Browse { return listDir(t, name, file, c.Response()) } - if os.IsNotExist(err) { - return next(c) - } + return next(c) } defer index.Close() @@ -242,11 +241,6 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { } } -func openFile(fs http.FileSystem, name string) (http.File, error) { - pathWithSlashes := filepath.ToSlash(name) - return fs.Open(pathWithSlashes) -} - func serveFile(c echo.Context, file http.File, info os.FileInfo) error { http.ServeContent(c.Response(), c.Request(), info.Name(), info.ModTime(), file) return nil diff --git a/middleware/static_other.go b/middleware/static_other.go new file mode 100644 index 000000000..0337b22af --- /dev/null +++ b/middleware/static_other.go @@ -0,0 +1,12 @@ +//go:build !windows + +package middleware + +import ( + "os" +) + +// We ignore these errors as there could be handler that matches request path. +func isIgnorableOpenFileError(err error) bool { + return os.IsNotExist(err) +} diff --git a/middleware/static_windows.go b/middleware/static_windows.go new file mode 100644 index 000000000..0ab119859 --- /dev/null +++ b/middleware/static_windows.go @@ -0,0 +1,23 @@ +package middleware + +import ( + "os" +) + +// We ignore these errors as there could be handler that matches request path. +// +// As of Go 1.20 filepath.Clean has different behaviour on OS related filesystems so we need to use path.Clean +// on Windows which has some caveats. The Open methods might return different errors than earlier versions and +// as of 1.20 path checks are more strict on the provided path and considers [UNC](https://en.wikipedia.org/wiki/Path_(computing)#UNC) +// paths with missing host etc parts as invalid. Previously it would result you `fs.ErrNotExist`. +// +// For 1.20@Windows we need to treat those errors the same as `fs.ErrNotExists` so we can continue handling +// errors in the middleware/handler chain. Otherwise we might end up with status 500 instead of finding a route +// or return 404 not found. +func isIgnorableOpenFileError(err error) bool { + if os.IsNotExist(err) { + return true + } + errTxt := err.Error() + return errTxt == "http: invalid or unsafe file path" || errTxt == "invalid path" +}