Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow access to all files from SAF #3165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

babaric-dev
Copy link

Allow access everything under /data/data/com.termux/files, not just /data/data/com.termux/files/home

@termux termux deleted a comment Jan 3, 2023
@babaric-dev
Copy link
Author

Did some tests and they worked well. Please merge my PR :)

@agnostic-apollo
Copy link
Member

I don't know what was the reason to restrict to $HOME with SAF in 026d0b4 instead of termux rootfs. I don't think it should put any greater security risk to termux user data since rc files like .bashrc can be used anyways to modify files under $PREFIX. I do plan on rewriting the document provider with support for custom directories, but I guess for now this should be mergable.

@babaric-dev
Copy link
Author

Cool. Hope this PR will be merged soon.

@babaric-dev
Copy link
Author

I do plan on rewriting the document provider with support for custom directories

Add a menu for selecting the directory?

@agnostic-apollo
Copy link
Member

Would be controlled via termux.properties file to publish multiple roots if adding such support is feasible.

@babaric-dev
Copy link
Author

Would be controlled via termux.properties file

Is this even possible? Can the path be changed during runtime? Else, it will likely cause data corruptions or errors.

@babaric-dev
Copy link
Author

multiple roots

Can this be implemented? Can't find anything about that from Google (maybe I just suck at Googling)

@tareksander
Copy link
Member

multiple roots

Can this be implemented? Can't find anything about that from Google (maybe I just suck at Googling)

I made an app that exposes many roots, works just fine. The cursor you return in queryRoots can have multiple rows. You create a MatrixCursor and add one row for each root. Here's how I did it in my app.

@agnostic-apollo
Copy link
Member

Yeah, pretty much. Will also have to prefix a root id in the Uris since they may refer to overlapping roots.

@babaric-dev
Copy link
Author

@agnostic-apollo I am looking into this. But how can I add an array property in termux.properties? I cant figure out.

@babaric-dev
Copy link
Author

IMHO, I think we can remove the information for usable bytes (Root.COLUMN_AVAILABLE_BYTES). It isn't useful for me at least. It seems to just show the same information as the disk usage feature found in file managers.

@agnostic-apollo
Copy link
Member

agnostic-apollo commented Jan 30, 2023

I am looking into this. But how can I add an array property in termux.properties? I cant figure out.

Adding support for multiple roots won't just require an array, it will require a nested dict where each root has its own sub properties like title, summary, icon, whether its read-only, whether canonical paths outside it are accessible, etc. The extra-keys is using json, but for this and the future, the property key names will be scoped, like under api.doument_provider. Moreover, since more complex APIs are planned for future, its unlikely that .properties file will be used in future for termux apps, since writing nested/scoped configs is very verbose/repeated with .properties files. Moving to yaml might be a good idea, but there are issues with that too, like no in-place editing java library (like yq) if a UI is added for modifying yaml config, which would remove user comments if modified yaml is just dumped to the file. I haven't decided on final designs for all these things yet, will look into after next release, not now. Moreover, the content provider would also require a rewrite and will require fixing the current issues like revoking uri permissions for root dir and files on copy/move/delete, otherwise would preserve access to files that was given to apps previously, which they shouldn't have post-operation, resulting in security issues/vulnerabilities. Basically, lot of things would be required to properly implement this.

IMHO, I think we can remove the information for usable bytes (Root.COLUMN_AVAILABLE_BYTES). It isn't useful for me at least. It seems to just show the same information as the disk usage feature found in file managers.

What about file managers that don't show this info directly but have internal usages? Moreover, file managers don't have access to termux directories to know how much free space there actually is. It may be same as external storage or their own app data directories but termux app may be installed on a different partition or have custom mounts for specific directories, which will have different free spaces. Moreover, currently the provider is not returning free space for all directories, which ideally should be done.

@babaric-dev

This comment was marked as outdated.

@babaric-dev
Copy link
Author

New update: I already implemented the majority of these. But how to use getActivity() inside TermuxDocumentsProvider? I need to get the properties from termux.properties.

@agnostic-apollo
Copy link
Member

Why do you need an Activity instance? Loading termux.properties only requires a Context, use getContext().

https://developer.android.com/reference/android/content/ContentProvider#getContext()

Also note that I have rewitten/refactored properties and preferences of all termux apps and moved the classes to support current requirements, user requests and planned future changes, so whatever changes you are making now will have to be redone later.

@agnostic-apollo
Copy link
Member

And any suggestive changes won't be merged unless they meet the required standards with the issues solved.

@babaric-dev
Copy link
Author

So maybe I will rework my changes from scratch after you made changes and pushed them to here.

Allow access everything under `/data/data/com.termux/files`, not just `/data/data/com.termux/files/home`
@babaric-dev
Copy link
Author

I realised we could just expose /data/data/com.termux. So we could just grant app's access like by selecting subdirectories of the exposed directories. That way, we don't need multiple roots that complexity. Simple yet working.

@agnostic-apollo
Copy link
Member

That wouldn't allow root specific options like read only, allow external files access via symlinks, etc.

@zongou
Copy link

zongou commented Apr 2, 2024

Would be controlled via termux.properties file

Is this even possible? Can the path be changed during runtime? Else, it will likely cause data corruptions or errors.

Provides accessing a dir, for example,
$HOME/.termux/public
then user can make symlink to overwrite that dir, it will become dynamic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants