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

Fix serving public/index.html as a static file #234

Merged
merged 8 commits into from
Mar 19, 2020

Conversation

nonrational
Copy link
Member

@nonrational nonrational commented Mar 9, 2020

This PR serves to illustrate behavior discussed in #140 and #87.
Fixes #140 (at least the part of it that is not covered by #87)

As of 0.13, puma-dev does not support serving "naked" static files without booting puma. It can, of course, be modified to support such behavior (see #87).

The headline here is #234 (comment)

Changes

  • link a bare-bones rack app (static-site) with a public directory containing greeting.txt
  • Ensure GET localhost/greeting.txt is handled by puma-dev
  • Ensure GET localhost/foo is handled by rack.
  • Fix golint warnings about using Url instead of URL variable names.
  • use ServeContent to avoid redirecting index.html back to /. See No static content serving on test domain #140 (comment)

@nonrational nonrational changed the title tests that exercise current static file functionality Fix serving static files named index.html Mar 10, 2020
@nonrational
Copy link
Member Author

h/t @momolog for pushing on this issue and providing critical debugging feedback.

@nonrational nonrational requested a review from evanphx March 10, 2020 22:42
@@ -72,7 +72,7 @@ func launchPumaDevBackgroundServerWithDefaults(t *testing.T) func() {
}
}

func getUrlWithHost(t *testing.T, url string, host string) string {
func getURLWithHost(t *testing.T, url string, host string) string {
Copy link
Member Author

Choose a reason for hiding this comment

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

golint wants URL not Url

@@ -0,0 +1 @@
<html><h1>index.html</h1></html>
Copy link
Member Author

Choose a reason for hiding this comment

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

this should never show up. it's included here to verify that we weren't able to look outside the public directory.

http.ServeFile(w, req, path)
return httputil.ErrHandled
if ofile, err := os.Open(path); err == nil {
http.ServeContent(w, req, req.URL.Path, fi.ModTime(), io.ReadSeeker(ofile))
Copy link
Member Author

Choose a reason for hiding this comment

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

https://golang.org/pkg/net/http/#ServeFile

As another special case, ServeFile redirects any request where r.URL.Path ends in "/index.html" to the same path, without the final "index.html". To avoid such redirects either modify the path or use ServeContent.

Copy link
Member Author

Choose a reason for hiding this comment

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

the downside with this approach is that you can't just drop an index.html in public and go to mysite.localhost. you have to navigate to mysite.localhost/index.html.

but, puma-dev never supported that anyway, given the req.URL.Path != "/"

I figure this is a good enhancement to consider when tackling #87

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine, your justification makes sense.

@nonrational nonrational changed the title Fix serving static files named index.html Fix serving public/index.html as a static file Mar 10, 2020
http.ServeFile(w, req, path)
return httputil.ErrHandled
if ofile, err := os.Open(path); err == nil {
http.ServeContent(w, req, req.URL.Path, fi.ModTime(), io.ReadSeeker(ofile))
Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine, your justification makes sense.

@nonrational nonrational merged commit 70c7b26 into puma:master Mar 19, 2020
@nonrational nonrational deleted the static-files branch March 19, 2020 17:03
@nonrational nonrational added this to the v0.14 milestone Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No static content serving on test domain
2 participants