Skip to content
This repository has been archived by the owner on Jun 25, 2022. It is now read-only.

box.open open "/" return dir #173

Closed
zcx2001 opened this issue Mar 9, 2019 · 11 comments
Closed

box.open open "/" return dir #173

zcx2001 opened this issue Mar 9, 2019 · 11 comments

Comments

@zcx2001
Copy link

zcx2001 commented Mar 9, 2019

now box.Open("/") is return a file (isDir is false) and err is nil
on http.serveFile the code in net/http/fs.go

	if redirect {
		// redirect to canonical path: / at end of directory url
		// r.URL.Path always begins with /
		url := r.URL.Path
		if d.IsDir() {
			if url[len(url)-1] != '/' {
				localRedirect(w, r, path.Base(url)+"/")
				return
			}
		} else {
			if url[len(url)-1] == '/' {
				localRedirect(w, r, "../"+path.Base(url))
				return
			}
		}
	}

	// redirect if the directory name doesn't end in a slash
	if d.IsDir() {
		url := r.URL.Path
		if url[len(url)-1] != '/' {
			localRedirect(w, r, path.Base(url)+"/")
			return
		}
	}

	// use contents of index.html for directory, if present
	if d.IsDir() {
		index := strings.TrimSuffix(name, "/") + indexPage
		ff, err := fs.Open(index)
		if err == nil {
			defer ff.Close()
			dd, err := ff.Stat()
			if err == nil {
				name = index
				d = dd
				f = ff
			}
		}
	}	
}

code is redirect to "/" not “/index.html"

i fix the code in func (b *Box) Open(name string) (http.File, error) is

        plog.Debug(b, "Open", "name", name)
        if strings.EqualFold("/", name) {
		return file.NewDir("/")
	}
	f, err := b.Resolve(name)
	if err != nil {
		if len(filepath.Ext(name)) == 0 {
			return b.openWoExt(name)
		}
		return f, err
	}
	f, err = file.NewFileR(name, f)
	plog.Debug(b, "Open", "name", f.Name(), "file", f.Name())
	return f, err
@vadimpiven
Copy link

vadimpiven commented Mar 24, 2019

Hot fix from above helps with opening /index.html when web server asks for / folder, but it doesn't help with opening /dir/index.html when web server asks for /dir. (I use github.com/labstack/echo as a web server. When using http.FileServer(http.Dir("static")) instead of http.FileServer(s), where s is s := packr.New("static", "static") from the example program below, everything works correctly.) The problem is directories are not detected at all. Example program:

package main

import (
	"fmt"
	"github.com/gobuffalo/packr/v2"
)

func main() {
	s := packr.New("static", "static")
	f, err := s.Open("/dir")
	if err != nil {
		fmt.Println(err)
	} else {
		fmt.Println(f)
		fmt.Println(f.Readdir(0))
	}
}

Wether you build with go build or packr2 build it's output is:


[{/dir/ 0 {13771438225996173496 3532839 0x17af2c0} false}] <nil>

And ls -R output in project directory is:

main.go static

./static:
dir        index.html test.html

./static/dir:
index.html test.html

Is there any hot fix for this?

@vadimpiven
Copy link

I've found a solution. This line of code should be replaced with

info, err := f.FileInfo()
if err != nil {
	return f, err
}
if info.IsDir() {
	f, err = file.NewDir(name)
} else {
	f, err = file.NewFileR(name, f)
}

and this line should be replaced with

info, err := f.FileInfo()
if err != nil {
	return f, errors.WithStack(err)
}
if info.IsDir() {
	f, err = file.NewDir(key)
} else {
	f, err = file.NewFile(key, b)
}

It worked for me, but I'm not sure is it a complete solution.

@matthijskooijman
Copy link

I've also seen Open("/") return success, but a file descriptor rather than a directory descriptor. I suspect that this might prevent net/http from translating the / url into /index.html internally (since it only appends index.html for directories), so perhaps that is also why Box::openWoExt exists (which seems like the wrong place to handle adding index.html).

Also, I suspect this might tie in with #192, which points out some other places where the responsibilities are (AFAICS) not very well defined and divided between box, resolvers and others.

@Lesterpig
Copy link
Contributor

Hello, this looks like a severe issue. From my understanding, the README example (https://github.com/gobuffalo/packr#usage-with-http) does not work anymore for index.html files.

Is there any elegant way to tackle that in the current version? Thanks! 👍

@markbates
Copy link
Member

It supports it.

@Lesterpig
Copy link
Contributor

It supports it.

Sorry, but no, it's not; or I'm not understanding something here 😞
I just created a file in my GOPATH, copied the main.go file from the README, and added a templates directory:

$ tree .
.
├── main.go
└── templates
    ├── index.html
    └── test.html

I cannot access index.html, but test.html works fine.

$ go version
go version go1.12.4 linux/amd64
$ cd $GOPATH/src/github.com/gobuffalo/packr/v2 && git describe --tags
v2.1.0

@markbates
Copy link
Member

markbates commented Apr 14, 2019 via email

@sagikazarmark
Copy link
Contributor

@markbates I've run the tests, they pass. Then exchanged the resolver with a disk resolver. Tests fail.

I'm trying to figure out what's going on.

@sagikazarmark
Copy link
Contributor

As far as I can tell the difference is the HexGzip used in the test returns an error when you try to resolve the "/" path, whereas the Disk resolver returns a directory.

Here is the relevant code:

packr/v2/box.go

Lines 146 to 153 in e0e2368

f, err := b.Resolve(name)
if err != nil {
if len(filepath.Ext(name)) == 0 {
return b.openWoExt(name)
}
return f, err
}
f, err = file.NewFileR(name, f)

So I think this is a valid issue after all.

@sagikazarmark
Copy link
Contributor

Pushed an experimental fix

@sagikazarmark
Copy link
Contributor

This seems to be a workaround:

func (b *binaryFileSystem) Open(name string) (http.File, error) {
	// This is necessary because of https://github.com/gobuffalo/packr/issues/173
	if b.fs.HasDir(name) {
		return file.NewDir(name)
	}

	return b.fs.Open(name)
}

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

No branches or pull requests

6 participants