fs: normalize cpSync and related methods to accept Buffer and URL paths (#58634)#58683
fs: normalize cpSync and related methods to accept Buffer and URL paths (#58634)#58683vedant713 wants to merge 1 commit intonodejs:mainfrom
Conversation
This patch ensures that fs.cpSync and copyDir handle Buffer and URL types for the src and dest arguments by normalizing them to string paths using toString() and fileURLToPath() respectively. Previously, these inputs would cause type errors when passed to path.join(), as the implementation assumed string paths. Fixes: nodejs#58634
jasnell
left a comment
There was a problem hiding this comment.
Unfortunately this does not work. The reason Buffer is supported in the first place is because many file system support filenames that are not UTF8 encoded... in fact, that's the norm in many legacy system. Calling p.toString() means the bytes in the buffer are assumed to be utf8 and converted as such, which would be incorrect and would just be trading one bug for another.
The other issue is that fileURLToPath(p) will fail if there are percent encodings that are not valid UTF8, which would be the case if the file URL is identifying a path that uses non-UTF8 encodings like Shift-JIS.
I have an alternative solution that I'm working on that instead normalizes in the other direction... that is, normalizes from string or URL representations into Buffer, which means that we wouldn't be performing any interpretation of the encoding used by the file names and would instead just represent them as byte strings. Unfortunately, however, it means a necessarily breaking change to the option.filter
This patch ensures that fs.cpSync and copyDir handle Buffer and URL types for the src and dest arguments by normalizing them to string paths using toString() and fileURLToPath() respectively.
Previously, these inputs would cause type errors when passed to path.join(), as the implementation assumed string paths.
Fixes: #58634