xperseguers / t3ext-image_autoresize

TYPO3 Extension image_autoresize. Simplify the way your editors may upload their images.
https://extensions.typo3.org/extension/image_autoresize
GNU General Public License v3.0
16 stars 22 forks source link

Wildcard pattern leads to preg_match error #105

Closed dogawaf closed 4 months ago

dogawaf commented 6 months ago

Hello

If I put 1:/*/import/ or 1:/**/import/ in the directories configuration, I get this error:

PHP Warning: preg_match(): Compilation failed: quantifier does not follow a repeatable item at offset 1 in /var/www/html/vendor/causal/image_autoresize/Classes/Service/ImageResizer.php line 519

The wildcard is compilated to respectively /^*\/import\// or /^**\/import\// which are non-valid patterns.

dogawaf commented 6 months ago

Fix proposal:

--- a/Classes/Utility/FAL.php      2024-03-11 09:56:55.196568893 +0000
+++ b/Classes/Utility/FAL.php     2024-03-11 09:54:55.036251580 +0000
@@ -280,8 +280,8 @@
             return '';
         }
         $pattern = '/^' . str_replace('/', '\\/', $directory) . '/';
-        $pattern = str_replace('\\/**\\/', '\\/([^\/]+\\/)*', $pattern);
-        $pattern = str_replace('\\/*\\/', '\\/[^\/]+\\/', $pattern);
+        $pattern = str_replace('**\\/', '([^\/]+\\/)*', $pattern);
+        $pattern = str_replace('*\\/', '[^\/]+\\/', $pattern);

         return $pattern;
     }

Why?

\Causal\ImageAutoresize\Utility\FAL::getDirectoryConfig matches the leading \, and doesn't pass it to \Causal\ImageAutoresize\Utility\FAL::getDirectoryPattern.

Also, \Causal\ImageAutoresize\Service\ImageResizer::getRuleset try to match the relative path of the file, so without any leading /.

Thus, \Causal\ImageAutoresize\Utility\FAL::getDirectoryPattern should not match nor include any leading / in the resulting pattern.

xperseguers commented 6 months ago

[...] matches the leading \, and doesn't pass it to [...]

Indeed, but it's the leading /.

Not matching the leading / anymore means the wildcard may now be used with some directory prefix, which was not intended at first. It doesn't seem to be a real problem however.

xperseguers commented 6 months ago

Other part of the code rely on having a leading slash: https://github.com/xperseguers/t3ext-image_autoresize/blob/master/Classes/Command/BatchResize.php#L93

As such, I wonder if we should not include that leading slash instead of stripping it away.