User tests: Successful: Unsuccessful:
Make sure the Itemid filter always casts to integer, when the itemid get passed an array of items the current getint filter (int
, integer
) returns an array but the itemid should never be an array.
Make sure the usual Frontend and backend still works and the correct itemid is in the html classes.
We type cast the itemid to integer
Passing an array would result into the itemId not to be an actual ID any more but an array.
None cc @SniperSister @HLeithner
Reported by: https://twitter.com/paulosyibelo
Status | New | ⇒ | Pending |
Category | ⇒ | Administration Templates (admin) Front End com_contact com_content com_search com_tags Layout External Library Libraries Modules Templates (site) |
Labels |
Added:
?
|
Is there a real situation why we need this change? Would be good to have a test case in core to reproduce the issue.
For me it looks strange when we have a function getInt and then have to do an int cast, especially on Itemid which is never an array. It is even more awkward that an array will be casted to integer. Because casting an array to integer doesn't return the value, it returns always 1, so it would be wrong anyway.
Execute var_dump((int)[2]);
and it will return 1. So if you want to handle that Itemid can be an array then do it right and make a type check and work with arrays properly.
Is there a real situation why we need this change?
See #26753. This is needed to avoid errors when Itemid
is being manipulated by user. You're right, though, this should not be cast like that. Should default to 0 or whatever we default to now.
Also test failure is related.
But would it then not be better to ensure that Itemid is always an integer before core gets executed instead afterwards?
I'm using on tons of places getItn since ages, if this is not safe anymore and doesn't always return an integer then we need to inform the extension devs about this change.
It's still safe to use. The array support traces back to joomla-framework/filter#13.
Basically since early 2016, stable releases of the joomla/filter
package have supported cleaning either a single item or an array of items using a given filter. So InputFilter::getInstance()->clean(['1', 2, '3', 4], 'int');
should be returning a cleaned array of integers. Without that you get array to string conversion errors when working with request data that you aren't expecting to be in an array, and there isn't a simple way to deal with that without breaking the inbuilt filtering from the joomla/input
API on any get()
call.
But would it then not be better to ensure that Itemid is always an integer before core gets executed instead afterwards?
I would suggest casting here is the wrong behavior. The Itemid
input variable probably needs to be checked early and the application just flat out abort with some kind of Exception if the Itemid
input variable is an array; an array is flat out invalid data and should be rejected and casting is going to accept it and create potentially unwelcome side effects. Same goes for other core input variables (i.e. view
, format
, option
, tmpl
, layout
, task
for the "not so legacy legacy" MVC, type
for the feed views, all of which are the main system variables; at the component level you've got input variables like id
that should also be checked for array versus single item). Basically a lack of proper input validation is letting some fields which are expected to be single items come in as arrays which causes problems (BTW the problem isn't the array support in the filter class, it's the fact that the data is coming out of the request superglobals as an array then going into the filter API before there is a chance to even check the data structure is valid).
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-04-22 11:04:57 |
Closed_By | ⇒ | zero-24 |
I'm using on tons of places getItn since ages, if this is not safe anymore and doesn't always return an integer then we need to inform the extension devs about this change.