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

301 status and ERR_TOO_MANY_REDIRECTS for static dir #20

Open
frederikhors opened this issue Oct 18, 2021 · 10 comments · Fixed by #21 or #23
Open

301 status and ERR_TOO_MANY_REDIRECTS for static dir #20

frederikhors opened this issue Oct 18, 2021 · 10 comments · Fixed by #21 or #23

Comments

@frederikhors
Copy link
Contributor

frederikhors commented Oct 18, 2021

I have these dirs:

- static
  - index.html
  - js
    - main.js

and I'm using the code listed here: https://bunrouter.uptrace.dev/guide/serving-static-files.html#fs-sub.

Single file retrieving is working fine (eg. static/js/main.js).

The issue I'm having is when I try to open static/. I get in browser a 301 for static/ to static and vice-versa and then ERR_TOO_MANY_REDIRECTS in Chrome.

Is this my fault?

@frederikhors
Copy link
Contributor Author

frederikhors commented Oct 18, 2021

You can verify this by changing this line in file-server example:

router.GET("/files/*path", bunrouter.HTTPHandler(fileServer))

to

router.GET("/*path", bunrouter.HTTPHandler(fileServer))

@vmihailenco
Copy link
Member

Please try v1.0.2 and the updated example.

@frederikhors
Copy link
Contributor Author

It works but I'm getting this error now with v1.0.2:

panic: /*path already handles GET

I think because of these lines:

router.GET("/", func(w http.ResponseWriter, r bunrouter.Request) error {
	serveHTMLFallback(w, r.Request, fsys)

	return nil
})
  • Do I still need the first router.GET("/", ...) now?

  • What do you think about this code? Can I avoid something?

const indexHTML = "index.html"

func serveHTMLFallback(w http.ResponseWriter, r *http.Request, fsys fs.FS) {
	indexHTMLFile, err := fsys.Open(indexHTML)
	if err != nil {
		panic(err)
	}

	indexHTMLFileStat, err := indexHTMLFile.Stat()
	if err != nil {
		panic(err)
	}

	indexHTMLModTime := indexHTMLFileStat.ModTime()

	if err = indexHTMLFile.Close(); err != nil {
		panic(err)
	}

	http.ServeContent(w, r, indexHTML, indexHTMLModTime, indexHTMLFile.(io.ReadSeekCloser))
}

func fileServer(router *bunrouter.Router) {
	fsys := getStaticDirAsFileSystem()

	router.GET("/", func(w http.ResponseWriter, r bunrouter.Request) error {
		serveHTMLFallback(w, r.Request, fsys)

		return nil
	})

	router.GET("/*path", func(w http.ResponseWriter, r bunrouter.Request) error {
		// From https://www.alexedwards.net/blog/disable-http-fileserver-directory-listings#using-middleware
		if r.URL.Path != "/" && r.URL.Path[len(r.URL.Path)-1] == '/' {
			serveHTMLFallback(w, r.Request, fsys)
			return nil
		}

		if _, err := fs.Stat(fsys, r.URL.Path[1:]); err != nil {
			serveHTMLFallback(w, r.Request, fsys)
			return err
		}

		http.FileServer(http.FS(fsys)).ServeHTTP(w, r.Request)

		return nil
	})
}

router := bunrouter.New()

fileServer(router)

@vmihailenco
Copy link
Member

panic: /*path already handles GET

I will fix this shortly.

@vmihailenco vmihailenco reopened this Oct 19, 2021
@frederikhors
Copy link
Contributor Author

Ok. I'm interested in your opinion on that code to serve the files of a SPA (single page application).

@frederikhors
Copy link
Contributor Author

panic: /*path already handles GET

I will fix this shortly.

I think it is ok without that first GET anyway. It's working good! And it is like go-chi works too.

@frederikhors
Copy link
Contributor Author

If you think about it, we don't need two routes that handle the same calls:

  1. router.GET("/", ...)

  2. router.GET("/*path", ...)

This is redundant!

Only the latter is ok: router.GET("/*path", ...).

I personally don't like writing path after the *.

@vmihailenco
Copy link
Member

This is redundant!

Okay, let's keep this behavior. I can imagine arguments against such routing too, but perhaps it is fine.

I personally don't like writing path after the *.

That is is needed so you can retrieve the param via req.Param("path").

@vmihailenco
Copy link
Member

What do you think about this code? Can I avoid something?

Looks good enough. I will try to add an example later.

@vmihailenco vmihailenco reopened this Oct 19, 2021
@frederikhors
Copy link
Contributor Author

What do you think about this code? Can I avoid something?

Looks good enough. I will try to add an example later.

Yeah I think would be awesome to have a SPA file server example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants