Project

General

Profile

Actions

Bug #87568

closed

Validation of ViewHelper arguments does not detect/cast "invalid" integers (or the image processing stack misses a check)

Added by Stefan P about 5 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Fluid
Target version:
-
Start date:
2019-01-29
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

(I reported for TYPO3 9, but it is already present in TYPO3 8 and even probably earlier)

I noticed this in f.uri.image on the line
$this->registerArgument('maxWidth', 'int', 'maximum width of the image');

If you pass for example maxWidth: '1024' the image gets processed, but if you pass maxWidth: '1024px' (notice the "px") just "nothing happens" when the image should be processed.

I dumped the contents of the variables in the ViewHelper and the integer(!) maxWidth just holds the string(!) "1024px". But it should either cast (leading to the integer 1024) or throwing an exception about mismatching types.

I'm not sure if this is actually intended behaviour. If it's intended than this is a bug/annoyance in the image processing stack, which does not handle values like "1024px" correctly (it silently skips it), here a cast or exception would be appropriate as well.

Actions #1

Updated by Claus Due about 5 years ago

Declaring this argument as "int" is technically wrong because it also allows strings - and Fluid does not and will not begin to cast variable types (I can explain in detail why if you need this). However, declaring it "mixed" doesn't really convey much meaning either.

In the case described in the issue the problem is actually an input error because "1024px" (or anything else with "px" suffix) isn't valid for the image processing - but "1024m" is. Hence we can't just cast the value to an integer as that would discard very important information. But we should perhaps have a check if there's a garbage crop/scale instruction; but that probably should go into the image processing logic itself and so isn't related to Fluid.

If someone picks this up to create a patch you should probably look at how f:image handles the width/height arguments when they contain scaling/cropping info to make sure that these VH are compatible, and to determine if perhaps some of that business logic is best moved to the image utility and extra input sanitation should be added.

Actions #2

Updated by Claus Due about 5 years ago

PS: if your use case means you're 100% certain you will never need to pass any scale/crop instruction, then using `width="{variable as int}"` is valid Fluid and that will cast the variable. But it doesn't work by inspecting the argument type and will not preserve the "c" or "m" modifiers.

Actions #3

Updated by Claus Due about 4 years ago

  • Status changed from New to Closed

Closing as "caused by incorrect use of arguments" - non-integer notations are not supported in this argument and Fluid will not attempt to cast arguments. To avoid, do not pass "px" suffixed values: this works in CSS but is not correct format for Fluid or general image processing dimensions.

Actions

Also available in: Atom PDF