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

Revert "box.Open: Do not call file.NewFileR when IsDir true, fix #198" #217

Merged
merged 1 commit into from
Jul 8, 2019

Conversation

markbates
Copy link
Member

When running the tests locally I keep getting the following error. I'm reverting this PR until the tests pass cleanly.

--- FAIL: Test_Box_Open (0.00s)
    require.go:157: 
        	Error Trace:	box_test.go:216
        	Error:      	Not equal: 
        	            	expected: true
        	            	actual  : false
        	Test:       	Test_Box_Open
        	Messages:   	stat.IsDir() != true for path "/baz"

@markbates markbates merged commit dc520c9 into master Jul 8, 2019
@daniel-sullivan
Copy link

daniel-sullivan commented Jul 12, 2019

Hi Mark, sorry if I'm missing something but wouldn't it seem that "/baz" should return IsDir = true?

We're currently being affected by this issue and I can confirm that applying the PR in its original form works fine, whereas the current git version (dc520c9) is still showing the issue.

In our case the flow that we're seeing is the request comes in through box.go/Resolve (name: "assets/design/cart" - redirected by net/http/fs.go "/assets/design/cart/index.html"). The output is:

name: "assets\design\cart"
info: {
  Path: "assets\design\cart"
  size: 0
  isDir: true
}

There is no err being set so there is never a chance to run b.openWoExt(name) and it simply falls through to the file.NewFileR call which creates a new file (not a directory)

Hopefully I've misunderstood something in the flow. If you'd like I could throw up a simple Buffalo repository with a similar file structure to what we're using to help with debugging?

EDIT: I don't seem to be able to replicate this myself in direct tests against V2... will keep on working on our particular issue but the troubleshooting above should still be reasonable.

EDIT 2: Replicated against V2 when using a file based store. I've created some very crude tests in the below referenced repo in /v2/http_file_test.go

daniel-sullivan pushed a commit to charm-jp/packr that referenced this pull request Jul 12, 2019
Added tests showing failure when there is a deeply embedded index.html (/foo/bar/deep/index.html)
daniel-sullivan pushed a commit to charm-jp/packr that referenced this pull request Jul 12, 2019
Added tests showing failure when there is a deeply embedded index.html
(/foo/bar/deep/index.html)

Main test /v2/http_file_test.go
Pulls data from: /fixtures
AlbinoDrought added a commit to AlbinoDrought/creamy-videos that referenced this pull request Jul 14, 2019
some kind of change was made and versions of creamy-videos built
with newer versions of packr2 caused redirect loops on webroot.
attempted fixing this using a couple methods but didn't have any luck.

there was a fix added here but it was later (temporarily) reverted:

gobuffalo/packr#198
gobuffalo/packr#217
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants