Unsanitize custom dimension values

[MichaelRoosz]

Description:

When migrating from custom variables to dimensions, we noticed that characters like a double quote (") are escaped when saved to the database.

For example: _paq.push(['setCustomVariable', 1, 'name1234', 'test"1234', 'visit']); will write test"1234 to the database

but _paq.push(['setCustomDimension', 1, 'test"1234']); will write test"1234 to the database.

Since the values are limited to 200 characters, this seems quite wasteful.

The reason for the difference is:

Custom variables values are unsanitized: https://github.com/matomo-org/plugin-CustomVariables/blob/40c61ad3f2161eec0f3a930613f58384b82af02c/Tracker/CustomVariablesRequestProcessor.php#L101

Custom dimension values coming from an extraction are unsanitized, too: https://github.com/matomo-org/matomo/blob/18513d60885856872128371827fe290ed147df72/plugins/CustomDimensions/Tracker/CustomDimensionsRequestProcessor.php#L160 https://github.com/matomo-org/matomo/blob/18513d60885856872128371827fe290ed147df72/plugins/CustomDimensions/Dimension/Extraction.php#L102

So it seems logical that "normal" dimension values should also be unsanitized.

Review

• [ ] Functional review done
• [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• [ ] Security review done
• [ ] Wording review done
• [ ] Code review done
• [ ] Tests were added if useful/possible
• [ ] Reviewed for breaking changes
• [ ] Developer changelog updated if needed
• [ ] Documentation added if needed
• [ ] Existing documentation updated if needed

[github-actions[bot]]

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

[MichaelRoosz]

Anything I can add or improve to get this merged?

[michalkleiner]

Thanks for the follow up @MichaelRoosz. Since no test started failing, it seems we don't have this specific use case covered. Would you be able to have a look where the processor is being tested and add a test for this scenario?

[github-actions[bot]]

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.