Closed Jakeler closed 3 years ago
machiav3lli
commented
3 years ago This error means that the app is possibly running in the background changing the data while the backup process, please try advanced settings > Stop-Cont. If it doesn't work, then it's probably caused by the privilged plug-in.
Jakeler
commented
3 years ago Stop-Cont or manually killing the app doesn't help unfortunately. I haven't used F-Droid directly before when I encountered this issue the first time and it also does not happen with any other apk so far, even if I just downloaded it and switch to OABX as fast as possible. I dont't have the F-Droid Privileged Extension installed.
I noticed something else with a my file manager (Solid Explorer): it fails to delete this file, but works for other files in the app data... it throws a pl.solidexplorer.common.exceptions.SEException, but checking the SELinux context with ls -lAZ shows that it is the same for all files in the directory. A root shell with simple rm does the job though. It has to be some property of this specifc file, but I just can't figure out what exactly.
machiav3lli
commented
3 years ago Oh, then it could be a cyclic symlinking!
Jakeler
commented
3 years ago Nope, it is not a symlink. btw: It handles cyclic symlinks completly fine and stores it just as link without following. I have tried it out and I think symlinks should not cause any issues with the current implementation.
But I have now found the actual issue: It is the filename that contains two consecutive spaces after ... Foto\ Manager\ \!
It is easily reproducible, renaming the file makes it work again.
Probably an issue with filename escaping? I have also noticed in the error that it doesn't get correctly displayed, in the popup it prints the second backslash and in logcat it omits the second space completely.
hg42
commented
3 years ago yes, "double spaces" indicates it's probably a quoting issue. The command line is probably split into words and then joined with single spaces.
If the display is also wrong, this would indicate it's caused by reading from ls.
Could you please show how it is exactly displayed in the popup?
(I assume you are aware of spaces being narrower than other characters for proportional fonts, so you compare it with other spaces)
Unfortunately, I currently don't have time to update my development environment to the current version and work with that. But I might find time to have a look at the sources (online, github)...and thinking about the reasons.
btw. I also use Simple Gallery Pro ... but my system language is English (despite I'm German).
I now see, the label is Schlichte\ Galerie\ Pro\ -\ Foto\ Manager\ \ Editor while it is 'Schlichte\ Galerie\ Pro\ -\ Foto\ Manager\ &\ Editor' in the F-Droid search, which explains the double space. The '&' seems to be removed by the App developer of Simple Gallery Pro or by some F-Droid procedures.
But this doesn't matter, OAandBackupX should be able to use any valid file name.
hg42
commented
3 years ago in a quick evening session, I updated my project and wrote a test case and I found the file name is read correctly from the line you provided:
// app/src/test/java/ShellHandlerTest.kt
import com.machiav3lli.backup.handler.ShellHandler
import org.junit.Assert
import org.junit.Test
class ShellHandlerTest {
@Test
fun test_fromLsOOutput_handlesDoubleSpace() {
val fileInfo = ShellHandler.FileInfo.fromLsOOutput(
"-rw------- 1 user0_a247 group0_a247 15951095 2021-01-19 01:03:29.000000000 +0100 Schlichte\\ Galerie\\ Pro\\ -\\ Foto\\ Manager\\ \\ Editor-6.18.0.apk",
"/data/data/org.fdroid.fdroid/files"
)
Assert.assertEquals(fileInfo.filePath, "Schlichte\\ Galerie\\ Pro\\ -\\ Foto\\ Manager\\ \\ Editor-6.18.0.apk")
Assert.assertEquals(fileInfo.fileSize, 15951095)
Assert.assertEquals(fileInfo.owner, "user0_a247")
Assert.assertEquals(fileInfo.group, "group0_a247")
Assert.assertEquals(fileInfo.fileModTime.time, 1611014609000)
Assert.assertEquals(fileInfo.fileMode, 0b0_110_000_000)
}
}so currently, I don't see what's going on.
btw. I used the latest sources, so you may try the (mostly) corresponding 5.0.1-alpha1
Jakeler
commented
3 years ago Thanks for looking into it @hg42! I have now attached the debugger with a breakpoint at this function. The difference to your test case is that the real input does not contain any backslashes, they get parsed just as spaces.
It is already wrong in the token array, more precisely the splitWithoutEmptyValues function causes it. Try these tests:
@Test
fun test_fromLsOOutput_handlesDoubleSpace() {
val fileInfo = ShellHandler.FileInfo.fromLsOOutput(
"-rw------- 1 user0_a247 group0_a247 15951095 2021-01-19 01:03:29.000000000 +0100 111 333.file",
"/data/data/org.fdroid.fdroid/files"
)
Assert.assertEquals(fileInfo.filePath, "111 333.file")
Assert.assertEquals(fileInfo.fileSize, 15951095)
Assert.assertEquals(fileInfo.owner, "user0_a248")
Assert.assertEquals(fileInfo.group, "group0_a247")
Assert.assertEquals(fileInfo.fileModTime.time, 1611014609000)
Assert.assertEquals(fileInfo.fileMode, 0b0_110_000_000)
}
@Test
fun test_splitWithoutEmptyValues_tripleSpace() {
val tokens = ShellHandler.splitWithoutEmptyValues(
"-rw------- 1 user0_a247 group0_a247 15951095 2021-01-19 01:03:29.000000000 +0100 111 333.file",
" ",8)
val expected = arrayOf(
"-rw-------",
"1",
"user0_a247",
"group0_a247",
"15951095",
"2021-01-19",
"01:03:29.000000000",
"+0100",
"111 333.file",
)
Assert.assertArrayEquals(expected, tokens)
}Note that I have prepared a file with 3 spaces, but there could be even more spaces, everything is reduced to one...
On adb shell this shows up as 111\ \ \ 333.file
The tests fail both:
org.junit.ComparisonFailure:
Expected :111 333.file
Actual :111 333.file
<Click to see difference>
arrays first differed at element [8];
Expected :111 333.file
Actual :111 333.file
<Click to see difference>sorry, I hadn't time to follow...I didn't and still can't test on a phone.
Some thoughts I had in the mean time:
splitWithoutEmptyValuestoybox ls -All gives me backslashes for spaces in file names, when I execute this in a terminal (or as you said in adb).hg42
commented
3 years ago ok, that's interesting :-) I created a file with spaces and found that ls handles tty output different from pipe output:
root@RMX1993L1:/ # toybox ls -All /data/data/net.rgruet.android.g3watchdog/files/
total 4
drwx------ 5 u0_a221 u0_a221 4096 2020-10-19 05:28:25.513354851 +0200 .Fabric
-rw-r--r-- 1 root root 0 2021-01-25 18:10:12.763104876 +0100 111\ \ \ file\ with\ \ \ spaces.txt
root@RMX1993L1:/ # toybox ls -All /data/data/net.rgruet.android.g3watchdog/files/ | cat
total 4
drwx------ 5 u0_a221 u0_a221 4096 2020-10-19 05:28:25.513354851 +0200 .Fabric
-rw-r--r-- 1 root root 0 2021-01-25 18:10:12.763104876 +0100 111 file with spaces.txtso, everything is as expected, the PR is fine, thanks fro the work!
Jakeler
commented
3 years ago Indeed very interesting, so it is actually caused by toybox. Thanks for the infos!
I was wondering that too, spent hours in the debugger yesterday. Figured that it comes from libsu, the output of Shell.su(cmd) is already without backslashes. Digging in that implementation shows that it directly takes a stdout stream without modifying it.
btw: The whole approach with parsing shell output seems very fragile to me, I think there are more possible issues. A unsolved one is #218 where a file contains '. Other characters with special meaning in the shell could cause similar issues.
Maybe a custom native module would improve the situation? Or Shizuku looks interesting, but I am not sure if it already has the required APIs.
hg42
commented
3 years ago well, I know, that several commands behave differently with tty, e.g. colors. In a pipe they explicitly try to create machine readable formats (or better optimized for processing). E.g. a simple ls | cat switches to one file name per line (option -1). There is also an option for escaping:
-b escape nongraphic chars
which pointed me to this conclusion.
I agree, we should definitely check more of the shell quoting/escaping.
Description Backing up the F-Droid store app with data fails if a specific APK file is in the app data.
Steps To Reproduce
I tried many other apps/APKs and could not find any others that cause this error... but with this gallery app it happens reproducibly.
Logcat
I don't see a difference to the other .apk files, but the backup works if I just delete
Schlichte\ Galerie\ Pro\ -\ Foto\ Manager\ \ Editor-6.18.0.apk, very strange.System Information: