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

Defaultresolver accesses current directory before box directory #192

Open
matthijskooijman opened this issue Apr 11, 2019 · 3 comments
Open
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@matthijskooijman
Copy link

matthijskooijman commented Apr 11, 2019

When running in non-packed mode (e.g. through normal go build), the idea is (AFAIU) that the directory passed to to packr.New() is used to serve files from. However, it seems that the global default resolve also allows serving files from the working directory).

Best illustrated by an example:

$ cat main.go 
package main

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

func main() {
        plog.Logger = logger.New(logger.DebugLevel)

        box := packr.New("Box", "./box")

        _, err := box.Open("/file_in_box.txt")
        fmt.Println("err=", err)

        _, err = box.Open("/box/file_in_box.txt")
        fmt.Println("err=", err)

        _, err = box.Open("/file_outside_box.txt")
        fmt.Println("err=", err)

        _, err = box.Open("/nonexistent.txt")
        fmt.Println("err=", err)
}

My file tree looks like this:

$ find
./box
./box/file_in_box.txt
./file_outside_box.txt
./main.go

So, I would expect the first Open call to succeed for the file inside the box, and the other three to fail. However, the actual output looks like this:

DEBU[2019-04-11T15:53:59+02:00] packr#New name=Box path=./box
DEBU[2019-04-11T15:53:59+02:00] packr#findBox key=Box name=Box
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#found box="{\"path\":\"/home/matthijs/foo/box\",\"name\":\"Box\",\"resolution_dir\":\"/home/matthijs/foo/box\",\"default_resolver\":null}"
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open name=/file_in_box.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box key=file_in_box.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#InsideResolve name=file_in_box.txt path=/home/matthijs/foo/file_in_box.txt root=/home/matthijs/foo
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#InsideResolve name=/home/matthijs/foo/box/file_in_box.txt path=/home/matthijs/foo/box/file_in_box.txt root=/home/matthijs/foo
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box file=file_in_box.txt key=file_in_box.txt
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open file=/file_in_box.txt name=/file_in_box.txt
err: <nil>
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open name=/box/file_in_box.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box key=box/file_in_box.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#InsideResolve name=box/file_in_box.txt path=/home/matthijs/foo/box/file_in_box.txt root=/home/matthijs/foo
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box file=box/file_in_box.txt key=box/file_in_box.txt
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open file=/box/file_in_box.txt name=/box/file_in_box.txt
err: <nil>
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open name=/file_outside_box.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box key=file_outside_box.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#InsideResolve name=file_outside_box.txt path=/home/matthijs/foo/file_outside_box.txt root=/home/matthijs/foo
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box file=file_outside_box.txt key=file_outside_box.txt
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open file=/file_outside_box.txt name=/file_outside_box.txt
err: <nil>
DEBU[2019-04-11T15:53:59+02:00] *packr.Box#Open name=/nonexistent.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box key=nonexistent.txt
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#InsideResolve name=nonexistent.txt path=/home/matthijs/foo/nonexistent.txt root=/home/matthijs/foo
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#InsideResolve name=/home/matthijs/foo/box/nonexistent.txt path=/home/matthijs/foo/box/nonexistent.txt root=/home/matthijs/foo
DEBU[2019-04-11T15:53:59+02:00] *resolver.Disk#Resolve box=Box err="{\"Op\":\"stat\",\"Path\":\"/home/matthijs/foo/box/nonexistent.txt\",\"Err\":2}" key=/home/matthijs/foo/box/nonexistent.txt
err: stat /home/matthijs/foo/box/nonexistent.txt: no such file or directory

As you can see, only the last call (to the actually nonexistent file) returns an error, while the second and third are resolved relative to the working directory rather than the box directory.

Note that I added a little debug information. After these lines

func (d *Disk) Resolve(box string, name string) (file.File, error) {
path := OsPath(name)
if !filepath.IsAbs(path) {
path = filepath.Join(OsPath(d.Root), path)
}

I added this line:

    plog.Debug(d, "InsideResolve", "root", d.Root, "name", name, "path", path)

Also note that some debug output is missing, since all the parser initialization happens before I set the loglevel.

As you can see, the Disk::Resolve function is called twice. Both times the root directory seems bogus (referring to the directory, not the box), but the second time the name parameter is already a full path (meaning that Disk::Resolve does not really have anything useful to do).

This seems to stem from the Box::Resolve function, which does this:

https://github.com/gobuffalo/packr/blob/v2.1.0/v2/box.go#L196-L220

Essentially, it always tries to use the resolver.DefaultResolver first (also the box' defaultResolver, but that's not set here). The default resolve is defined as:

func defaultResolver() Resolver {
pwd, _ := os.Getwd()
return &Disk{
Root: pwd,
}
}

IOW, it looks in the current directory the first time it is called, and ignores its root directory when it is called for the second time.

All this seems confusing to me: I would expect such a box to have its own disk resolver with a proper root path. I also wonder what the point of a global default resolver is. Resolving files in the current directory sounds like a potential security issue. Finally, I wonder why Box::Resolve does this fallback to the box' ResolutionDir, since that sounds like a task for the resolver rather than this function.

It could be that the actual problem is that I have boxes without a defaultResolver of their own, in which case the parsing might be at fault (but even then, the code pointed above makes no sense to me, and if some of it is not meant to be used, it should probably be removed?).

@matthijskooijman
Copy link
Author

(w00ps, submitted my issue to soon. Description updated now)

@nlepage
Copy link
Contributor

nlepage commented May 14, 2019

Hi,

I'm not sure what the expected behavior is here.

But I agree that it might get a little confusing, especially when switching between dev mode and built mode.

When using a simple file structure like this one:

packr-app/
├── app
│   └── test.html (Right one)
├── go.mod
├── go.sum
├── main.go
└── test.html (Wrong one)

with main.go:

package main

import (
	"log"
	"net/http"

	"github.com/gobuffalo/packr/v2"
)

func main() {
	var app = packr.New("App", "./app")

	http.Handle("/", http.FileServer(app))

	log.Fatal(http.ListenAndServe(":8080", nil))
}

I have two versions of test.html, the "right one" in app/ and the "wrong one" packr-app/.

Running go run . in packr-app/ and opening http://localhost:8080/test.html gets me the wrong one, which is a little unexpected.

After executing packr2 in packr-app, running again go run . and opening http://localhost:8080/test.html now gets me the right one, which is what I expected.

@markbates Is this the expected behavior ?

@markbates markbates added bug Something isn't working help wanted Extra attention is needed labels May 21, 2019
@zerobyted
Copy link

any package level solution

var _box *packr.Box
func init() {
    _, filename, _, _ := runtime.Caller(0)
    _box = packr.New("name", "../../assets/templates") // relative to this file path
    _box.DefaultResolver = &resolver.Disk{Root: path.Clean(path.Join(path.Dir(filename), _box.Path))}
}

that works for me using this box at any level of app entry point

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants