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.

Sign up for GitHub

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

Jump to bottom

Inconsistent error reporting when using ReadOnlyFs and Windows #350

Open
jochil opened this issue May 3, 2022 · 1 comment · May be fixed by #351
Open

Inconsistent error reporting when using ReadOnlyFs and Windows #350

jochil opened this issue May 3, 2022 · 1 comment · May be fixed by #351

Comments

@jochil
Copy link

jochil commented May 3, 2022 •
edited

When using the ReadOnlyFs filter, the error returned when trying to write to the FS is different on Windows than on Linux or MacOSX

How to reproduce:

package afero_win

import (
	"errors"
	"os"
	"testing"

	pkgErrors "github.com/pkg/errors"
	"github.com/spf13/afero"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestAferoPerm(t *testing.T) {
	fs := &afero.Afero{Fs: afero.NewReadOnlyFs(afero.NewMemMapFs())}

	err := fs.WriteFile("test.txt", []byte("foobar"), 0644);
	require.Error(t, err)

	assert.True(t, errors.Is(err, os.ErrPermission))
	assert.True(t, os.IsPermission(pkgErrors.Cause(err)))
	assert.True(t, pkgErrors.Is(err, os.ErrPermission))
}

while the test is running fine on Linux, on Windows it results in:

 --- FAIL: TestAferoPerm (0.00s)
    perm_test.go:22: 
        	Error Trace:	perm_test.go:22
        	Error:      	Should be true
        	Test:       	TestAferoPerm
    perm_test.go:23: 
        	Error Trace:	perm_test.go:23
        	Error:      	Should be true
        	Test:       	TestAferoPerm
    perm_test.go:24: 
        	Error Trace:	perm_test.go:24
        	Error:      	Should be true
        	Test:       	TestAferoPerm

Expected behavior

Under windows it already returns an error operation not permitted but it is not matching with os.ErrPermission
I think it would be more consistent if this behaves identical on different operating systems
jochil added a commit to jochil/afero that referenced this issue May 3, 2022
@jochil
Return os consistent error in ReadOnlyFs filter
8e474f5 
fixes spf13#350: Use `os.ErrPermission` instead of `syscall.EPerm` to make sure
that the error is consistent over different operating systems and can be
checked with `os.isPermission(err)`
@jochil jochil linked a pull request May 3, 2022 that will close this issue
Open
@jochil
Copy link
Author

jochil commented May 3, 2022

I created the linked PR, where the returned error is changed from syscall.EPERM to os.ErrPermission. I think this is more idiomatic to the std package
This code creates a permission error under windows... this might helpful to verify

  os.Chmod(file, 0400)
  err := os.WriteFile(file, []byte("test"), 0400)
  assert.True(t, errors.Is(err, os.ErrPermission))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Return os consistent error in ReadOnlyFs filter jochil/afero
1 participant
@jochil