newhinton / Round-Sync

An android cloud file manager, powered by rclone. Visit https://roundsync.com for more information!
https://roundsync.com
GNU General Public License v3.0
1.29k stars 51 forks source link

sftp upload bug #165

Closed thri1hous closed 1 year ago

thri1hous commented 1 year ago

re-posting of this issue https://github.com/x0b/rcx/issues/239

What version of Round Sync are you using (About -> App version)?

RoundSync - v2.2.2

What is your Android version, phone model and manufacturer?

Android 13, OnePlus 8t

Which steps are required to reproduce this issue?

1. Create new SFTP remote with only username and password (issue persistent with public/private key)
2. Log into remote and navigate to directory outside of the home directory e.g. /media/ext/test/
3. Upload file to this directory from local storage or create new folder e.g test.txt
4. Instead of copying the file to /media/ext/test/test.txt rclone copies the file to /home/username/media/ext/test/test.txt creating new directories if needed. Basically it treats the home directory as the root directory

Please also enable rclone logging (Settings > Logging > Log Rclone errors). You're going to need the log for the last question. The app doesn't think it had an error so this is left blank

What is your configuration (rclone.conf)?

You can reproduce this with a clean .conf and new SFTP remote

Does the same issue also occur when using the same configuration on a PC or in Termux?

no but you are passing the path on cli, not relying on a gui to pass the right path

this seems to be an issue where the leading / is just left out of the path name. so i would think this is an easy fix this might also be the cause of this issue https://github.com/newhinton/Round-Sync/issues/113 if the user tries to download a file /test.txt but rclone tries to download ~/test.txt that file might not exist so the download will fail

newhinton commented 1 year ago

Thanks for the analysis, i will have to eventually look into it. Though your hint with the missing slash is a really good starting point!

newhinton commented 1 year ago

Okay, i have a good idea where the issue is.

However, to fix it, i have to understand the implications of my change, and that can take a while. The responsible file is close to 2200 lines long.

For Reference: https://github.com/newhinton/Round-Sync/blob/1d305f75ce2a0cb9927945930da3f007469b7e19/app/src/main/java/ca/pkay/rcloneexplorer/Fragments/FileExplorerFragment.java#L1563 (New Folder) and https://github.com/newhinton/Round-Sync/blob/1d305f75ce2a0cb9927945930da3f007469b7e19/app/src/main/java/ca/pkay/rcloneexplorer/Fragments/FileExplorerFragment.java#L557 (Upload Files)

This very likely also applies to any interaction with "non-standard" paths that do not start at / by default. And all actions with them.

thri1hous commented 1 year ago

Yeah fix could have unknown implications. I'd be happy to help test different remotes once you've decided on a path forward for the fix. Hopefully we can make sure a fix won't break something else.

newhinton commented 1 year ago

I have created a prerelease which contains a fix. It is still under testing, so use at your own risk, but if you are willing to check it, feel free!

thri1hous commented 1 year ago

I tested the prerelease and I can confirm the issue is now fixed. I tried to test the other issue issue And it's seems to be fixed but I never tried to recreate the issue on the older version so I can't confirm. Maybe the person who created that can test to confirm. I will continue to test with other remotes to try to make sure the fix didn't create unintentional issues