Provide more escaping helpers

Development
#1

Hello all,

We observed that some escaping scenarios are tricky to handle, and it would be interesting to provide more helpers to cover those scenarios.

The use case I have in mind is the escaping of wiki macro parameters when the macro expects an array or a collection. But I’m sure we can find others were we’d like to have helpers to ease escaping and prevent mistakes.

As an example, doing a valid escaping for the use case presented above looks like this.

#set ($eval = $services.rendering.escape($escapetool.java($value), 'xwiki/2.1'))
{{translation key="..." parameters="~"$eval~""/}}

And here, every bit counts:

  • the call to $escapetool.java
  • the call to $services.rendering.escape(..., 'xwiki/2.1')
  • wrapping the parameter in ~"...~"

So the question is, where do think is the best location for those escape helpers?

  • Option 1: In org.xwiki.velocity.tools.EscapeTool (see my draft PR to introduce $escapetool.xwiki21.parameterArray)
  • Option 2: in the rendering script service
  • Option 3: in both places (with the same implementations, just called from two different locations).

While option 3 is probably the best for discoverability, it can be misleading to have two ways to do the same thing.
I’d personally opt for option 1 or 2 (even though my PR follows option 1, it not a sign of preference for that option).

WDYT?

#2

In summary velocity tool or script services need to call java component/POJO (unless for the case where the code is only for velocity and it’s not needed for other scripting languages or java).

Now re velocity tool vs script service, the answer is simple:

  • If it’s useful for more than just velocity (i.e. for other scripting languages) then it must go in a script service. Otherwise it should go in a velocity tool.

Thanks

#3

I would like to restart the discussions on this topic, because I had the same issue recently with the translation macro and had a talk with @vmassol on other possible solutions.

We ended up with basically two more options:

  • Option 1: provide a script method to build the macro call and automatically escape the parameters. It could be used this way:
$services.rendering.buildMacro('translation', {
  'key': '...',
  'parameters': ["..."]
}, 'xwiki/2.1')
  • Option 2: provide a more general API to create and manage blocks. This would make it possible to manually create macro blocks and render them with their parameters escaped automatically by XWikiSyntaxMacroRenderer.

However, these two options have the same drawback: it is no longer explicit that Velocity code actually calls a macro.

Another way to look at this problem would be to consider that a macro is not the right tool to manipulate dynamic input, and that we should work on providing alternatives, as well as listing good practices for them. For example, technically we have $services.localization.render() to manage translations (even though this service has its own issues), and we could consider suggesting it when we need to use dynamic input as long as we have an example with the proper escaping for it.

#4

Between your option 1 and 2 (not to be confused with Manuel’s options 1, 2, and 3 ;)), I prefer option 2 which is more genetic. We already support rendering XDOM so I’d simply add a $services.rendering.createBlock(...) method to create a MacroBlock, and then use $services.rendering.render(block) on it.

Now, there are 2 other options:

  • Use $services.rendering.escape($services.localization.render(...), ...) instead of the macro, as you mentioned.
  • Introduce a new $services.rendering.escapeMacroParameters(...) api to continue using the translation macro. This is what @mleduc was proposing above I think.

What are the known problems? We should document them.

+1 to define a best practice

Thx

#5

At the very least, the output should be xml escaped I’m pretty sure.
Not really a “problem” but something to document if we want to suggest it for dynamic input.

#6

I’d have expected the renderer to take care of that: If you put HTML content in a wiki page (using xwiki/2.1 syntax for ex) and save it, you should see the HTML displayed and not rendered as HTML.