Repository navigation
Cross file-system path handling - #8227
Conversation
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
Approving with a couple of suggestions that seem important before merging.
| { | ||
| throw new FileUtil.InvalidPathReferenceException(part, x.getReason()); | ||
| } | ||
| if (name.getRoot() != null || name.getNameCount() != 1 || ".".equals(part) || "..".equals(part)) |
There was a problem hiding this comment.
Good strengthening but three callers that probably need matching updates:
- DiscvrLabKeyModules
SequenceAnalysisController.java:1076and:2234:resolveFile(new Path(form.getPath())) - snprcEHRModules
FeeScheduleController.java:176:allowedRoot.resolveFile(new Path(relativeURI.getPath())), the same pattern this PR fixed inFileSystemLike.getVerifiedFileLike
Could those switch to Path.parse(...) in companion PRs?
| assertEquals("b.txt", a.resolveFile(new Path("b.txt")).getName()); | ||
|
|
||
| // a Path part is a single file name, never a path | ||
| for (String name : new String[] {"../x", "a/../../x", "a/b", "/x"}) |
There was a problem hiding this comment.
resolve(fs) runs from vfs() as well as nio(), but only FileSystemLocal validates each part.
FileSystemVFS.resolveFile resolves "a/b" and "../x" through commons-vfs and can only throw IllegalArgumentException/UnexpectedException.
Could the single-file-name check move into shared code (e.g. AbstractFileLike.resolvePath) so FileSystemVFS and FileSystemBlobStore enforce it too? That also lets FileSystemVFS.resolveFile throw InvalidPathReferenceException too.
There was a problem hiding this comment.
I'll check on this. I focussed on FileSystemLocal because the root issue is local windows fs supporting an alternate path separator in nio.Path. Not sure this applies anywhere else.
Rationale
Fix associated issue with cross filesystem character handling.
Some scope creep refactor, cleanup, and test in FileSystem.
Related Pull Requests
Changes
\or a drive prefix on Windows) is rejected instead of silently re-split.Fileor nio path as aFileLikesplit the relative path into parts rather than passing it as one name.