Fixed issue #19442: Generated analytics script does not properly escape question group name

[mfavetti]

Fixed issue #19442: Generated analytics script does not properly escape question group name

https://bugs.limesurvey.org/view.php?id=19442

master

[Shnoulle]

I prefer to encode only part of the data

Else : GA have a documentation about authorized character ?

[Shnoulle]

htmlspecialchars($surveyName) . '-[' . $surveyId . ']/[' . $page . ']-' . htmlspecialchars($groupName)

Maybe use Chtml::encode ?

[mfavetti]

this issue is not related to GA itself at all

limesurvey is generating javascript (in this context a string), so the contents of the string must be properly escaped or the browser will not be able to properly parse the javascript. i think it's appropriate to escape the function's entire return value as it's dumped directly into a javascript context later by twig:

LimeSurvey/themes/survey/vanilla/views/subviews/header/google_analytics.twig

Line 56 in ddf8833

gtag('event', 'page_view', { page_title: '{{ trackUrl }}' })

usage of Chtml::encode vs htmlspecialchars seems to be split 50/50 across code base, is Chtml::encode the preferred method?

[Shnoulle]

this issue is not related to GA itself at all

OK

limesurvey is generating javascript (in this context a string), so the contents of the string must be properly escaped or the browser will not be able to properly parse the javascript. i think it's appropriate to escape the function's entire return value as it's dumped directly into a javascript context later by twig:

Then escape it in twig part ? No ?

usage of Chtml::encode vs htmlspecialchars seems to be split 50/50 across code base, is Chtml::encode the preferred method?

I don't know, but i always think it's better to use framework functionnality if we have a framework. Personnal opinion here.

[mfavetti]

ok will escape in twig and update tmrw

[Shnoulle]

ok will escape in twig and update tmrw

Still my opinion :) Not sure at 100% (i set it as comment, not are "To update")

[Shnoulle]

ok will escape in twig and update tmrw

Oh ! It's not only an opinion : if someone fix it in template : you broke the fix.

This NEED to be done in template only ! Else : broke API.

[Shnoulle]

Don't broke API : if someone fix views/subviews/header/google_analytics.twig in extended theme : updating value sent : you broke the theme.