Preparing for Confluence 9.0 - EAP out now

[AndreasEbert]

@Kusal, I found a small bug with com.atlassian.confluence.user.actions.AbstractUserProfileAction#getRenderedAboutMe when called in a Velocity template for an Action class that is extending AbstractUserProfileAction:

• when called as $renderedAboutMe: all good, value is shown
• when called as $action.renderedAboutMe: value is not shown, but literal “$action.renderedAboutMe”, i.e. used value in Velocity is null
• when called as $action.getRenderedAboutMe(): Server error is thrown, with root-cause java.lang.IllegalArgumentException: Attempting to box an already boxed value.
  • Relevant part of stacktrace:

Caused by: java.lang.IllegalArgumentException: Attempting to box an already boxed value
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:143)
	at com.atlassian.velocity.htmlsafe.introspection.AnnotatedValue.<init>(AnnotatedValue.java:45)
	at com.atlassian.velocity.htmlsafe.introspection.AnnotationBoxingMethod.invoke(AnnotationBoxingMethod.java:25)
	at com.atlassian.velocity.htmlsafe.introspection.UnboxingMethod.invoke(UnboxingMethod.java:28)

After taking a deep-dive into how Velocity handles html-safe methods, I found out the following:

• According to com.atlassian.velocity.htmlsafe.HtmlSafeMethodNameAnnotator methods starting with “render” or “getRender” (or ending in “Html”) are treated as @HtmlSafe
• Problematic method AbstractUserProfileAction#getRenderedAboutMe returns a com.atlassian.velocity.htmlsafe.HtmlFragment instead of a normal String.
• → These 2 facts collide somehow: The method is treated as being doubly html-safe, which com.atlassian.velocity.htmlsafe.introspection.AnnotatedValue doesn’t like → Exception “Attempting to box an already boxed value” is thrown

I also looked into why it matters how that method is called. (Did that via the stacktrace):

• when called as $renderedAboutMe: Velocity’s introspection is not used, but Struts/OGNL stuff
  • → Exception does not occur
• when called as $action.renderedAboutMe: Velocity’s introspection IS used → Exception IS thrown, but swallowed by org.apache.velocity.runtime.parser.node.ASTIdentifier: try { ...} catch(IllegalArgumentException iae) { return null; }
  • → null is used instead
• when called as $action.getRenderedAboutMe(): Velocity’s introspection IS used → Exception IS thrown, and NOT swallowed by org.apache.velocity.runtime.parser.node.ASTMethod
  • → Exception bubbles up and is thrown in user’s face

Possible solutions:
Ideally, we as app vendors don’t have to think about how we call a method in Velocity. For that, the solution is to fix AbstractUserProfileAction#getRenderedAboutMe (and possibly other methods also). I see two options:

• Rename it to not start with “getRender” → Not ideal, because a breaking-change.
• Don’t return HtmlFragment, but String instead → No change by callers needed

Your thoughts?