Repository: gin-gonic/gin
Pull request #3939 (2024-04-29 14:20:27)

feat(fs): Export, test and document OnlyFilesFS
I want to serve static assets from an `embed.FS` instance without directory listing. The `onlyFilesFS` wrapper is a perfect fit.

This PR exports `onlyFilesFS`, adds unit tests and documentation.

Usage example:

```go
//go:embed dist/*
var assets embed.FS

router.StaticFS("/assets/", gin.OnlyFilesFS{FileSystem: http.FS(assets)})
```

appleboy: We don't need export `onlyFilesFS`.

https://github.com/gin-gonic/examples/blob/0ebcc73e0c647a273847ee97519ee1dcc3dfb944/assets-in-binary/example02/main.go#L11-L20

```go
//go:embed assets/* templates/*
var f embed.FS

func main() {
	router := gin.Default()
	templ := template.Must(template.New("").ParseFS(f, "templates/*.tmpl", "templates/foo/*.tmpl"))
	router.SetHTMLTemplate(templ)

	// example: /public/assets/images/example.png
	router.StaticFS("/public", http.FS(f))
```

Please revert the export and keep testing the code block.
joeig: ```go
router.StaticFS("/public", http.FS(f))
```

The problem with the example is that it allows directory listing. That's not an option for my use case, so I need `OnlyFilesFS` to avoid that.

Fixed the gci finding.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/gin-gonic/gin/pull/3939?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 99.18%. Comparing base [(`3dc1cd6`)](https://app.codecov.io/gh/gin-gonic/gin/commit/3dc1cd6572b4e3a0cd170a15debe546c2c72294f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) to head [(`8b18c09`)](https://app.codecov.io/gh/gin-gonic/gin/pull/3939?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).
> Report is 48 commits behind head on master.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #3939      +/-   ##
==========================================
- Coverage   99.21%   99.18%   -0.03%     
==========================================
  Files          42       43       +1     
  Lines        3182     2714     -468     
==========================================
- Hits         3157     2692     -465     
+ Misses         17       12       -5     
- Partials        8       10       +2     
```

| [Flag](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | Coverage Δ | |
|---|---|---|
| [](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `?` | |
| [-tags "sonic avx"](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (?)` | |
| [-tags go_json](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (?)` | |
| [-tags nomsgpack](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.17% <100.00%> (?)` | |
| [go-1.18](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `?` | |
| [go-1.19](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `?` | |
| [go-1.20](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (-0.03%)` | :arrow_down: |
| [go-1.21](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (-0.03%)` | :arrow_down: |
| [go-1.22](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (?)` | |
| [macos-latest](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (-0.03%)` | :arrow_down: |
| [ubuntu-latest](https://app.codecov.io/gh/gin-gonic/gin/pull/3939/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | `99.18% <100.00%> (-0.03%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/gin-gonic/gin/pull/3939?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).


Patch:
diff --git a/fs.go b/fs.go
index f17d743..51c3db8 100644
--- a/fs.go
+++ b/fs.go
@@ -9,37 +9,43 @@ import (
 	"os"
 )
 
-type onlyFilesFS struct {
-	fs http.FileSystem
+// OnlyFilesFS implements an http.FileSystem without `Readdir` functionality.
+type OnlyFilesFS struct {
+	FileSystem http.FileSystem
 }
 
-type neuteredReaddirFile struct {
+// Open passes `Open` to the upstream implementation without `Readdir` functionality.
+func (o OnlyFilesFS) Open(name string) (http.File, error) {
+	f, err := o.FileSystem.Open(name)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return neutralizedReaddirFile{f}, nil
+}
+
+// neutralizedReaddirFile wraps http.File with a specific implementation of `Readdir`.
+type neutralizedReaddirFile struct {
 	http.File
 }
 
-// Dir returns a http.FileSystem that can be used by http.FileServer(). It is used internally
-// in router.Static().
-// if listDirectory == true, then it works the same as http.Dir() otherwise it returns
-// a filesystem that prevents http.FileServer() to list the directory files.
+// Readdir overrides the http.File default implementation and always returns nil.
+func (n neutralizedReaddirFile) Readdir(_ int) ([]os.FileInfo, error) {
+	// this disables directory listing
+	return nil, nil
+}
+
+// Dir returns an http.FileSystem that can be used by http.FileServer().
+// It is used internally in router.Static().
+// if listDirectory == true, then it works the same as http.Dir(),
+// otherwise it returns a filesystem that prevents http.FileServer() to list the directory files.
 func Dir(root string, listDirectory bool) http.FileSystem {
 	fs := http.Dir(root)
+
 	if listDirectory {
 		return fs
 	}
-	return &onlyFilesFS{fs}
-}
-
-// Open conforms to http.Filesystem.
-func (fs onlyFilesFS) Open(name string) (http.File, error) {
-	f, err := fs.fs.Open(name)
-	if err != nil {
-		return nil, err
-	}
-	return neuteredReaddirFile{f}, nil
-}
 
-// Readdir overrides the http.File default implementation.
-func (f neuteredReaddirFile) Readdir(_ int) ([]os.FileInfo, error) {
-	// this disables directory listing
-	return nil, nil
+	return &OnlyFilesFS{FileSystem: fs}
 }
diff --git a/routergroup.go b/routergroup.go
index c833fe8..b2540ec 100644
--- a/routergroup.go
+++ b/routergroup.go
@@ -218,7 +218,7 @@ func (group *RouterGroup) createStaticHandler(relativePath string, fs http.FileS
 	fileServer := http.StripPrefix(absolutePath, http.FileServer(fs))
 
 	return func(c *Context) {
-		if _, noListing := fs.(*onlyFilesFS); noListing {
+		if _, noListing := fs.(*OnlyFilesFS); noListing {
 			c.Writer.WriteHeader(http.StatusNotFound)
 		}
 


Test patch:
diff --git a/fs_test.go b/fs_test.go
new file mode 100644
index 0000000..a1690cd
--- /dev/null
+++ b/fs_test.go
@@ -0,0 +1,71 @@
+package gin
+
+import (
+	"errors"
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type mockFileSystem struct {
+	open func(name string) (http.File, error)
+}
+
+func (m *mockFileSystem) Open(name string) (http.File, error) {
+	return m.open(name)
+}
+
+func TestOnlyFilesFS_Open(t *testing.T) {
+	var testFile *os.File
+	mockFS := &mockFileSystem{
+		open: func(name string) (http.File, error) {
+			return testFile, nil
+		},
+	}
+	fs := &OnlyFilesFS{FileSystem: mockFS}
+
+	file, err := fs.Open("foo")
+
+	assert.NoError(t, err)
+	assert.Equal(t, testFile, file.(neutralizedReaddirFile).File)
+}
+
+func TestOnlyFilesFS_Open_err(t *testing.T) {
+	testError := errors.New("mock")
+	mockFS := &mockFileSystem{
+		open: func(_ string) (http.File, error) {
+			return nil, testError
+		},
+	}
+	fs := &OnlyFilesFS{FileSystem: mockFS}
+
+	file, err := fs.Open("foo")
+
+	assert.ErrorIs(t, err, testError)
+	assert.Nil(t, file)
+}
+
+func Test_neuteredReaddirFile_Readdir(t *testing.T) {
+	n := neutralizedReaddirFile{}
+
+	res, err := n.Readdir(0)
+
+	assert.NoError(t, err)
+	assert.Nil(t, res)
+}
+
+func TestDir_listDirectory(t *testing.T) {
+	testRoot := "foo"
+	fs := Dir(testRoot, true)
+
+	assert.Equal(t, http.Dir(testRoot), fs)
+}
+
+func TestDir(t *testing.T) {
+	testRoot := "foo"
+	fs := Dir(testRoot, false)
+
+	assert.Equal(t, &OnlyFilesFS{FileSystem: http.Dir(testRoot)}, fs)
+}