Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latest FS improvement breaks static routes #2117

Closed
3 tasks done
heat1q opened this issue Mar 9, 2022 · 5 comments
Closed
3 tasks done

Latest FS improvement breaks static routes #2117

heat1q opened this issue Mar 9, 2022 · 5 comments
Labels

Comments

@heat1q
Copy link
Contributor

heat1q commented Mar 9, 2022

Issue Description

It appears that the file system improvement (#2064) in the latest release v4.7.0 is breaking the backwards compatibility of static routes. Files that are registered with the Static method are no longer served on that route and a 404 is returned. This issue has already been briefly mentioned in the comments of #2064. I'd like to continue the discussion on this issue here and give a short example.

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

The Static method should work and files should be served the same as with <=4.6.3. In other words, the minor update should not change the behaviour of the Static method.

Actual behaviour

Static file routes registered with Static are not serving the given files when updating to echo v4.7.0.

Steps to reproduce

  1. Create a file in the main directory, eg. index.html
  2. Run the code below with echo v4.6.3
  3. curl http://localhost:8080 -v ==> 200 OK
  4. Run the code with echo v4.7.0
  5. curl http://localhost:8080 -v ==> 404 Not found

Working code to debug

.
├── go.mod
├── go.sum
├── index.html
└── main.go
// main.go
package main

import (
	"github.com/labstack/echo/v4"
)

func main() {
	e := echo.New()
	e.Static("", "./index.html")
	e.Logger.Fatal(e.Start(":8080"))
}

Version/commit

go version go1.17.6 darwin/arm64
echo v4.7.0

@aldas
Copy link
Contributor

aldas commented Mar 9, 2022

I can confirm that this happens.

It is limited to case when second parameter for e.Static is not a folder but a file. Although this seems to be "unintended" feature that e.Static can be used to serve single file. Second parameter is meant to be actually path (folder). https://echo.labstack.com/guide/static-files/#using-echostatic

v4.6.3 code

echo/echo.go

Lines 494 to 502 in 6b5e62b

func (e *Echo) Static(prefix, root string) *Route {
if root == "" {
root = "." // For security we want to restrict to CWD.
}
return e.static(prefix, root, e.GET)
}
func (common) static(prefix, root string, get func(string, HandlerFunc, ...MiddlewareFunc) *Route) *Route {
h := func(c Context) error {

Actually e.File("/favicon.ico", "images/favicon.ico") is proper way to serve single file (in this case e.Static("", "index.html")

I'll look what we can do with e.Static

@aldas
Copy link
Contributor

aldas commented Mar 9, 2022

Just to point out that serving single file with e.Static may not be that secure as e.Static takes everything from URL after your prefix (this case prefix = "" and uses it as suffix to your root (in this case ./index.html)

So if you have

e.Static("", "./index.html")

and for some odd reason that index.html is not a file but folder instead. We can download files from that folder by appending file name to request.

./
./main.go
./index.html/
./index.html/.htpasswd

with

x@x:~/code/$ curl -v  http://localhost:8080/.htpasswd
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /.htpass HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.74.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Accept-Ranges: bytes
< Content-Length: 4
< Content-Type: text/plain; charset=utf-8
< Last-Modified: Wed, 09 Mar 2022 18:27:46 GMT
< Date: Wed, 09 Mar 2022 18:29:15 GMT
< 
hi

* Connection #0 to host localhost left intact

@aldas aldas added the bug label Mar 9, 2022
@heat1q
Copy link
Contributor Author

heat1q commented Mar 10, 2022

Thanks for clarifying! I'll definitely use e.File for serving single files from now on.

Edit:
I just noticed when using the e.File method and appending the current directory to the path, it also seems to show different behaviour:

e.File("", "./index.html")

is working with v4.6.3 but returns a 404 Not found with v4.7.0.

@lammel
Copy link
Contributor

lammel commented Mar 14, 2022

@heat1q The edge case should be fixed in v4.7.1.
Could you please verify and close this issue. Thnx.

@heat1q
Copy link
Contributor Author

heat1q commented Mar 14, 2022

It's working now as expected. Thanks a lot!

@heat1q heat1q closed this as completed Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants