update thumbnail, allow user to choose output format#3714
Conversation
|
As a general question - is there a command that would "warm up" thumbnail cache so that even first user can benefit from fast loading image? |
|
There is no command for that yet. This command may generate a lot of thumbnails that will never be displayed on the website. |
… structure, allowing Apache/Nginx to serve them directly without any PHP call
bobvandevijver
left a comment
bobvandevijver
left a comment
There was a problem hiding this comment.
I will need more time to definitively check this, but here's already my first findings. I also believe that are changes in this PR that are not necessarily needed to fix the main issue (which is fixing the webserver direct file serving).
|
|
||
| $this->parseParameters($paramString); | ||
| $urlFilename = $filename; | ||
| $sourceFilename = $this->parseFormatFromFilename($filename); |
There was a problem hiding this comment.
Direct usage of the filename will cause a regression into a security incident (#3661) we had in the past, namely a path traversal. Paths can only be used after a canonicalize call.
There was a problem hiding this comment.
The filename is still used directly without sanitation, this needs to be resolved first.
* Using PathCanonicalize::canonicalize() instead of Symfony's Path::canonicalize() closes a path traversal vulnerability. * fix rector * fix BadRequestHttpException
Bumps [systeminformation](https://github.com/sebhildebrandt/systeminformation) from 5.31.1 to 5.31.6. - [Release notes](https://github.com/sebhildebrandt/systeminformation/releases) - [Changelog](https://github.com/sebhildebrandt/systeminformation/blob/master/CHANGELOG.md) - [Commits](sebhildebrandt/systeminformation@v5.31.1...v5.31.6) --- updated-dependencies: - dependency-name: systeminformation dependency-version: 5.31.6 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Backport the default value from Symfony 7.1+ for easy configuration against Host Header Injection
bobvandevijver
left a comment
bobvandevijver
left a comment
There was a problem hiding this comment.
Lets target this for 6.2 as it changes the paths. Also, can you enable edits from maintainers? If that was enabled I could have fixed the pipeline issues for you.
|
|
||
| $this->parseParameters($paramString); | ||
| $urlFilename = $filename; | ||
| $sourceFilename = $this->parseFormatFromFilename($filename); |
There was a problem hiding this comment.
The filename is still used directly without sanitation, this needs to be resolved first.
| $ext = mb_strtolower(pathinfo($filename, PATHINFO_EXTENSION)); | ||
| if ($this->isSupportedFormat($ext) && pathinfo(pathinfo($filename, PATHINFO_FILENAME), PATHINFO_EXTENSION) !== '') { | ||
| $this->parameters['fm'] = $ext; | ||
|
|
||
| return mb_substr($filename, 0, -(mb_strlen($ext) + 1)); | ||
| } | ||
|
|
||
| return $filename; |
There was a problem hiding this comment.
If I understand correctly, the goal is to retrieve the last part of the path here. Why not just explode on ., and validate if the match is one of the supported extensions?
There was a problem hiding this comment.
I'm not able to allow maintainers to edit the PR ???
I switch the PR to 6.2 and fix both issues.
3 participants
Add output format support to thumbnail URLs
Update thumbnail file path in Twig and filesystem to match image URL structure, allowing Apache/Nginx to serve them directly without any PHP call