[Issue] Some applications cannot use RSAF provider as datasource

[CrazyManLabs]

Summary

Generic usage of RSAF via built-in application Files (com.google.android.documentsui) works without any problem (contents can be seen/loaded/saved).

Some specific 3rd-party applications (example - X-plore file manager (com.lonelycatgames.Xplore)) are silently failing (no errors are being shown + no contents are visible).

Issue is reproducible only for RSAF, other SAF providers (example - Termux (com.termux)) are working fine.

Steps to reproduce

• install RSAF, configure remote (in example - SMB share named share):

  Screenshot of configured RSAF remote

  

• install X-plore, link RSAF remote storage (via application menu -> More -> Link storage):

  Screenshot of path to storage linking menu action

  

• try to open newly-linked storage (by simply tapping on it), see no error and no contents:

  Screenshot of empty linked RSAF remote storage

  

Proof of RSAF-specific issue

As additional testing step I've linked Termux SAF storage (the only other SAF provider I have) to X-plore and was able to use it without any problem:

Screenshot of non-empty Termux storage

Details

Device: OnePlus 8T KB2000
ROM: OxygenOS 11 (Android 11 with GSF)
RSAF: 1.18 (from latest release artifact / built from latest sources)
X-plore: 4.34.07 (free; GPlay)
Termux: 0.118.0 (F-Droid)

While trying to open linked remote storage in X-plore this error appears in logcat:

Logcat error message

ContentProviderNative   E  onTransact error from {P:27981;U:10259}
DatabaseUtils           E  Writing exception to parcel
                           java.lang.SecurityException: Document share:/ is not a descendant of share:
                            	at android.provider.DocumentsProvider.enforceTree(DocumentsProvider.java:229)
                            	at android.provider.DocumentsProvider.query(DocumentsProvider.java:916)
                            	at android.content.ContentProvider$Transport.query(ContentProvider.java:284)
                            	at android.content.ContentProviderNative.onTransact(ContentProviderNative.java:120)
                            	at android.os.Binder.execTransactInternal(Binder.java:1165)
                            	at android.os.Binder.execTransact(Binder.java:1134)

This error looks quite odd to me, as share:/ is definitely a descendant of share:.

By looking at source of android.provider.DocumentsProvider it seems, that this exception can be thrown in case, when DocumentsProvider::isChildDocument() returns false.

By looking at source of com.chiller3.rsaf.RcloneProvider it seems, that method isChildDocument() will return false in cases, when "directory-like" path/URI (anything with trailing slash symbol; examples: share:/ , share:/some-folder/) will be requested by SAF consumer (in this case splitName will be empty, which will cause force stop of loop and false result):

RSAF/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt

Lines 419 to 433 in 04e22c3

	var currentDoc = documentId
	while (true) {
	val (splitParent, splitName) = splitPath(currentDoc)
	if (splitName.isEmpty()) {
	// Root of the remote
	break
	} else if (splitParent == parentDocumentId) {
	return true
	}
	currentDoc = splitParent
	}
	return false

By debugging this seems to be the case for X-plore: for some reason it requests share:/some-folder/ first, then it also requests share:/some-folder.

As this issue is reproducible with RSAF provider, but not reproducible with Termux provider => I assume, that this is an edge case in RSAF and it should be fixed in RSAF and not in X-plore.

Solutions

I've looked at code and it seems, that code should "normalize" documentId passed by SAF consumer before processing it (if this will not be done - processing loop will work with "incosistent" data: first it'll process path with trailing slash, on next iterations it'll process path without trailing slash, bacause splitPath() will trim this symbol and resulting value will be stored in currentDoc).

Additionally conditions inside loop should be swapped (so case share:/ will work correctly; I see comments in code, referencing to documented behavior (method should return true only for child paths, not for the same paths), but it seems like some SAF consumers are bound to "undocumented" behavior of isChildDocument() returning true, if both parentDocumentId and documentId refer to the same path (like AOSP's FileSystemProvider does)).

Diff with implemented changes

Index: app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt b/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt
--- a/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt	(revision 04e22c346d3393514dcc9c535dc4d8ce6dfff85b)
+++ b/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt	(date 1703200552565)
@@ -416,15 +416,20 @@
         // [1] https://cs.android.com/android/platform/superproject/+/android-13.0.0_r49:frameworks/base/core/java/com/android/internal/content/FileSystemProvider.java;l=141
         // [2] https://cs.android.com/android/platform/superproject/+/android-13.0.0_r49:frameworks/base/core/java/android/os/FileUtils.java;l=917
 
-        var currentDoc = documentId
+        // NOTE:
+        //   documentId can be passed with trailing slash sometimes (when directory is requested),
+        //   which breaks next logic, so normalize it first
+        //   (get document parent, as everything works with parents)
+        val (documentParent, _) = splitPath(documentId)
+        var currentDoc = documentParent
 
         while (true) {
             val (splitParent, splitName) = splitPath(currentDoc)
-            if (splitName.isEmpty()) {
+            if (splitParent == parentDocumentId) {
+                return true
+            } else if (splitName.isEmpty()) {
                 // Root of the remote
                 break
-            } else if (splitParent == parentDocumentId) {
-                return true
             }
 
             currentDoc = splitParent

---

Also I've looked at Termux implementation and they are doing this check way simpler.

Probably, RSAF can also remove all this complexity altogether and use simple "starts with" check instead.

Diff with implemented changes

Index: app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt b/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt
--- a/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt	(revision 04e22c346d3393514dcc9c535dc4d8ce6dfff85b)
+++ b/app/src/main/java/com/chiller3/rsaf/RcloneProvider.kt	(date 1703200552565)
@@ -407,30 +407,7 @@
         debugLog("isChildDocument($parentDocumentId, $documentId)")
         enforceNotBlocked(parentDocumentId, documentId)
 
-        // AOSP's FileSystemProvider [1] returns true [2] if parentDocumentId and documentId refer
-        // to the same path, but this conflicts with the documented behavior of isChildDocument(),
-        // which is supposed to test "if a document is descendant (child, grandchild, etc) from the
-        // given parent". We'll go with the documented behavior instead of matching AOSP's
-        // FileSystemProvider.
-        //
-        // [1] https://cs.android.com/android/platform/superproject/+/android-13.0.0_r49:frameworks/base/core/java/com/android/internal/content/FileSystemProvider.java;l=141
-        // [2] https://cs.android.com/android/platform/superproject/+/android-13.0.0_r49:frameworks/base/core/java/android/os/FileUtils.java;l=917
-
-        var currentDoc = documentId
-
-        while (true) {
-            val (splitParent, splitName) = splitPath(currentDoc)
-            if (splitName.isEmpty()) {
-                // Root of the remote
-                break
-            } else if (splitParent == parentDocumentId) {
-                return true
-            }
-
-            currentDoc = splitParent
-        }
-
-        return false
+        return documentId.startsWith(parentDocumentId)
     }
 
     /**

---

Both provided solutions make RSAF provider work in X-plore (contents can be seen/loaded/saved), but I'm not sure, which one is better-looking/more correct.

Screenshot of non-empty RSAF remote storage after applying fix

Postscript

1. If it'll be easier - I can create PR with provided changes (please, choose one option then).

2. I'm glad that I've found RSAF, thank you for this!
   Very good solution, which I was constantly searching for years and finaly found today (sadly found some issues, so here we are 😄 ).

[chenxiaolong]

Glad you like RSAF and thanks for the very detailed report!

It seems like X-plore is manually appending things to SAF URIs instead of relying on what the provider returns in queryChildDocuments(). This isn't guaranteed to work for all providers, but it does for all the common ones. It's not really correct for X-plore to do this, but I do think making RSAF a bit more lenient is a good idea.

I ended up doing a mix of both your suggested options in #45. First, it normalizes both the parent and child document IDs and then it does a child.startsWith(parent + '/')-style check. This allows any arbitrary combination of duplicated and trailing slash to work, like:

isChildDocument("remote://dir/////", "remote:dir///nested//child///")

(I don't think the Termux implementation is quite correct. If the parent is /sdcard/foobar and the child is /sdcard/foobar.txt, isChildDocument would incorrectly return true.)