[
https://issues.apache.org/jira/browse/WW-5772?focusedWorklogId=1047483&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1047483
]
ASF GitHub Bot logged work on WW-5772:
--------------------------------------
Author: ASF GitHub Bot
Created on: 14/Oct/26 19:27
Start Date: 14/Oct/26 19:27
Worklog Time Spent: 10m
Work Description: aeonframework opened a new pull request, #2012:
URL: https://github.com/apache/struts/pull/2012
Fixes [WW-5772](https://issues.apache.org/jira/browse/WW-5772)
### Problem
The `optiontransferselect`, `inputtransferselect` and `updownselect` tags
render every button with an `onclick` handler that the framework builds itself.
The `combobox` tag renders its select with an inline `onChange`. An application
that wants a nonce-based Content-Security-Policy without `'unsafe-inline'`
therefore cannot use these tags, even though `<s:script>` already applies the
nonce to the scripts the same templates emit.
The generated `onclick` code also contains the `headerKey`/`doubleHeaderKey`
values as JavaScript string literals that are only HTML-escaped. The browser
decodes `'` back to `'` before it runs the handler, so a header key that
contains a quote breaks the handler.
### Approach
- **Transfer selects and updownselect.** Each button now carries its action,
the target select id(s) and the header key(s) in `data-struts-*` attributes,
which FreeMarker HTML-escapes. `optiontransferselect.js` (also used by
updownselect) and `inputtransferselect.js` each register one delegated click
listener per page, in the capture phase, guarded against double registration.
The listener calls the same existing functions with the same arguments as the
old inline handlers, header-key/no-header-key branches included.
`optiontransferselect.ftl` folds its duplicated per-button branches into one
`transferButton` macro, about 100 lines shorter. A developer-supplied
`*Onclick` attribute is still rendered as `onclick` and still runs after the
framework action.
- **Combobox (simple and html5).** The select carries
`data-struts-combobox="<escapedId>"`. The `change` listener is attached from an
`<@s.script>` block, which carries the nonce like the `autoPopulate_<id>`
function the template already emits. The script binds every select with that
marker that is not bound yet. Repeated comboboxes with the same id (e.g. inside
an iterator) therefore each get exactly one handler, and fragments loaded over
AJAX work too: jQuery `.html()` evaluates scripts through a temporary element
in `<head>`, so `document.currentScript` cannot be used.
- The old global JS functions are unchanged, so custom themes that copied
the old templates keep working.
### Why it is systematic
Each of these tags had the same pattern: framework-built JavaScript in
event-handler attributes. The patch removes that pattern from them entirely
rather than escaping individual values, so neither the CSP blocker nor the
quoting problem can come back through these templates.
### Upgrade note (please add to the release notes / migration guide)
The new markup needs the new `optiontransferselect.js` and
`inputtransferselect.js`.
- An application that serves its own copy of the static files
(`struts.serve.static=false` or a custom `struts.ui.staticContentPath`) must
update those two files together with the templates. Otherwise the transfer
buttons silently do nothing.
- The static URL is not versioned and browsers cache it for one day, so a
browser holding the old copy right after an upgrade sees inactive buttons until
the cache expires.
- Code that calls `button.onclick()` directly no longer triggers the
framework action. Real clicks and `element.click()` still do.
### Tests
- `./mvnw -pl core test` on JDK 17: 3466 tests, 0 failures, 0 errors, 0
skipped.
- New tests in `OptionTransferSelectTagTest` and `UpDownSelectTagTest` check
that framework buttons render no `onclick` and that a header key containing a
quote (`it's-left`) lands HTML-escaped in a `data-*` attribute. The rendering
fixtures are updated. On the base templates, 51 of the 57 tests in the four
affected tag test classes fail.
- Manual check in headless Chromium, under `<meta
http-equiv="Content-Security-Policy" content="script-src 'nonce-…'">` with no
`'unsafe-inline'`: zero CSP violations. These all worked:
- optiontransferselect add-to-right, add-all-to-left and select-all, with
the header options kept in place;
- updownselect move-up and select-all;
- inputtransferselect add, and move-up stopping at the header key;
- two comboboxes with the same id in two forms, each populating only its
own field;
- a combobox inserted with `innerHTML` and its scripts evaluated
jQuery-style, which populated its field.
### Follow-ups (separate tickets; this PR does not make the themes fully
CSP-clean)
- xhtml/css_xhtml `form.ftl`: default `onreset="clearErrorMessages…"` and
the `onsubmit="return validateForm_…"` for client-side validation
- xhtml `tooltip.ftl`: `domTT_activate` `onmouseover` when
`javascriptTooltip` is on
- `doubleselect`: the `onchange="<id>Redirect(…)"` built in
`DoubleSelect.java`
This change was prepared with AI assistance and reviewed and tested by me.
Issue Time Tracking
-------------------
Worklog Id: (was: 1047483)
Remaining Estimate: 0h
Time Spent: 10m
> Remove framework-generated inline event handlers from transfer-select,
> updownselect and combobox tags (CSP)
> -----------------------------------------------------------------------------------------------------------
>
> Key: WW-5772
> URL: https://issues.apache.org/jira/browse/WW-5772
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Tags
> Reporter: Aaron Elijah Mars
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h3. Problem
> The {{optiontransferselect}}, {{inputtransferselect}} and {{updownselect}}
> tags render every button with an {{onclick}} handler that the framework
> builds itself. The {{combobox}} tag renders its select with an inline
> {{onChange}}. An application that wants a nonce-based Content-Security-Policy
> without {{'unsafe-inline'}} therefore cannot use these tags, even though
> {{<s:script>}} already applies the nonce to the scripts the same templates
> emit.
> The generated {{onclick}} code also contains the
> {{headerKey}}/{{doubleHeaderKey}} values as JavaScript string literals that
> are only HTML-escaped. The browser decodes {{'}} back to {{'}} before it
> runs the handler, so a header key that contains a quote breaks the handler.
> h3. Approach
> - *Transfer selects and updownselect.* Each button now carries its action,
> the target select id(s) and the header key(s) in {{data-struts-*}}
> attributes, which FreeMarker HTML-escapes. {{optiontransferselect.js}} (also
> used by updownselect) and {{inputtransferselect.js}} each register one
> delegated click listener per page, in the capture phase, guarded against
> double registration. The listener calls the same existing functions with the
> same arguments as the old inline handlers, header-key/no-header-key branches
> included. {{optiontransferselect.ftl}} folds its duplicated per-button
> branches into one {{transferButton}} macro, about 100 lines shorter. A
> developer-supplied {{*Onclick}} attribute is still rendered as {{onclick}}
> and still runs after the framework action.
> - *Combobox (simple and html5).* The select carries
> {{data-struts-combobox="<escapedId>"}}. The {{change}} listener is attached
> from an {{<@s.script>}} block, which carries the nonce like the
> {{autoPopulate_<id>}} function the template already emits. The script binds
> every select with that marker that is not bound yet. Repeated comboboxes with
> the same id (e.g. inside an iterator) therefore each get exactly one handler,
> and fragments loaded over AJAX work too: jQuery {{.html()}} evaluates scripts
> through a temporary element in {{<head>}}, so {{document.currentScript}}
> cannot be used.
> - The old global JS functions are unchanged, so custom themes that copied the
> old templates keep working.
> h3. Why it is systematic
> Each of these tags had the same pattern: framework-built JavaScript in
> event-handler attributes. The patch removes that pattern from them entirely
> rather than escaping individual values, so neither the CSP blocker nor the
> quoting problem can come back through these templates.
> h3. Upgrade note (please add to the release notes / migration guide)
> The new markup needs the new {{optiontransferselect.js}} and
> {{inputtransferselect.js}}.
> - An application that serves its own copy of the static files
> ({{struts.serve.static=false}} or a custom {{struts.ui.staticContentPath}})
> must update those two files together with the templates. Otherwise the
> transfer buttons silently do nothing.
> - The static URL is not versioned and browsers cache it for one day, so a
> browser holding the old copy right after an upgrade sees inactive buttons
> until the cache expires.
> - Code that calls {{button.onclick()}} directly no longer triggers the
> framework action. Real clicks and {{element.click()}} still do.
> h3. Follow-ups (separate tickets; this PR does not make the themes fully
> CSP-clean)
> - xhtml/css_xhtml {{form.ftl}}: default {{onreset="clearErrorMessages..."}}
> and the {{onsubmit="return validateForm_..."}} for client-side validation
> - xhtml {{tooltip.ftl}}: {{domTT_activate}} {{onmouseover}} when
> {{javascriptTooltip}} is on
> - {{doubleselect}}: the {{onchange="<id>Redirect(...)"}} built in
> {{DoubleSelect.java}}
> This change was prepared with AI assistance and reviewed and tested by me.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
