IBX-12042: Deprecated filename-less download route in favor of filename-validated variant#775
Conversation
wiewiurdp
force-pushed
the
IBX-12042-Deprecate-filename-less-download-route-content-download-contentId-fieldId-and-replace-it-with-a-filename-validated-variant
branch
from
e495713 to
5aa6a94
Compare
ibexa-workflow-automation-1
Bot
requested review from
Steveb-p,
ViniTou,
alongosz,
barw4,
bnowak,
ciastektk,
konradoboza,
mikadamczyk and
tbialcz
and removed request for
a team
konradoboza
left a comment
konradoboza
left a comment
There was a problem hiding this comment.
Apart from one comment and Andrew's remarks.
…in ContentDownloadUrlGenerator
|
| ibexa.user_hash: | ||
| path: /_fos_user_context_hash | ||
|
|
||
| # Must be defined before ibexa.content.download, so that numeric {fieldId} takes precedence over {fieldIdentifier} |
There was a problem hiding this comment.
We could add requirement for filename as well here and use https://github.com/symfony/routing/blob/7.4/Requirement/Requirement.php as their value source, eg. replace \d+ with Requirement::DIGITS
The same for missing requirements in ibexa.content.download. This way, I think we'll be secure about route ordering here as their expected argument types won't conflict with each other
| { | ||
| /** @var \Symfony\Component\Routing\RouterInterface */ | ||
| private $router; | ||
| private const string ROUTE = 'ibexa.content.download.field_id.filename'; |
There was a problem hiding this comment.
Do we need that const if use only in one place in this class?
| } | ||
|
|
||
| if ($filename !== null && $field->value->fileName !== $filename) { | ||
| throw new NotFoundException('File', $filename); |
There was a problem hiding this comment.
Is it worth to add test for that exception?
6 participants
Description:
The
ibexa.content.download.field_idroute accepts plain integer parameters only, which allows enumerating and mass-downloading all accessible files with a simple loop overcontentId× fieldId (see IBX-11681, where filename validation was added to the main download route — this route bypassed it).Changes: