@mattab opened this Issue on October 14th 2014 Owner

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: dbtable
  • Got: the value with trailing space dbtable[ ]

Steps

  • In config, return value after trimming
@tsteur commented on October 14th 2014 Owner

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.

@diosmosis commented on October 14th 2014 Member

We can have per-config setting validation when DI is added and used for config.

@ThaDafinser commented on October 16th 2014 Contributor

Alternative: enhance the error/exception message with the needed information. Try their if with trim the string is not equal?

@tsteur commented on October 16th 2014 Owner

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.

Powered by GitHub Issue Mirror