nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.86k stars 29.72k forks source link

fs.copyFile does not actually do a full copy #30575

Open greggman opened 4 years ago

greggman commented 4 years ago

copyFile on MacOS does not correctly copy files. A simple example

Here's a .jpg downloaded with Chrome

$ ls -l -@     
total 6792
-rw-r--r--@ 1 gregg  staff  3476085 Nov 21 20:28 original.jpg
    com.apple.macl       72 
    com.apple.metadata:_kMDItemUserTags      42 
    com.apple.metadata:kMDItemWhereFroms        190 
    com.apple.quarantine         57 

copy with it cp

$ cp original.jpg copied-with-cp.jpg
$ls -l -@                          
total 13584
-rw-r--r--@ 1 gregg  staff  3476085 Nov 21 20:48 copied-with-cp.jpg
    com.apple.macl       72 
    com.apple.metadata:_kMDItemUserTags      42 
    com.apple.metadata:kMDItemWhereFroms        190 
    com.apple.quarantine         57 
-rw-r--r--@ 1 gregg  staff  3476085 Nov 21 20:28 original.jpg
    com.apple.macl       72 
    com.apple.metadata:_kMDItemUserTags      42 
    com.apple.metadata:kMDItemWhereFroms        190 
    com.apple.quarantine         57 

now copy it with fs.copyFileSync

$ node
Welcome to Node.js v13.1.0.
Type ".help" for more information.
> const fs = require('fs')
undefined
> fs.copyFileSync('original.jpg', 'copied-with-fs-copyFile.jpg');
undefined
> 
$ ls -l -@
total 20376
-rw-r--r--@ 1 gregg  staff  3476085 Nov 21 20:48 copied-with-cp.jpg
    com.apple.macl       72 
    com.apple.metadata:_kMDItemUserTags      42 
    com.apple.metadata:kMDItemWhereFroms        190 
    com.apple.quarantine         57 
-rw-r--r--  1 gregg  staff  3476085 Nov 21 20:49 copied-with-fs-copyFile.jpg
-rw-r--r--@ 1 gregg  staff  3476085 Nov 21 20:28 original.jpg
    com.apple.macl       72 
    com.apple.metadata:_kMDItemUserTags      42 
    com.apple.metadata:kMDItemWhereFroms        190 
    com.apple.quarantine         57 

notice all the metadata is missing.

I believe like windows calls an OS level copy function in MacOS copyItem needs to be called.

greggman commented 4 years ago

Tested the windows code. Because it's calling into the OS to do the copy it does copy the entire file including meta data. Example:

C:\Users\gregg\temp\test> powershell
Windows PowerShell                                                                                                           
Copyright (C) Microsoft Corporation. All rights reserved.                                                                    

Try the new cross-platform PowerShell https://aka.ms/pscore6                                                                 

PS C:\Users\gregg\temp\test> echo > foo.txt "hello world"                                                                    
PS C:\Users\gregg\temp\test> Set-Content -Path foo.txt -Stream FooBar                                                        

cmdlet Set-Content at command pipeline position 1                                                                            
Supply values for the following parameters:                                                                                  
Value[0]: node should copy this too                                                                                          
Value[1]:                                                                                                                    
PS C:\Users\gregg\temp\test> Get-Content -Path foo.txt -Stream FooBar                                                        
node should copy this too                                                                                                    
PS C:\Users\gregg\temp\test> node                                                                                            
Welcome to Node.js v12.10.0.                                                                                                 
Type ".help" for more information.                                                                                           
> require('fs').copyFileSync('foo.txt', 'bar.txt')                                                                           
undefined                                                                                                                    
> ^d                                                                                                                           
PS C:\Users\gregg\temp\test> Get-Content -Path bar.txt -Stream FooBar                                                        
node should copy this too                                                                                                    
hatton commented 4 years ago

@greggman Did you find a work around?

RedYetiDev commented 4 months ago

Is this issue still occuring in the latest of LTS version of Node.js?


CC @nodejs/platform-macos

greggman commented 4 months ago

Yes, it's still an issue. Here's a test

test.js

import fs from 'fs';
import { spawnSync } from 'child_process';
import assert from 'assert';

function assertStringsEqual(a, b) {
  assert(a === b, `expected: '${a}' === '${b}'`);
}

function run(cmd, args) {
  const r = spawnSync(cmd, args, { encoding: 'utf8'});
  assert(!r.status, `${r.status}: ${r.stdout}${r.stderr}`);
  return r;
}

// create test.txt
fs.writeFileSync('test.txt', 'hello');

// give test.txt an xattribute
let r = run('xattr', [
  '-w', 'foo', 'bar', 'test.txt',
]);

// copy test.txt to dup.txt
r = run('cp', [
  'test.txt', 'dup.txt',
]);

// list the attributes on both test.txt and dup.txt
let r1 = run('xattr', ['-l', 'test.txt']);
let r2 = run('xattr', ['-l', 'dup.txt']);

// compare - they're the same.
const s1 = r1.stdout;
const s2 = r2.stdout.replaceAll('dup', 'test');
assertStringsEqual(s1, s2);

// use fs.copyFile to copy test.txt to fscopy.txt
fs.copyFileSync('test.txt', 'fscopy.txt');

// list the attributes of fscopy.txt
const r3 = run('xattr', ['-l', 'fscopy.txt']);

// compare - they are not the same but should be
const s3 = r3.stdout.replaceAll('fscopy', 'test');
console.log('s1:', JSON.stringify(s1));
console.log('s2:', JSON.stringify(s2));
console.log('s3:', JSON.stringify(s3));
assertStringsEqual(s1, s3);

pacakge.json

{
  "name": "test-fs-copyfile",
  "version": "1.0.0",
  "main": "test.js",
  "type": "module",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "description": ""
}

run with node test.js

results

❯ node test.js
s1: "com.apple.provenance: \u0001\nfoo: bar\n"
s2: "com.apple.provenance: \u0001\nfoo: bar\n"
s3: "com.apple.provenance: \u0001\n"
node:internal/modules/run_main:129
    triggerUncaughtException(
    ^

AssertionError [ERR_ASSERTION]: expected: 'com.apple.provenance:
foo: bar
' === 'com.apple.provenance:
'
    at assertStringsEqual (file:///Users/gregg/src/test-fs-copyfile/test.js:6:3)
    at file:///Users/gregg/src/test-fs-copyfile/test.js:48:1
    at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Node.js v20.15.0