birdofpreyru / react-native-fs

File system access for React Native
https://dr.pogodin.studio/docs/react-native-file-system
Other
160 stars 12 forks source link

iOS: Implement security scope access (bookmark) for read and readDir #51

Open zenoxs opened 3 months ago

zenoxs commented 3 months ago

Hey, First of all thank you for taking over the development of the fs lib.

I'm trying to access shared directories from my app on iOS, it should be possible using the new security scoped URL with bookmark: https://developer.apple.com/documentation/uikit/view_controllers/providing_access_to_directories.

I can submit a PR to implement this feature but I would like to know which approach do you prefer? Would it make sense to create "readDirBookmark" and "readBookmark" functions only available for iOS? then passing the base64 encoded iOS bookmark (this would allow the implementation to work easily with https://react-native-documents.github.io/docs/doc-picker-api#bookmarkingresponse)

birdofpreyru commented 3 months ago

Hi @zenoxs , thanks for your interest!

I am strongly against specific-OS-only functions in RN libraries. This one has a bunch of them, but that is the legacy, I want to get rid of them all over time (though, it might take a while).

Re. bookmarks, I've added their support to pickFile(), copyFile(), and exists() methods some time ago (see https://github.com/birdofpreyru/react-native-fs/issues/28, and the commit https://github.com/birdofpreyru/react-native-fs/commit/2437f8e72008e0c12bbd8435c74beabf25c64f9a, in particular). I might not remember the exact details now, but I believe in TS layer bookmark URLs are handled just as regular URLs passed to the existing functions, and having the format bookmark://<BASE64_ENCODED_SCOPED_URL_DATA>, and on the native side, in iOS code, there is already pathToUrl() method that, if passed such string, correctly restores the original bookmark URL, which can be used to access scoped resources (and if passed a regular URL, it also handles it appropriately). So, essentially to support that in other functions, you should just ensure they pass URL they got through that pathToUrl() method, and may be a few other minor checks / code changes on top of that — just replicate what I did in that change I referred to. Unless I am missing anything here, then let's discuss if further.

zenoxs commented 3 months ago

Hi @zenoxs , thanks for your interest!

I am strongly against specific-OS-only functions in RN libraries. This one has a bunch of them, but that is the legacy, I want to get rid of them all over time (though, it might take a while).

Re. bookmarks, I've added their support to pickFile(), copyFile(), and exists() methods some time ago (see #28, and the commit 2437f8e, in particular). I might not remember the exact details now, but I believe in TS layer bookmark URLs are handled just as regular URLs passed to the existing functions, and having the format bookmark://<BASE64_ENCODED_SCOPED_URL_DATA>, and on the native side, in iOS code, there is already pathToUrl() method that, if passed such string, correctly restores the original bookmark URL, which can be used to access scoped resources (and if passed a regular URL, it also handles it appropriately). So, essentially to support that in other functions, you should just ensure they pass URL they got through that pathToUrl() method, and may be a few other minor checks / code changes on top of that — just replicate what I did in that change I referred to. Unless I am missing anything here, then let's discuss if further.

Thanks for the quick reply and for all the details, that's exactly what I was expecting. I will try to replicate what you did for pickFile(), copyFile(), and exists() for the read and readDir with the bookmark scheme in the url. 👍

birdofpreyru commented 3 months ago

Presumably done for readDir() (though, I have not explicitly tested it with a bookmark, just refactored the code, and checked it still works for regular URLs).