Skip to content

mem: return error when reading a directory (fixes #430)#597

Open
nghiack7 wants to merge 1 commit into
spf13:masterfrom
nghiack7:fix/memmap-read-eisdir
Open

mem: return error when reading a directory (fixes #430)#597
nghiack7 wants to merge 1 commit into
spf13:masterfrom
nghiack7:fix/memmap-read-eisdir

Conversation

@nghiack7

@nghiack7 nghiack7 commented May 9, 2026

Copy link
Copy Markdown

Summary

afero.ReadFile(fs, dirPath) on a MemMapFs silently returned empty content with a nil error. The real OS filesystem returns an error (read /some/dir: is a directory). This inconsistency was reported in #430.

Root cause

(*File).Read delegates to ReadAt, which had no guard for directory-backed files. It would simply return n=0, err=nil because the file data slice is empty for directories.

Fix

Added a directory check at the top of ReadAt:

if f.fileData.dir {
    return 0, &os.PathError{Op: "read", Path: f.fileData.name, Err: errors.New("is a directory")}
}

Read already delegates to ReadAt, so both are covered by one guard.

Test

Added TestReadFileOnDirectory in util_test.go that:

  1. Creates a MemMapFs directory
  2. Calls afero.ReadFile on it
  3. Asserts a non-nil error is returned
$ go test ./... -run TestReadFile
ok      github.com/spf13/afero  0.6s

Full test suite passes without regressions.

Fixes #430

@CLAassistant

CLAassistant commented May 9, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

ReadAt on a MemMapFs file backed by a directory now returns
os.PathError{Err: "is a directory"}, matching the behaviour of
the real OS filesystem.  Previously it silently returned 0 bytes
and no error, causing afero.ReadFile(fs, dirPath) to succeed and
return empty content.

Fixes spf13#430
@nghiack7 nghiack7 force-pushed the fix/memmap-read-eisdir branch from 00da920 to 81d5b57 Compare May 11, 2026 02:15
@nghiack7

Copy link
Copy Markdown
Author

@CLAassistant recheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

memfs directory can be read by ReadFile

2 participants