packr icon indicating copy to clipboard operation
packr copied to clipboard

Defaultresolver accesses current directory before box directory

Open matthijskooijman opened this issue 6 years ago • 3 comments

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 https://github.com/gobuffalo/packr/blob/cef464b1513d9b6cf3d4ad6d6983a716e048e89d/v2/file/resolver/disk.go#L25-L29

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: https://github.com/gobuffalo/packr/blob/cef464b1513d9b6cf3d4ad6d6983a716e048e89d/v2/file/resolver/resolver.go#L15-L20

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 avatar Apr 11 '19 14:04 matthijskooijman

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

matthijskooijman avatar Apr 11 '19 14:04 matthijskooijman

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 ?

nlepage avatar May 14 '19 05:05 nlepage

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

zerobyted avatar Nov 25 '20 17:11 zerobyted