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:

matomo/plugins/CustomDimensions/Tracker/CustomDimensionsRequestProcessor.php

Line 160 in 18513d6

$value = $extraction->extract($request);

matomo/plugins/CustomDimensions/Dimension/Extraction.php

Line 102 in 18513d6

$dimension = Common::unsanitizeInputValue($dimension);

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]

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]

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