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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Can't find index.html with PathPrefix? #1439

Closed
rayterrill opened this issue Jul 15, 2021 · 16 comments 路 Fixed by #1547
Closed

馃悰 Can't find index.html with PathPrefix? #1439

rayterrill opened this issue Jul 15, 2021 · 16 comments 路 Fixed by #1547

Comments

@rayterrill
Copy link

rayterrill commented Jul 15, 2021

Fiber version
v2.14.0

Issue description
Getting a 404 trying to browse to / mounting a React app using PathPrefix and Index

Code snippet

package main

import (
	"embed"
	"net/http"

	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/fiber/v2/middleware/filesystem"
)

// Embed build directory
//go:embed ui/build/*
var embedDirReact embed.FS

func main() {
	app := fiber.New()

	//serve basic api route returning data
	app.Get("/ping", func(c *fiber.Ctx) error {
		return c.SendString("Hello, World!")
	})

	//serve react app
	app.Use("/", filesystem.New(filesystem.Config{
		Root: http.FS(embedDirReact),
		PathPrefix: "ui/build",
		Index: "index.html",
	}))
	
	app.Listen(":3000")
}

Result:
Cannot GET /

Browsing to http://localhost:3000/index.html directly works fine.

@welcome
Copy link

welcome bot commented Jul 15, 2021

Thanks for opening your first issue here! 馃帀 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

The pathPrefix is used in front of the index path. Please test it with

"index.html"

@rayterrill
Copy link
Author

@ReneWerner87 sorry mate - i played around a ton with this and tried that just to see if it worked. Updated my config above to be the actual one I have (my fault), and it's the same behavior.

@wirepair
Copy link

I'm having a similar problem, what's weird is:

//go:embed static/favicon.ico
var faviconFile embed.FS

//go:embed static/*
var embedDirStatic embed.FS

...
	app.Use("/", filesystem.New(filesystem.Config{
		Root:       http.FS(embedDirStatic),
		PathPrefix: "",
		Browse:     false,
		Index:      "static/index.html",
	}))

	app.Use(favicon.New(favicon.ConfigDefault))

Works just fine treating index.html as the index. But I can't access GET /index.html, I have to access GET /static/index.html.

If I set PathPrefix to "static" or "static/" then it stops working, but I can access GET /index.html.

@wirepair
Copy link

Ah, I think I figured it out, it's due to how the paths are being prefixed with / in filesystem.go

You can easily reproduce this with:

test := http.FS(embedDirStatic)
_, err := test.Open("/static/")
if err != nil {
	log.Fatal("boop") // will get called
}

vs 
test := http.FS(embedDirStatic)
_, err := test.Open("static")
if err != nil {
	log.Fatal("boop") // will not get called
}

Here's the offending code filesystem.go#L91:

	if cfg.PathPrefix != "" && !strings.HasPrefix(cfg.PathPrefix, "/") {
		cfg.PathPrefix = "/" + cfg.PathPrefix
	}

@ReneWerner87
Copy link
Member

yes, i also found the errors, the filesytem middleware is not yet fully ready for embedded filesystems

these filesystems behave a little bit different, so the index file can not be found, if i have time today, i will try to provide a fix

@rayterrill
Copy link
Author

Thanks both of y'all. I'm coming over to Golang from NodeJS and Express and I have to say Fiber has been a real treat to work with. 鉂わ笍

@ReneWerner87
Copy link
Member

didn't make it today unfortunately, try to provide a fix on the weekend, if you have time before and find a solution, then feel free to create a pull request

@wirepair
Copy link

I took a crack at fixing this, but we are unfortunately out of luck testing the cfg.Root underlying type. With embed it's actually an internal type (http.ioFS) so we can't type check it :(.

I tried with:

  // non http.Dir types (embed.FS) should not prefix
  _, shouldPrefix := cfg.Root.(http.Dir)
  ... 
  return func(c *fiber.Ctx) (err error) {
    ...
    // Strip prefix
    path := strings.TrimPrefix(c.Path(), prefix)
    if !shouldPrefix {
        path = strings.TrimPrefix(path, "/")
    } else if !strings.HasPrefix(path, "/") {
        path = "/" + path
    }
   ...
}

But i'm worried that all the other filesystems would fail with this. One option is to have a cfg option of ShouldStripPrefix set to false, but can be enabled for different file system types (but that's super ugly too).

Not sure the best way to fix this without requiring additional information from the caller.

@wirepair
Copy link

wirepair commented Jul 19, 2021

Super janky, but it's a work around:

//go:embed testdata/*
var testEmbedDirStatic embed.FS

// For wrapping embed.FS to suffix any trailing / directories
// with an index.html
type EmbedFS struct {
	f embed.FS
}

// append index.html to any 'directories' that are opened
func (embed *EmbedFS) Open(name string) (fs.File, error) {
	if strings.HasSuffix(name, "/") {
		name += "index.html"
	}
	return embed.f.Open(name)
}

// wrap the embed.FS in &EmbedFS{...} and pass that to http.FS
func Setup(...) {
	app.Use("/embed", New(Config{
		Root:       http.FS(&EmbedFS{testEmbedDirStatic}),
		PathPrefix: "testdata",
		Index:      "index.html",
	}))
}

@kobenauf
Copy link

kobenauf commented Aug 21, 2021

This seems possibly related to #1490.

The workaround above wouldn't work for a situation where there's a directory without an index file but that does have an html file with the same name as the directory (this is how NextJs exports project structures by the way):

\orgs
\orgs\[id].html
orgs.html

Fiber looks for \orgs\index.html, but if it's not there, does not find orgs.html, and instead says reading the directory '\orgs' is Forbidden.

@imraan-go
Copy link

This seems possibly related to #1490.

The workaround above wouldn't work for a situation where there's a directory without an index file but that does have an html file with the same name as the directory (this is how NextJs exports project structures by the way):

\orgs
\orgs\[id].html
orgs.html

Fiber looks for \orgs\index.html, but if it's not there, does not find orgs.html, and instead says reading the directory '\orgs' is Forbidden.

If that's the case, you can overwrite index file with fiber.Static{}.

@goliatone
Copy link

I ran into this issue today, what I saw is the same behavior @wirepair described, the Open fails due to PathPrefix having a / suffix.
If your PathPrefix is testdata then this check fails:

file, err = cfg.Root.Open("/testdata/")

We could add an extra check after we add the prefix:

if path != "/" && strings.HasSuffix(path, "/") {
	path = strings.TrimSuffix(path, "/")
}

@wedobetter
Copy link

wedobetter commented Sep 24, 2021

this is doing it for me, while I wait for a proper fix.

//go:embed dist/bws/*
var angular embed.FS


app.Use("/", filesystem.New(filesystem.Config{
    Root: http.FS(angular),
    Browse:       true,
    PathPrefix: "/dist/bws",
    Index:        "index.html",
    NotFoundFile: "/dist/bws/index.html",
  }))

@fufuok
Copy link
Contributor

fufuok commented Oct 6, 2021

this is doing it for me, while I wait for a proper fix.

//go:embed dist/bws/*
var angular embed.FS


app.Use("/", filesystem.New(filesystem.Config{
    Root: http.FS(angular),
    Browse:       true,
    PathPrefix: "/dist/bws",
    Index:        "index.html",
    NotFoundFile: "/dist/bws/index.html",
  }))

go.mod

module tmp_fiber_1439

go 1.17

require github.com/gofiber/fiber/v2 v2.19.1-0.20211005120320-587f3ae9df4b

require (
	github.com/andybalholm/brotli v1.0.2 // indirect
	github.com/klauspost/compress v1.13.4 // indirect
	github.com/valyala/bytebufferpool v1.0.0 // indirect
	github.com/valyala/fasthttp v1.29.0 // indirect
	github.com/valyala/tcplisten v1.0.0 // indirect
	golang.org/x/sys v0.0.0-20210514084401-e8d321eab015 // indirect
)

test

image

image

image

testCode

tmp_fiber_1439.zip

@ReneWerner87
Copy link
Member

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

Successfully merging a pull request may close this issue.

8 participants