lang/funcs: fileexists slightly better "not a file" error message

Previously we were just returning a string representation of the file mode,
which spends more characters on the irrelevant permission bits that it
does on the directory entry type, and is presented in a Unix-centric
format that likely won't be familiar to the user of a Windows system.

Instead, we'll recognize a few specific directory entry types that seem
worth mentioning by name, and then use a generic message for the rest.

The original motivation here was actually to deal with the fact that our
tests for this function were previously not portable due to the error
message leaking system-specific permission detail that are not relevant
to the test. Rather than just directly addressing that portability
problem, I took the opportunity to improve the error messages at the same
time.

However, because of that initial focus there are only actually tests here
for the directory case. A test that tries to test any of these other file
modes would not be portable and in some cases would require superuser
access, so we'll just leave those cases untested for the moment since they
weren't tested before anyway, and so we've not _lost_ any test coverage
here.
This commit is contained in:
Martin Atkins 2022-01-10 16:06:19 -08:00
parent bdc5f152d7
commit e95f29bf9d
2 changed files with 26 additions and 4 deletions

View File

@ -226,8 +226,30 @@ func MakeFileExistsFunc(baseDir string) function.Function {
return cty.True.WithMarks(pathMarks), nil return cty.True.WithMarks(pathMarks), nil
} }
return cty.False, fmt.Errorf("%s is not a regular file, but %q", // The Go stat API only provides convenient access to whether it's
redactIfSensitive(path, pathMarks), fi.Mode().String()) // a directory or not, so we need to do some bit fiddling to
// recognize other irregular file types.
filename := redactIfSensitive(path, pathMarks)
fileType := fi.Mode().Type()
switch {
case (fileType & os.ModeDir) != 0:
err = function.NewArgErrorf(1, "%s is a directory, not a file", filename)
case (fileType & os.ModeDevice) != 0:
err = function.NewArgErrorf(1, "%s is a device node, not a regular file", filename)
case (fileType & os.ModeNamedPipe) != 0:
err = function.NewArgErrorf(1, "%s is a named pipe, not a regular file", filename)
case (fileType & os.ModeSocket) != 0:
err = function.NewArgErrorf(1, "%s is a unix domain socket, not a regular file", filename)
default:
// If it's not a type we recognize then we'll just return a
// generic error message. This should be very rare.
err = function.NewArgErrorf(1, "%s is not a regular file", filename)
// Note: os.ModeSymlink should be impossible because we used
// os.Stat above, not os.Lstat.
}
return cty.False, err
}, },
}) })
} }

View File

@ -228,12 +228,12 @@ func TestFileExists(t *testing.T) {
{ {
cty.StringVal(""), cty.StringVal(""),
cty.BoolVal(false), cty.BoolVal(false),
`"." is not a regular file, but "drwxr-xr-x"`, `"." is a directory, not a file`,
}, },
{ {
cty.StringVal("testdata").Mark(marks.Sensitive), cty.StringVal("testdata").Mark(marks.Sensitive),
cty.BoolVal(false), cty.BoolVal(false),
`(sensitive value) is not a regular file, but "drwxr-xr-x"`, `(sensitive value) is a directory, not a file`,
}, },
{ {
cty.StringVal("testdata/missing"), cty.StringVal("testdata/missing"),