openzim / libzim

Reference implementation of the ZIM specification
https://download.openzim.org/release/libzim/
GNU General Public License v2.0
163 stars 47 forks source link

Allow extern `getFd()` to get file descriptors from Java for Kiwix Android #852

Open MohitMaliFtechiz opened 6 months ago

MohitMaliFtechiz commented 6 months ago

We have an issue in Android directly accessing the fd via file path on Android 11 and above without MANAGE_EXTERNAL_PERMISSION(Which is special permission in Android and Play Store does not allow us to take this permission).

Archive and SuggestionSearcher need the file path to work properly with the Xapian index as per conversation with @mgautierfr in https://github.com/kiwix/kiwix-android/pull/3636. See https://github.com/kiwix/kiwix-android/issues/3636#issuecomment-1876742791 for more details.

So to solve this issue can we open a new fd from the existing fd via fileInputStream API and use it for Xapian without affecting the existing one? Somehow this is possible at the libzim level. Or if you think of any better suggestion that can achieve this with existing fd would be welcome.

kelson42 commented 4 months ago

@mgautierfr Any feedback here?

kelson42 commented 4 months ago

We have an issue in Android directly accessing the fd via file path on Android 11 and above without MANAGE_EXTERNAL_PERMISSION(Which is special permission in Android and Play Store does not allow us to take this permission).

Phrasing seems incorrect. Correct phrasing is: Issues on Android to fully (suggestions/ft search) use ZIM files when open via fd only (so without a - ZIM - file path). In such a case Xapian indexes can not be open (as they do require a dedicated fd, which can only be properly retrieved in "Java world" from the ZIM file path).

Archive and SuggestionSearcher need the file path to work properly with the Xapian index as per conversation with @mgautierfr in kiwix/kiwix-android#3636. See kiwix/kiwix-android#3636 (comment) for more details.

So to solve this issue can we open a new fd from the existing fd via fileInputStream API and use it for Xapian without affecting the existing one? Somehow this is possible at the libzim level. Or if you think of any better suggestion that can achieve this with existing fd would be welcome.

I answer here under the control of @mgautierfr, but "yes" you can open many fd from the same file. Each fileInputStream will have its own fd, but you can open many fileInputStream for the same file and then you should have many fds.

So. here I see only one solution: the libzim should be modified to allow an "external" getFD() method which would be able to call java code to get one. This is a feature request which implies modifications here, in kiwix/java-libkiwix and kiwix/kiwix-android.

mgautierfr commented 2 months ago

This comment https://github.com/kiwix/libkiwix/issues/1015#issuecomment-1976498068 is interesting and must not be forgotten, and here an answer:

Opening an archive with a list of (file-path, offset, size) would be interesting/needed in itself but not enough. We may have system (android, flatpak,...) where we cannot have full access to filesystem. In those cases, file is opened by an external tool and we only get a fd.

veloman-yunkan commented 2 months ago

This comment kiwix/libkiwix#1015 (comment) is interesting and must not be forgotten, and here an answer:

Opening an archive with a list of (file-path, offset, size) would be interesting/needed in itself but not enough. We may have system (android, flatpak,...) where we cannot have full access to filesystem. In those cases, file is opened by an external tool and we only get a fd.

@mgautierfr As I noted in the said comment, we convert a file descriptor to a file path (of the form /dev/fd/XXX) and actually open the file by path. What I had proposed was to move that trick out of libzim - let the users do that conversion themselves (libzim may simply provide a utility) and the public API will only accept paths.

mgautierfr commented 2 months ago

The problem is that the trick may be not possible at all. Moving it to the user side move the problem outside of libzim but the root issue is still there: We cannot assume we can open by path (even /dev/fd/xxx)