Skip to content

Commit

Permalink
XWIKI-19805: Improve parameter escaping in IconPickerMacro
Browse files Browse the repository at this point in the history
  • Loading branch information
@michitux
michitux committed Jun 9, 2022
1 parent 0b732f2 commit 47eb8a5
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions ...latform-icon/xwiki-platform-icon-ui/src/main/resources/IconThemesCode/IconPickerMacro.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@
options['prefix'] = '$escapetool.javascript($xcontext.macro.params.prefix)';
#end
#if("$!xcontext.macro.params.id" != '')
$('#${xcontext.macro.params.id}').xwikiIconPicker(options);
$('#${escapetool.javascript(${xcontext.macro.params.id})}').xwikiIconPicker(options);

This comment has been minimized.

# to view

Copy link
@mflorea

mflorea Aug 1, 2022

Member

This Velocity code is building a CSS selector that is injected in JavaScript so you need two levels of escaping:

  • escape the CSS identifier when building the CSS selector
  • escape the CSS selector when injected in JavaScript

Also note that this change caused a regression, XWIKI-20033, but it's actually not the fault of the fix but how the icon picker macro is used in AppWithinMinutes. This shows that the macro was wrongly used without expecting the macro parameter to be escaped. We need to fix all the places where the icon picker macro is called.
#end
#if("$!xcontext.macro.params.get('class')" != '')
$('.${xcontext.macro.params.get('class')}').xwikiIconPicker(options);
$('.${escapetool.javascript(${xcontext.macro.params.get('class')})}').xwikiIconPicker(options);
#end
});
</script>
Expand Down

0 comments on commit 47eb8a5

Please # to comment.