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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readdir should treat zero for count value as it was negative to be fully compatible with the interface #68

Closed
shakahl opened this issue May 27, 2019 · 7 comments

Comments

Projects
None yet
4 participants
@shakahl
Copy link

commented May 27, 2019

Hi @rakyll

Thanks for the statik, it's a great lib:)

I found an issue in the Readdir implementation while I was using another library that called statik's fs.Readdir(0) and it resulted in a problem. No file was found in this case.

It should treat the zero value and the negatives the same as the GoDoc recommends: https://golang.org/pkg/os/#File.Readdir

The problematic Readdir implementation:

statik/fs/fs.go

Line 190 in 3bac566

// If dirIdx reaches the end and the count is a positive value,

I fould the issue when I was using this library: shurcooL/httpfs#8

All best,
@shakahl

@shakahl

This comment has been minimized.

Copy link
Author

commented May 27, 2019

Maybe related to this issue: #58

@rakyll

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

Happy to review the change if you would like to send one! Otherwise, I can only look at it later this week.

@dmitshur

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2019

@shakahl Are you still planning to send a PR? If not, I can do it.

@dmitshur

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2019

I'll send a PR since I'm already looking at this, it's a very small change.

Edit: Sent PR #69. /cc @rakyll

@jcchavezs

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2019

@shakahl

This comment has been minimized.

Copy link
Author

commented Jul 8, 2019

@dmitshur Thank you for the PR! :) Unfortunately, I've been a little bit overwhelmed in the last few weeks.

@dmitshur

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

No problem @shakahl, that's very relatable. Thanks for reporting this issue.

@rakyll rakyll closed this in #69 Jul 9, 2019

rakyll added a commit that referenced this issue Jul 9, 2019

fs: handle Readdir(0) same as Readdir(< 0) (#69)
Implementations of the File.Readdir method are expected to treat
all n <= 0 values the same, returning all the os.FileInfo structures
from the directory in a single slice. This is documented at
https://godoc.org/net/http#File.Readdir and https://godoc.org/os#File.Readdir.

Previously, the n == 0 value was not being handled correctly.
This change fixes it by making it work the same as all other
negative values of n.

Add a test case for it.

Also simplify string(out.Bytes()) to just out.String() in a test.

Fixes #68.
Fixes shurcooL/httpfs#8.
Closes shurcooL/httpfs#9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.