node-fs-extra icon indicating copy to clipboard operation
node-fs-extra copied to clipboard

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

Open Offirmo opened this issue 4 years ago • 8 comments

  • Operating System: macOs 10.15.7 (latest Catalina)
  • Node.js version: v14.15.3
  • fs-extra version: 9.0.1

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 avatar Jan 15 '21 04:01 Offirmo

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

Offirmo avatar Jan 15 '21 05:01 Offirmo

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.

Offirmo avatar Jan 15 '21 05:01 Offirmo

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.

RyanZim avatar Jan 18 '21 18:01 RyanZim

@RyanZim no worries, your lib is awesome 👍

Offirmo avatar Jan 24 '21 06:01 Offirmo

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!

Offirmo avatar May 06 '21 18:05 Offirmo

Attn @manidlou

RyanZim avatar May 06 '21 19:05 RyanZim

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

manidlou avatar May 07 '21 05:05 manidlou

@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:

  • https://withblue.ink/2019/03/11/why-you-need-to-normalize-unicode-strings.html
  • https://www.win.tue.nl/~aeb/linux/uc/nfc_vs_nfd.html

Offirmo avatar May 07 '21 06:05 Offirmo