Fix logic that detects when a file is outside the base path so it works well with Windows-style paths. Also fixed an edge case with paths that start with ".." but are not external like "..file.js".
What changes did you make? (Give an overview)
Windows paths have some unique features not shared by Unix paths. For one, absolute local paths start with a drive letter followed by a colon like "C:". The fact that the drive letter must be the first character in a local path means that a relative path cannot specify a drive. In Node.js, path.relative() will just return the absolute path of the second argument when it cannot calculate a relative path across different drives, so for example with path.relative("C:\\dir", "D:\\file.js").
Besides local paths, Windows uses UNC paths in the form \\Server\Volume\dir1\dir2\file to specify files on a network. Those paths are always absolute.
Both config-array and ESLint use relative paths to check if a file is inside a base directory: if the file's relative path from the directory starts with "..", then the file is not in the directory. This check is currently failing when the base directory and the file were located on different drives or servers, because as previously mentioned path.relative() doesn't handle those cases.
The fix I've come up with consists in calculating what Node.js calls namespace-prefixed paths of the base directory and the file and calling path.relative() on those two. Namespaced paths are prefixed with \\?\ for local paths or with \\?\UNC for network paths. When the provided arguments are namespaced paths on different drives, path.relative() will return a path in the form ..\..\C:\dir1\dir2\file.js. While such a path is not valid as it contains a drive letter in middle position, it can be readily recognized as for being outside the base path because of the leading .. and discarded as external.
Is there anything you'd like reviewers to focus on?
Instead of using toNamespacedPath, we could manually parse the absolute path, normalize the drive letter or server specification and compare the path components to the corresponding values of the base path. This would make the check more explicit and possibly easier to maintain. The downside is that we would end up reinventing (part of) the wheel.
Prerequisites checklist
What is the purpose of this pull request?
Fix logic that detects when a file is outside the base path so it works well with Windows-style paths. Also fixed an edge case with paths that start with ".." but are not external like "..file.js".
What changes did you make? (Give an overview)
Windows paths have some unique features not shared by Unix paths. For one, absolute local paths start with a drive letter followed by a colon like "C:". The fact that the drive letter must be the first character in a local path means that a relative path cannot specify a drive. In Node.js,
path.relative()will just return the absolute path of the second argument when it cannot calculate a relative path across different drives, so for example withpath.relative("C:\\dir", "D:\\file.js").Besides local paths, Windows uses UNC paths in the form
\\Server\Volume\dir1\dir2\fileto specify files on a network. Those paths are always absolute.Both
config-arrayand ESLint use relative paths to check if a file is inside a base directory: if the file's relative path from the directory starts with "..", then the file is not in the directory. This check is currently failing when the base directory and the file were located on different drives or servers, because as previously mentionedpath.relative()doesn't handle those cases.The fix I've come up with consists in calculating what Node.js calls namespace-prefixed paths of the base directory and the file and calling
path.relative()on those two. Namespaced paths are prefixed with\\?\for local paths or with\\?\UNCfor network paths. When the provided arguments are namespaced paths on different drives,path.relative()will return a path in the form..\..\C:\dir1\dir2\file.js. While such a path is not valid as it contains a drive letter in middle position, it can be readily recognized as for being outside the base path because of the leading..and discarded as external.Related Issues
refs https://github.com/eslint/eslint/issues/18575
Is there anything you'd like reviewers to focus on?
Instead of using
toNamespacedPath, we could manually parse the absolute path, normalize the drive letter or server specification and compare the path components to the corresponding values of the base path. This would make the check more explicit and possibly easier to maintain. The downside is that we would end up reinventing (part of) the wheel.