We had a problem that the config file contained value:
session_save_handler = "dbtable ". There is an extra space after
dbtable which was added by mistake and is hard to spot.
- Expected: Piwik returns the value trimmed:
- Got: the value with trailing space
Steps - In config, return value after trimming
Not sure if it is actually a good idea to trim all values. In the end this is a user mistake even if it is hard to spot. Alternatively it could be done in some places where surely no whitespace is expected.
Keep in mind there might be for instance also plugins etc that are using the config for their own values etc.
We can have per-config setting validation when DI is added and used for config.
Alternative: enhance the error/exception message with the needed information. Try their if with
trim the string is not equal?
This could be a nice idea as well. We should convert (eg to integer) or trim keys automatically when we know it is safe. Showing a message that somewhere is a trailing or leading space where we know there should certainly not be one is not needed in this case. This should NOT be handled by the config class and for sure not for all keys. Instead this should be done in the code where needed. To avoid having duplicated code to convert a single config value we ideally create a getter method outside the config class where it belongs to. This is also easily testable then.
If something is suspicious about a value we should show a message or trigger an exception. For instance if a number is expected but got a string. This should not be converted to int automatically as it might be wrong.