Skip to content

cp and cpSync incorrectly handle existing dest symlinks #58468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dario-piotrowicz opened this issue May 26, 2025 · 1 comment · May be fixed by #58476
Open

cp and cpSync incorrectly handle existing dest symlinks #58468

dario-piotrowicz opened this issue May 26, 2025 · 1 comment · May be fixed by #58476
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented May 26, 2025

Version

v24.1.0

Platform

Linux Laptop-16-AMD-Ryzen-7040-Series 6.8.0-60-generic #63-Ubuntu SMP PREEMPT_DYNAMIC Tue Apr 15 19:04:15 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

fs

What steps will reproduce the bug?

Run the following code:

"use strict";

const fs = require("node:fs");
const path = require("node:path");

// cleans up test and dest directories
fs.rmSync("./tmp/test", { recursive: true, force: true });
fs.rmSync("./tmp/dest", { recursive: true, force: true });

// prepares the test directory
fs.mkdirSync("./tmp/test", { recursive: true });

// in the test directory creates a source.txt file and a link.txt symlink that points to the source.txt file
fs.writeFileSync("./tmp/test/source.txt", "test content");
fs.symlinkSync(path.resolve("./tmp/test/source.txt"),"./tmp/test/link.txt");

// call cpSync
fs.cpSync("./tmp/test", "./tmp/dest", { recursive: true });
// call cpSync again (this so with dest already existing/populated)
fs.cpSync("./tmp/test", "./tmp/dest", { recursive: true });

How often does it reproduce? Is there a required condition?

the issue can always be reproduced and it does not require a specific condition

What is the expected behavior? Why is that the expected behavior?

I believe that the second cpSync call should not throw any error, but the existing symlink files should be skipped unless force is set to false and errorOnExist is true, in such case an EEXIST error should be thrown instead

What do you see instead?

The second cpSync call throws an ERR_FS_CP_EINVAL error

Additional information

  • The ERR_FS_CP_EINVAL error is thrown even when force is being used (and the dest file is not overridden).

  • This applies both to cp and cpSync

    `cp` reproduction
    "use strict";
    
    const fs = require("node:fs");
    const path = require("node:path");
    
    fs.rmSync("./tmp/test", { recursive: true, force: true });
    fs.rmSync("./tmp/dest", { recursive: true, force: true });
    
    fs.mkdirSync("./tmp/test", { recursive: true });
    
    fs.writeFileSync("./tmp/test/source.txt", "test content");
    fs.symlinkSync(path.resolve("./tmp/test/source.txt"),"./tmp/test/link.txt");
    
    fs.cp("./tmp/test", "./tmp/dest", { recursive: true }, (err) => {
        if(err) throw err;
    
        fs.cp("./tmp/test", "./tmp/dest", { recursive: true }, (err) => {
            if (err) throw err;
        });
    });
  • setting dereference to true seems to fix the issue, however I am not sure if dereferencing is actually taking place since the symlinks are copied as such and not as the files they point to (which I think is the expected behavior? 🤔)
@dario-piotrowicz dario-piotrowicz changed the title cp/cpSync incorrectly handles existing dest symlinks cp and cpSync incorrectly handle existing dest symlinks May 26, 2025
@dario-piotrowicz
Copy link
Member Author

cc. @nodejs/fs 🙂

@dario-piotrowicz dario-piotrowicz added the fs Issues and PRs related to the fs subsystem / file system. label May 26, 2025
@jakecastelli jakecastelli linked a pull request May 27, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant