René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"fsPromises.cp(...) inconsistencies and bugs","articleBody":"The fsPromises.cp and fs.cp methods are inconsistent with the sync version because they do not correctly accept Buffer file paths. This also means it also improperly handles non-UTF8 encoded filenames. The sync variation supports Buffer.\n\nThe method also does not appropriately validate inputs. Rather than throwing a proper Node.js type error when a Buffer is passed, it tries to use it and fails at another point deeper in the function. \n\nThe impl of the async version of the method is also needlessly structured differently from the sync version leading to a fair amount of duplicated and inconsistent code, making it difficult to fix the inconsistencies. The sync version was updated recently to move significant pieces to C++ while the async versions were not similarly updated.\n\nExample:\n\n```js\nfs.cpSync(Buffer.from('a'), Buffer.from('b'));  // works!\n\nfs.cp(Buffer.from('a'), Buffer.from('b'), (err) =\u003e { /* ... */ });  // fails!\n```\n\nThe error thrown is:\n\n```\nTypeError [ERR_INVALID_ARG_TYPE]: The \"paths[0]\" argument must be of type string. Received an instance of Buffer\n    at resolve (node:path:1195:7)\n    at normalizePathToArray (node:internal/fs/cp/cp:185:45)\n    at isSrcSubdir (node:internal/fs/cp/cp:190:18)\n    at checkPaths (node:internal/fs/cp/cp:110:32)\n    at async cpFn (node:internal/fs/cp/cp:66:17)\n```\n\n`fs.cp(...)` is currently just a `callbackified(...)` version of `fsPromise.cp(...)`, so fixing one should fix the other:\n\nThere are several fixes necessary: If `Buffer` is not going to be accepted, then the method should properly validate that the input `src` and `dest` are not Buffers and throw a proper `ERR_INVALID_ARG_TYPE` error. However, not accepting `Buffer` mean that the async versions of these will not properly handle non-UTF8 encoded file names. Ideally, the method would be updated to accept `Buffer` paths, but that means the `normalizePathToArray` method needs to be updated/refactored.\n\nSecond, the differences that were introduced when the sync version was updated to move chunks to C++ make it rather difficult to keep these methods in sync with each other. There is a strong possibility/likelihood of inadvertently introducing behavioral differences between the two that increase the likelihood of more inconsistencies being introduced in the future. We should probably move the entire implementation of the cp callback, cp promise, and cp sync methods into C++ if we're going to have any of the implementation in C++, having those eliminate duplicated logic as much as possible.\n\nThere is another bug in the implementation of these methods with regards to the `options.filter` option, which does not correctly handle the case in cpSync when the `src` and `dest` are passed as `Buffer` and when the children of copied directories are not UTF8 encoded file names. The differences in the implementation of the sync and async versions of the cp function make it quite difficult to introduce a consistent fix across the variations. The implementations should be reconciled and moved to C++ first, *including* the case where `options.filter` is called, before we can properly fix the `options.filter` implementation in a consistent way.\n\n@nodejs/fs @dario-piotrowicz @anonrig ","author":{"url":"https://github.com/jasnell","@type":"Person","name":"jasnell"},"datePublished":"2025-06-27T20:04:32.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":3},"url":"https://github.com/58869/node/issues/58869"}