Joomla! Issue Tracker | Joomla! CMS #21887 - Check for the presence of the optional input param before checking data

Fixed in Code Base
28 Aug 2018
Medium
Build: staging
# 21887
joomla:fix/form-validation

mbabker
28 Aug 2018

Summary of Changes

One of the security patches for 3.8.12 adds an extra check in Joomla\CMS\Form\Form::validateField() and makes use of an optional method parameter to do this. For the single legitimate call to this method in the production code, this assumption is OK to make, but it has broken our unit tests and potentially causes issues if someone has subclassed this class and calls the method omitting the optional $input argument. Therefore, we must check the argument is provided before using it.

Testing Instructions

I think you're going to have to rely on the tests and code review for this one. There is not a use in this repo that can create this issue outside the unit test suite, and I'm not about to build an extension package to try and recreate this bug so it can be "easily" validated.

ff598b0 28 Aug 2018

Check for the presence of the optional input param before checking data

mbabker - open - 28 Aug 2018

brianteeman - comment - 28 Aug 2018

Maybe I am being daft but isn't the point of unit tests is that code changes shouldnt break the tests

mbabker - comment - 28 Aug 2018

Yep.

To be honest though the tests that failed with this change aren't great tests in the first place (they're doing some janky stuff to directly test non-public APIs versus only testing the public API which indirectly would cover this method). But, the janky tests did expose a legitimate production issue with trying to use an optional argument without checking that it was actually given in the first place, so in this case it did help with something, but in a very unexpected way.