Rolodex expects LocalFS to Implement Open With Support for Absolute Paths

[jkerfs]

While attempting to use this fantastic package with a custom LocalFS (namely the embed.FS for files embedded in golang source code), I ran across some issues.

The issues all seem to stem from the fact that Rolodex is calling the Open method on LocalFS (which has typefs.FS) with an absolute path.

But the definition of the fs.FS interface dictates that absolute paths should be rejected.

Here is the documentation for FS interface:

type FS interface {
	// Open opens the named file.
	//
	// When Open returns an error, it should be of type *PathError
	// with the Op field set to "open", the Path field set to name,
	// and the Err field describing the problem.
	//
	// Open should reject attempts to open names that do not satisfy
	// ValidPath(name), returning a *PathError with Err set to
	// ErrInvalid or ErrNotExist.
	Open(name [string](https://pkg.go.dev/builtin#string)) ([File](https://pkg.go.dev/io/fs#File), [error](https://pkg.go.dev/builtin#error))
}

And here is the documentation for ValidPath(name):

Path names passed to open are UTF-8-encoded, unrooted, slash-separated sequences of path elements, like “x/y/z”. Path names must not contain an element that is “.” or “..” or the empty string, except for the special case that the root directory is named “.”. Paths must not start or end with a slash: “/x” and “x/” are invalid.

Note the unrooted requirement.

In some places like below it appears that both the relative and absolute paths are attempted, but in practice the location variable seems to already have been made absolute in most scenaiors before it even gets to this point:

https://github.com/pb33f/libopenapi/blob/main/index/rolodex.go#L528-L541

			// check if this is a URL or an abs/rel reference.
			if !filepath.IsAbs(location) {
				fileLookup, _ = filepath.Abs(filepath.Join(k, location))
			}

			f, err := v.Open(fileLookup)
			if err != nil {
				// try a lookup that is not absolute, but relative
				f, err = v.Open(location)
				if err != nil {
					errorStack = append(errorStack, err)
					continue
				}
			}

The entrypoint that I'm using is libopenapi.NewDocumentWithConfiguration.

I have now tried to get embed.FS, fstest.MapFS, and afero.NewIOFS to work without success (all running into the same issue that the Open method is being called with absolute paths).

Suggestion / TLDR

At this point, I think it would be a breaking change to alter the use of LocalFS to adhere fully to the specification of fs.FS.

Therefore I think we have two options:

1. just add a small documentation note on LocalFS to indicate that unlike fs.FS where Open must reject absolute paths, here we require that Open supports absolute paths.
2. we can make a backwards-compatible change to Rolodex so that we do attempt calling Open with relative paths instead of only trying absolute paths.

I'm happy to put up a PR for 1 or 2 if you agree. Thanks for the amazing package. It's been a pleasure to work with.

[daveshanley]

Thank you for this! I appreciate it. I guess I never read all the docs fully... whoops!

I am happy to see a backwards compatible change going in, it it satisfies the current use cases, as well as making it compliant with the standard lib.

As long as it doesn't break anything, I am happy to see it grow and more than happy to see you contribute.

Thanks.