sindresorhus / cpy

Copy files
MIT License
425 stars 63 forks source link

Possible data-loss bug when using absolute paths #114

Open dylang opened 1 year ago

dylang commented 1 year ago

Hi @sindresorhus and @Idered -

I think there's an issue in preprocessDestinationPath caused by path.relative(entry.pattern.normalizedPath, entry.path) in https://github.com/sindresorhus/cpy/pull/77 when the cwd is an absolute path

Possible Bug

When the source and the destination are both absolute paths, the preprocessDestinationPath can (if they share a common root path perhaps?) return full path (with filename) that happens to be original source path.

When preprocessDestinationPath results in the source and destination having the same filename, cpy still tries to copy the file, calling cp-file, which stream-reads the source to a stream-write at the destination.

In a monorepo with multiple tasks running simultaneously, it's possible that a file is copied multiple times. The undesired result is that in some rare cases, like when a file is large (over 6000 lines), we end streaming a file back to itself before it is done being written. This results in a source file that is truncated. In our case, it only happened to 2 files out of thousands.

Example Code

// v9 or v10
import cpy from 'cpy';

const source = '/absolute/project/package/package-name';
const destination = '/absolute/project/build/package/package-name';

// example file
// /absolute/project/package/package-name/src/example.ts

await cpy(
  [
    '**/*', 
    '**/*.*',
    '!package.json', // We'll re-create this specifically for the docker image.
    '!**/node_modules/**',
    '!**/*.ts', // Don't need pre-compiled code, then typing will use the ts files
    '!**/*.tsx', // instead of the d.ts and all devDependencies will need to be included.
    '!**/*.test.*', // Don't need tests or related assets.
    '!**/__snapshots__/**',
    '!**/__fixtures__/**',
    '!**/__mocks__/**'
  ],
  destination,
  {
      cwd: source
  }
);

Possible Fix

I've narrowed the issue to this code: https://github.com/sindresorhus/cpy/blob/main/index.js#L90-L94

The following change to those lines happen to work for my code, but I don't know if it's the right fix for all cases:

return path.join(destination, entry.path.replace(options.cwd, ''));

History

I know the change to preprocessDestinationPath was a while ago: my build had unexpected files changed when v9 came out, so I didn't upgrade to v9. Now that v10 is out and it's still happening, I looked for the cause.