Skip to content

Fix Static files route not working #1723

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Fix Static files route not working #1723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8635642
Update echo.go
pwli0755 Dec 17, 2020
e08b6f9
Update echo_test.go
pwli0755 Dec 17, 2020
74b84a5
Fix Static files route not working
pwli0755 Dec 17, 2020
3d8e07c
Fix Static files route not working
pwli0755 Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions echo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl
}
return c.File(name)
}
get(prefix, h)
Copy link
Contributor

@aldas aldas Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then dealing with this method we need to consider where is is called. Is it e.Static() or group.Static()

  • e.Static() is on server root level i.e. http://server/assets/cat.gif URLs
  • group.Static is on group level which means in addition to this method prefix we have group path as prefix in url. i.e. http://server/group/assets/cat.gif

and regarding to prefix we should consider it ends with slash or not
i.e. prefixes:

  • ``
  • /
  • /assets
  • /assets/

now lets add two possibilities here: if group path is /user or /user/ what routes we will end up with?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like that

	// prefix does not end with slash
	if prefix != "" && prefix[len(prefix) - 1] != '/' {
		get(prefix, h) // i.e. prefix=/assets and URL `/assets` must give directory index
		return get(prefix+"/*", h) // and asterisk to match anything in that directory
	}
	if prefix != "" {
		get(prefix[:len(prefix) - 1], h) // i.e. prefix=/assets/ then add route `/assets` for directory index
	}
	// i.e. prefix="" (empty) and URL `http://server` or `http://server/` must give directory index
	// i.e. prefix=/assets/ and URL `/assets/` must give directory index, `/assets/*` serve files
	return get(prefix+"*", h)

if prefix == "/" {
return get(prefix+"*", h)
}
Expand Down
43 changes: 43 additions & 0 deletions echo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ func TestEchoStatic(t *testing.T) {
expectHeaderLocation: "/folder/",
expectBodyStartsWith: "",
},
{
name: "Directory Redirect with non-root path",
givenPrefix: "/static",
givenRoot: "_fixture",
whenURL: "/static",
expectStatus: http.StatusMovedPermanently,
expectHeaderLocation: "/static/",
expectBodyStartsWith: "",
},
{
name: "Directory with index.html",
givenPrefix: "/",
Expand Down Expand Up @@ -161,6 +170,40 @@ func TestEchoStatic(t *testing.T) {
}
}

func TestEchoStaticRedirectIndex(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not run tests like that because:

  • it adds constant 200ms to execution time - using sleeps makes tests slow in long run and brittle some cases
  • it assumes that port 1323 is free - use 127.0.0.1:0 :0 etc instead. :0 would bind to random port and you can get the address where server was binded with addr := e.Listener.Addr(). See documentation https://golang.org/pkg/net/#Listen

    If the port in the address parameter is empty or "0", as in "127.0.0.1:" or "[::1]:0", a port number is automatically chosen. The Addr method of Listener can be used to discover the chosen port.

TestEchoStatic that you modified above is pretty much same as this is. When you spin up new http server. Each request will invoke ServeHTTP(rec, req) method same as TestEchoStatic does ... but without starting a server.

assert := assert.New(t)
e := New()

// HandlerFunc
e.Static("/static", "_fixture")

errCh := make(chan error)

go func() {
errCh <- e.Start("127.0.0.1:1323")
}()

time.Sleep(200 * time.Millisecond)

if resp, err := http.Get("http://127.0.0.1:1323/static"); err == nil {
defer resp.Body.Close()
assert.Equal(http.StatusOK, resp.StatusCode)

if body, err := ioutil.ReadAll(resp.Body); err == nil {
assert.Equal(true, strings.HasPrefix(string(body), "<!doctype html>"))
} else {
assert.Fail(err.Error())
}

} else {
assert.Fail(err.Error())
}

if err := e.Close(); err != nil {
t.Fatal(err)
}
}

func TestEchoFile(t *testing.T) {
e := New()
e.File("/walle", "_fixture/images/walle.png")
Expand Down