nextcloud / android

📱 Nextcloud Android app
https://play.google.com/store/apps/details?id=com.nextcloud.client
GNU General Public License v2.0
4.22k stars 1.75k forks source link

Prevent upload of files with filenames that contain invalid characters #11241

Open kaystrobach opened 1 year ago

kaystrobach commented 1 year ago

⚠️ Before posting ⚠️

Steps to reproduce

  1. Use and App, which has a share dialogue.
  2. Use nextcloud as a target and select a folder
  3. upload the file

During the process you do not see the actual filename created, nor have an option to change it.

Expected behaviour

Options:

a) silently replace potentially illegal characters with an underscore (:, /, backslash, ...) b) warn user and let user change the filename c) warn user about the problematic filename and ask for permission to proceed like in a)

Actual behaviour

file with a filename like whatever_08:30.pdf is created and the user expects it to be synced to his desktop, which fails with a silent message in the sync log.

Android version

12

Device brand and model

Samsung

Stock or custom OS?

Stock

Nextcloud android app version

3.23.1

Nextcloud server version

25.0.1

Using a reverse proxy?

No

Android logs

no

Server error logs

not relevant here

Additional information

2023-01-10_12-50-36

Might be related to #2711

AlvaroBrey commented 1 year ago

which fails with a silent message in the sync log.

What is the "sync log" in this case? The server log? The Android log? The desktop log?

a) silently replace potentially illegal characters with an underscore (:, /, backslash, ...) b) warn user and let user change the filename c) warn user about the problematic filename and ask for permission to proceed like in a)

a) is not a good idea, we should not replace filenames without asking the user. I'd go for b) but would have to think about the UX.

kaystrobach commented 1 year ago

What is the "sync log" in this case? The server log? The Android log? The desktop log?

The one in the visual desktop sync client, which normally shows the synced files:

grafik

Did not create a screenshot sadly, as this was not my maschine.

It's basically the same error message as in #2711

Could not download tro:lol : Illegal filename

The difference is, that the user is using NTFS and not FAT and Windows as operating system.

a) is not a good idea, we should not replace filenames without asking the user. I'd go for b) but would have to think about the UX.

Agree, that's why I created options I thought about.

Workaround:

AlvaroBrey commented 1 year ago

The one in the visual desktop sync client, which normally shows the synced files:

Hm, then this means the upload to the server does succeed, but the download to the desktop does not. Not sure we can or should do anything about this, as the illegal characters ultimately depend on the filesystem and OS of the desktop client.

kaystrobach commented 1 year ago

The point is, that even on android the download would fail with fat32, as the colon is forbidden.

Maybe an option in the client would solve this - always create windows safe file names

kaystrobach commented 1 year ago

https://support.nextcloud.com/#ticket/zoom/40929

AlvaroBrey commented 1 year ago

The point is, that even on android the download would fail with fat32, as the colon is forbidden.

Yes, but I'm not sure we should be policing which filenames users can upload, as long as the upload works. This is a problem when downloading the file.

Perhaps this could be handled in the clients when trying to download a filename that won't work in the specific system they're trying to use.

kaystrobach commented 1 year ago

I'm open, I just want to have the following user story working across devices:

As a user I want to access files I created on any device on any other device without having to rename stuff (which also failed in the web UI) in order to work with my files.

AlvaroBrey commented 1 year ago

cc @tobiasKaminsky as this affects all clients

amalon commented 1 year ago

This one keeps catching me out. Its only on android that I find a whole bunch of my files don't read, thanks to colons in the file paths.. Is it practical for the app to escape invalid characters somehow when saving in fat32 fs, and still keep track of what file/directory they relate to? I need those colons there on other platforms.

amalon commented 1 year ago

Having a play in a terminal, i can touch a file called "a:b" as root in the nextcloud data directories, but not as the terminal app user, even though I can touch "ab" as the terminal app user. I presume the fuse filesystem android uses (not fat32 as I assumed) is limiting it for compatibility with devices with actual SD cards.

There a precedent with escaping paths. My server address already has an explicit port number and therefore includes a colon, so the data directory path already has an escape in it, using "%3A" instead of ":". e.g.: /storage/emulated/0/Android/media/com.nextcloud.client/nextcloud/user@address%3Aport/

Jerome-Herbinet commented 1 year ago

I was confronted with this problem a few days ago. In the web interface (NC 25.0.6), I had created folders with ":" in their name, and it was impossible for me to synchronize these folders locally on my phone with my 3.24.2 application for Android. 2 possibilities :

joshtrichards commented 1 year ago

Related: #9215

aurelf commented 3 months ago

Confirmed here, file with a : in the name created and synced to nextcoud without problem, but impossible to download from Android, without any error message visible to the user, just failed download. It took me a while to understand the problem and it would be nice to at least get an explicit error message in the notification such as "Impossible do download invalid character ':' in file name.". Making it possible would be even better.

aurelf commented 3 months ago

Also this should be tagged as a bug not enhancement. Files uploaded to the server from one side (linux/nextcloud client) not accessible the other side (android client) without any indication of what is the actual issue! Or shall it be reported to nextcloud server/other clients?