[g0w6y]

Fixes WW-5830

The on* attributes (and doubleOn*, doubleAccesskey) of the bundled FreeMarker templates are written inside <#outputformat 'JavaScript'>${...}</#outputformat>. These wrappers date from the WW-4972 work (3d20e80, 3549764); doubleselect got its own in WW-5240 (ac16896), the html5 theme copied them in WW-5444 (e24d2f2), and one more line was added to optiontransferselect in WW-5772 (a91460a). With FreeMarker 2.3.35 the JavaScript output format escapes nothing and also switches off the HTML auto-escape that covers every other attribute, so these values are written raw. A handler value such as x" autofocus onfocus="alert(1) closes the attribute and adds new ones, while the same value in title is escaped. The javatemplates engine already escapes these attributes, so the engines also differ.

THREAT_MODEL §8.8 says a value a UI tag renders into an attribute is HTML-escaped "under every bundled theme and rendering engine", with only escape/escapeHtmlBody and URL-valued attributes excepted; §9 says a value the application hands to a framework tag is covered by §8.8.

Change

Drop the outputformat wrappers on exactly these attributes: simple/html5 scripting-events.ftl, simple/html5 form onsubmit/onreset (simple/form-common.ftl, html5/form.ftl), xhtml/form.ftl onreset, simple/doubleselect.ftl, simple/optiontransferselect.ftl (doubleOn*, doubleAccesskey, the button onclick) and the JS tooltip onmouseover in xhtml/tooltip.ftl. No outputformat remains under template/. css_xhtml includes the simple/xhtml templates and is covered.

Compatibility

A handler containing ', &, <, > or " is now entity-encoded in the markup (alert('x') is written alert('x')). Browsers decode character references in attribute values before the handler runs, so legitimate handlers behave the same. An application that worked around the raw output by escaping quotes itself (e.g. onclick="alert("x")") would now be double-escaped.

This closes the attribute breakout only. It does not make request data safe inside JavaScript code or strings in a handler (' is decoded before the script runs); that remains the application's job. I found no default framework path that feeds request data to these attributes; exposure needs an application that binds data into one, e.g. onclick="%{#parameters.x}".

Tests

EventHandlerEscapeTest renders x" autofocus onfocus="alert(1) through textfield (simple, xhtml, css_xhtml, html5), form, doubleselect, optiontransferselect and the JS tooltip, and checks the escaped output, with title as the control and onclick="doIt();" still rendered as is. With the templates reverted 5 of its 6 tests fail (the sixth, the legitimate handler, passes both ways); with the change all 6 pass.

17 existing expected-output fixtures (34 tests) carried ' inside an on* value and now carry ', as Radio-6.txt did in 54d3dd1. I checked every changed fixture line: the only difference is ' to '.

Full core suite on the rebased branch: 3631 run, 0 failures, 0 errors, 2 skipped.

Also checked against a running Struts Boot app rendering <@s.textfield onclick="%{payload}" title="%{payload}"/> with that payload in all four themes: before, onclick="x" autofocus onfocus="alert(1)" (breakout) next to an escaped title; after, onclick="x" autofocus onfocus="alert(1)", escaped like title.

[lukaszlenart]

Thanks, this looks right for the attribute handlers, and the fixture updates (' → ') plus EventHandlerEscapeTest cover them well. One change is needed before merging, plus one note.

1. xhtml/tooltip.ftl needs JavaScript string escaping as well. Unlike the developer-written on* handlers, here the template itself places ${attributes.tooltip} inside a JavaScript string literal: domTT_activate(this, event, 'content', '…'). With HTML escaping alone, a ' in the tooltip text becomes ' in the markup, but the browser decodes it before the handler runs, so the apostrophe still ends the string and the rest of the tooltip text runs as script on mouseover. Escaping the value for the string as well, ${attributes.tooltip?js_string} (the HTML auto-escape then still applies on top), would keep the tooltip text a plain string. Could you add a test that renders a tooltip containing ' and checks the result? Note that domTT inserts the content with innerHTML, so markup in tooltip text still renders as markup. This change doesn't affect that.

2. Behaviour change for the release notes. ', ", &, < and > in on* values are now entity-encoded in the markup, and an application that escaped quotes in its handlers itself will now get double escaping. The PR description already explains this well. I'll carry it into the 7.5.0 notes.

[g0w6y]

Added the JS-string escaping for tooltip.ftl (bd0a664): ${attributes.tooltip?js_string} so the value is backslash-escaped before the existing HTML auto-escaping applies on top, same as you described. EventHandlerEscapeTest gets a new case with ' + a script tag in the tooltip text confirming it can't break out of the JS string; the existing tooltip test's expectation is updated for the extra backslash now ahead of the entity encoding. Full core suite: 3632/0 failures.

[g0w6y]

One more in the same spot: tooltipDelay and tooltipCssClass sit inside their own single-quoted JS-string literals in that same onmouseover attribute, same as tooltip did. Both are plain developer-settable String attributes (go through findString(), OGNL-evaluable), so the same JS-string-breakout applies. Added ?js_string to both (b95ee52) with matching tests. Full core suite: 3634/0 failures.