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

🪟📁📄 Ability to enforce windows compatible file names on the server #44963

Closed
1 of 3 tasks
AndyScherzinger opened this issue Apr 22, 2024 · 17 comments · Fixed by #47185
Closed
1 of 3 tasks

🪟📁📄 Ability to enforce windows compatible file names on the server #44963

AndyScherzinger opened this issue Apr 22, 2024 · 17 comments · Fixed by #47185

Comments

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Apr 22, 2024

How to use GitHub

  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Is your feature request related to a problem? Please describe.
In some scenarios with a large chunk of the user base being Windows users, using the Desktop client for example sync conflict can get created easily via the web interface, other clients or directly via any client software supporting WebDAV by creating files or folders that cannot be synced down to Windows ever.

Describe the solution you'd like
Add a setting to 'enforce windows compatible file names' on the server, so when the user tries to rename/create files/folders that would generate conflict when syncing, the server would prevent the action. This setting should be optional and needs to be turned on explicitly.
Also there is no logic expected to migrate existing data, just to prevent it from the moment on the setting is active.

Describe alternatives you've considered
None, given the fact that the person creating a file/folder might not be the one being affected by it and the affected person might not have the permissions on the system to change the names, i.e. when shared read-only.

@AndyScherzinger AndyScherzinger added enhancement 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Apr 22, 2024
@AndyScherzinger AndyScherzinger added this to the Nextcloud 30 milestone Apr 22, 2024
@AndyScherzinger AndyScherzinger moved this to 🧭 Planning evaluation (don't pick) in 📁 Files team Apr 22, 2024
@sorbaugh sorbaugh moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📁 Files team Apr 22, 2024
@tobiasKaminsky
Copy link
Member

Subscribing myself, as I think this is then also a task for clients.

  • If server returns a specific error message, clients should show a correct info
  • if setting is enabled via capabilities, clients can directly prohibit renaming/creating such a file

@susnux
Copy link
Contributor

susnux commented Apr 23, 2024

@tobiasKaminsky we need this implemented / fixed on the clients, server and WebUI are already done:

Implementation: Get the capability "forbidden_filename_characters" from the files app. This is an array of forbidden characters (could be multibyte characters, so it is an array of strings). If any of the strings in the array matches the filename it is considered invalid.

@susnux susnux self-assigned this Apr 23, 2024
@SystemKeeper
Copy link
Contributor

Besides some characters not allowed on windows, there are more rules that need to be taken into account, e.g. a Filename is also not allowed to end with a dot or a space. There are also some reserved filenames.
See https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions for details.

@susnux
Copy link
Contributor

susnux commented Apr 23, 2024

Yes but having the forbidden characters implemented in clients is a good first step and also allows to configure other compatibility (like with some filesystems that forbid just :).

See https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions for details.

This is also only partially true, Windows supports a lot of naming things, but the Explorer simply does not, like trailing . is working just win32 API needs the ? flag to disable path normalization (and of course Explorer fails to handle it).
Just like with Mac where some buggy GUI application forbid files with colon in the name, while on OS layer this is pretty fine.

But we probably should still enforce the character + the trailing space / dot rules. About the path length we can not do much, that depends on the users mount point and file type (e.g. for spreadsheets the table name counts into the path length 🤦).

@susnux susnux moved this from 📄 To do (~10 entries) to 🏗️ In progress in 📁 Files team Apr 30, 2024
@skjnldsv
Copy link
Member

skjnldsv commented Apr 30, 2024

Maybe that is relevant?

/**
* Blacklist characters from being used in filenames. This is useful if you
* have a filesystem or OS which does not support certain characters like windows.
*
* The '/' and '\' characters are always forbidden.
*
* Example for windows systems: ``array('?', '<', '>', ':', '*', '|', '"', chr(0), "\n", "\r")``
* see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
*
* Defaults to ``array()``
*/
'forbidden_chars' => [],

@susnux
Copy link
Contributor

susnux commented Apr 30, 2024

Not only it is not only specific characters but also files and casing, and that list for characters is not complete (missing e.g. char 1-31).

@AndyScherzinger
Copy link
Member Author

You also can't have a space or a dot at the end of a folder name on Windows.

@skjnldsv
Copy link
Member

skjnldsv commented May 2, 2024

You also can't have a space or a dot at the end of a folder name on Windows.

pretty sure we trim the names on NC side 🤔

@AndyScherzinger
Copy link
Member Author

pretty sure we trim the names on NC side 🤔

created via "New" in the webUI
2024-05-02 13_54_50-Files - Nextcloud – Mozilla Firefox
and also
2024-05-02 13_56_11-Files - Nextcloud – Mozilla Firefox

@susnux
Copy link
Contributor

susnux commented May 2, 2024

pretty sure we trim the names on NC side 🤔

No, spaces are pretty valid on Mac and Linux. But we restrict even on Linux to not include ascii 0-31 (including tab).

@susnux
Copy link
Contributor

susnux commented May 29, 2024

After discussion with @Altahrim and @come-nc we came to the conclusion the best is to keep the configuration generic but just provide a "preset" for windows.
Meaning we have multiple options to fine tune our file restrictions:

  • Forbidden filename characters (already implemented by us)
  • Forbidden filenames (already implemented by us)
  • Forbidden filename prefixes (needed for windows due to namespace support)
  • Forbidden filename extensions (optional but makes sense to replace unused filename regex)

@jancborchardt

This comment was marked as off-topic.

@tobiasKaminsky

This comment was marked as off-topic.

@susnux
Copy link
Contributor

susnux commented Jun 24, 2024

@AndyScherzinger @jancborchardt

How do we handle this use case:

  1. No rules enabled
  2. User uploads file that is not compatible but currently allowed
  3. Admin enabled Windows support

Now the behavior can be:

  1. No access to the files anymore
  2. Readonly access with ability to rename
  3. Update but not new files with those rules

3 is probably very hard and requires work on internal filesystem -> 2 would be be from my point of view.

@tobiasKaminsky
Copy link
Member

Capabilities:

  • provides 4 (above) options with each forbidden characters/patterns

Webdav:

  • method: put, mkcol, copy, move
  • if wrong character
  • http 400 return, with sabre dav exception

OCS:

  • find all endpoints, e.g. create new file from template via direct editing (/ocs/v2.php/apps/files/api/v1/directEditing/create)

@susnux checks if exception can be localised via http header

I can provide a LTD with branch/app, so that we can all have the same state.

@AndyScherzinger
Copy link
Member Author

@susnux I would say option (3). Why do you think it is hard? Wouldn't is just mean that while updating an existing file's content the checks simply won't be executed?

So the checks would be on

  • Create
  • Move (including rename)

@susnux
Copy link
Contributor

susnux commented Jul 12, 2024

Status update:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment