getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

File upload: Take ASCII conversion rules of current language and/or user language into account #4972

Open lukasbestle opened 1 year ago

lukasbestle commented 1 year ago

Description

Kirby only uses the default conversion rules when converting filenames on upload. This breaks when files with names in another language are uploaded.

Expected behavior

Kirby should take the current language into account. We need to determine whether this should be the current content language, the current user language or both.

To reproduce

  1. Name a file 日本語.pdf
  2. Upload the file to a files section
  3. Error "You are not allowed to upload invisible files" because all characters are stripped by the default ASCII rules

Additional context

https://github.com/getkirby/getkirby.com/issues/1817

distantnative commented 1 year ago

I would think that the current user language has the higher chances to match the language of uploaded file names.

lukasbestle commented 1 year ago

Thinking about it again, I think you are right. The content language does not make much sense as the filenames are independent from the language. I.e. if you upload a file while the content language is set to language A, the filename will be exactly the same once you switch to language B.

So we can only rely on the user language here.

distantnative commented 1 year ago

I would think this is the line where we would need to add the change: https://github.com/getkirby/kirby/blob/main/src/Cms/FileActions.php#L183

However, I am wondering whether it would make sense to use File\Filename here as well, just without any attributes in the template string. And then add the logic to File\Filename instead, which should call F::safeName probably with a new second parameter for additional rules?

distantnative commented 1 year ago

Something like this for File\Filename::sanitizeName():

// temporarily store language rules
$rules = Str::$language;
$kirby = App::instance(null, true);

// if current user with language, use those rules for `Str` class
if ($language = $kirby?->language($kirby?->translation()->code())) {
    Str::$language = $language->rules();
}

// sanitize name
$name = F::safeName($name);

// reset language rules
Str::$language = $rules;
return $name;
lukasbestle commented 1 year ago

Sounds about right. I think we should also harmonize the logic of F::safeName() and Filename::sanitize*(). They currently behave a bit differently.

distantnative commented 11 months ago

Harmonizing methods: https://github.com/getkirby/kirby/pull/5760

bastianallgeier commented 6 days ago

For the second part, we need to look into here: https://github.com/getkirby/kirby/blob/d14bc478632c3f1eec9b06ff2a5679ef1db2992e/src/Cms/FileActions.php#L228

… and then pass the custom language slug rules to the F::safeName method.