search

jprichardson / node-fs-extra

Node.js: extra methods for the fs object like copy(), remove(), mkdirs()
MIT License
9.47k stars 772 forks source link

Error "Source and destination must not be the same" in move-sync on unicode normalization difference #859

Open Offirmo opened 3 years ago

Offirmo commented 3 years ago

First I'd like to thank you for fs-extra which is really great!

I'm fighting with an error when trying to move files: "Source and destination must not be the same."

I filtered the src and destination with === and they are NOT the same -> so why is fs-extra rejecting me?

Digging deeper, they look the same. When comparing their normalized unicode representation, they are indeed the same. So I'm suspecting (but don't really know) that fs-extra is doing a unicode normalization before validating src vs dest. (maybe Object.checkPathsSync (xxx/node_modules/fs-extra/lib/util/stat.js:51:11) [EDIT] the file system is doing unicode normalization.

If true, I believe this is incorrect. There are discussions for ex. in https://news.ycombinator.com/item?id=13953800 that states that for ex. APFS is treating pathes are "bags of bytes" and intentionally not normalizing (cf. linked https://mjtsai.com/blog/2017/03/24/apfss-bag-of-bytes-filenames/)

Hence I believe that fs-extra should not normalize. (not an expert, happy to be explained wrong)

somehow related to https://github.com/jprichardson/node-fs-extra/issues/759

Offirmo commented 3 years ago

Update: my FS is APFS, encrypted, NOT case-sensitive

Offirmo commented 3 years ago

Digging into the code, it seems the check made by fs-extra is an inode check, which would imply that APFS is doing unicode normalization… Hence my analysis is false.

Keeping the thread open in case some wisdom/advice is to be shared by the owners, but feel free to close this issue.

RyanZim commented 3 years ago

My apologies for the radio silence here; I've been super-busy, and put open-source on hold for a few days. Glad you managed to figure it out; filesystems in general tend to do weird things, which is why we abandoned comparing actual paths for inode checks.

Offirmo commented 3 years ago

@RyanZim no worries, your lib is awesome 👍

Offirmo commented 3 years ago

Reopening this issue after reading https://github.com/jprichardson/node-fs-extra/issues/759

Could we fix this unicode normalization issue in the same way than case sensitivity ? Renaming a file into a different unicode normalization should work as well!

RyanZim commented 3 years ago

Attn @manidlou

manidlou commented 3 years ago

@Offirmo can you please give us a reproducible test case?

Offirmo commented 3 years ago

@manidlou sure. On a file system doing automatic unicode normalization:

const path = `voyage à Paris.jpg`.normalize('NFD') // user input = force normalization to an exotic one for demo purpose
const target_path = path.normalize('NFC') // systematic normalization of user input
assert(a !== b) // same visual appearance but not same binary reprensentation
fs_extra.moveSync(path, target_path) // error "same inode"

This is not a cosmetic fix, I read in a blog post recently someone having trouble storing then retrieving their file on S3 due to this kind of normalization being done by safari on file upload, then the database storing a different representation.

interesting reads: