Use core file sanitization

Initial patch.

What about switching that setting to off by default and recommand against using it in the description, with an explanation that Drupal 10.2 can now do this, maybe link to the change record or so? Then we can get it in, don't have to worry about the settings people might have right now and the core version dependency. And if we ever do a 3.x release of this project, we can remove the setting.

I'd say we trigger the event anyway (and we should do so only once, wherever that is exactly), might cause some double transliteration but that probably doesn't cause any issues and if it does, people can disable it.

A slightly different approach: use the old logic only if the transliteration setting is still there add a checkbox to the file settings form as an option to remove it.

The last submitted patch, 5: dropzonejs-use-core-file-sanitization-3384546-4.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

I was thinking about putting the setting on the same page, fine with that. I'm not convinced about the conditional visibility and clearing it though. I don't think it serves much of a purpose compared to always showing it and just setting it to false.

If we ever do a major new version and drop the setting completely, we need an update function anyway to remove it from the sites that for some reason still have it. I'm also quite confused by the current title and description :)

What about the following checkbox title and description:
title: Enable custom filename sanitization for DropzoneJS (deprecated)
description: When enabled, files uploaded through DropzoneJS go through a hardcoded and not customizable sanitization/transliteration process. Starting with Drupal 10.2, site-wide transliteration options are available that also always apply to files uploaded through DropzoneJS. It is recommend to configure those settings and disable this. This feature will be removed in a future version.

The only thing I am not sure about is: we are changing the dropzone setting meaning from "use transliteration alongside the renaming stuff" to "use/don't use trasliteration AND the renaming stuff".

Fixing lint issues

The last submitted patch, 10: dropzonejs-use-core-file-sanitization-3384546-10.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

This patch leaves the setting and provides transliteration+sanitation if the option is turned to TRUE. If the option is FALSE, then no transliteration and no sanitation is provided. \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber::sanitizeName still runs.

+++ b/src/UploadHandler.php
@@ -81,25 +91,33 @@ class UploadHandler implements UploadHandlerInterface {
+      $filename = preg_replace('/(_)_+|(\.)\.+|(-)-+/', '\\1\\2\\3', $filename);
+      // Force lowercase to prevent issues on case-insensitive file systems.
+      $filename = strtolower($filename);
+
+      return $filename . '.txt';
     }
 

you return here, so the event actually does _not_ run?

That said, As you said, there's also a call to the event in \Drupal\dropzonejs\Element\DropzoneJs::valueCallback that is not removed here.

I think that runs later and is sufficient for this to work. That would mean that we don't have to touch this at all except change the todo or so?

There is an existing implementation of that event in core for quite a while now, \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber. We should be able to add a test that uploads for example a .js file, and verify that it gets renamed in the end. If that works, the new core sanitation feature will also work.

also, as suggested, keep the default value to TRUE for now to not change the current behavior.

> you return here, so the event actually does _not_ run?

Here it doesn't, correct.

> That would mean that we don't have to touch this at all except change the todo or so?

I am not sure if I understand. You mean later, if we want to ditch this logic?

The last submitted patch, 14: dropzonejs-use-core-file-sanitization-3384546-14.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Forgot a screenshot there.

Restored the setting in default config and explicitly set to FALSE, the fallback always enabled it again in the setting which was extremely confusing.

Automatically closed - issue fixed for 2 weeks with no activity.