Skip to content

Let autocomplete selections and ChoiceFilter auto-submit work under the default CSP (WICKET-6938) - #1667

Open
aeonframework wants to merge 2 commits into
apache:masterfrom
aeonframework:csp-autocomplete-choicefilter
Open

aeonframework wants to merge 2 commits into
apache:masterfrom
aeonframework:csp-autocomplete-choicefilter

Conversation

@aeonframework

Copy link
Copy Markdown

Resolves WICKET-6938.

Problem

Wicket's default Content Security Policy (strict(): script-src 'strict-dynamic' 'nonce-…') blocks two things wicket-extensions still relies on:

  • AutoCompleteTextField selection hooks. AbstractAutoCompleteRenderer#getOnSelectJavaScriptExpression is written into an onselect attribute of each choice, and wicket-autocomplete.js runs it with eval(). Without 'unsafe-eval' that throws, so selecting an item with an expression does nothing, and the only way to use the hook is to weaken the CSP for the whole application. The hook also invites building JavaScript from item data by string concatenation, as its own javadoc examples do; the markup escaping of the attribute does not protect that data once eval() decodes it.
  • ChoiceFilter auto-submit. With autoSubmit the select is rendered with onchange="this.form.submit();", an inline handler the CSP blocks, so the filter silently stops submitting on change.

Approach

Two commits, each standing on its own:

  1. ChoiceFilter registers the same handler with an OnEventHeaderItem, the way FilterForm#enableFocusTracking already does, so the script carries the CSP nonce. The markup only loses the attribute.
  2. Autocomplete dispatches a cancelable, bubbling wicket-autocomplete-select CustomEvent on the input. detail.item is the chosen element and detail.value the text value; a listener may replace detail.value, or call preventDefault() to leave the input unchanged (what returning null from the expression did). Data a listener needs comes from the new AbstractAutoCompleteRenderer#getChoiceData(T), written as markup-escaped data-* attributes whose names must match [a-z][a-z0-9-]*. The listener is registered from renderHead(), so it carries the nonce and nothing is evaluated. The class javadoc shows a complete listener.

getOnSelectJavaScriptExpression is @Deprecated(since = "11.0.0", forRemoval = true) and keeps working where 'unsafe-eval' is allowed: its result becomes the initial detail.value, and the expression still sees the text value as input and arguments[0]. Applications that do not override it see no change.

Migration: move the expression into a wicket-autocomplete-select listener and expose the values it needs through getChoiceData.

Tests

  • mvn clean verify -Pjs-test -pl wicket-extensions,testing/wicket-js-tests -amd (JDK 21, after mvn clean install -DskipTests -Pfast): BUILD SUCCESS. wicket-extensions 114/114, wicket-examples 67 run with 0 failures (14 skipped), QUnit 162/162 (jQuery) and 161/161 (vanilla). japicmp and javadoc ran clean.
  • New ChoiceFilterTest: checks there is no inline onchange and that the header registers the change handler. It fails on the base code.
  • AbstractAutoCompleteRendererTest: checks getChoiceData escaping, skipping of null values, and rejection of invalid attribute names.
  • New QUnit autocomplete-test.js + autocomplete.html, wired into Gruntfile.js for both engines and listed in JAVASCRIPTTESTING.md. It covers the event and its detail, a listener replacing the value, a listener cancelling the selection, and a deprecated expression still getting the text value as arguments[0]. Against the base wicket-autocomplete.js its 3 event assertions fail.

Follow-ups

  • On master after this lands: remove getOnSelectJavaScriptExpression and the eval() fallback.
  • Separate change: core still builds string Ajax call listeners with new Function(), which also needs 'unsafe-eval'.

This change was prepared with AI assistance and reviewed and tested by me.

ChoiceFilter with autoSubmit wrote onchange="this.form.submit();" into the
markup. Wicket's default Content Security Policy (strict(): script-src
'strict-dynamic' 'nonce-...') blocks inline event handler attributes, so
under the default settings the filter silently stopped submitting on change.

It now registers the same handler with an OnEventHeaderItem, as
FilterForm#enableFocusTracking already does for focus tracking, so the
script carries the CSP nonce. The markup is unchanged apart from the removed
attribute: the select already rendered its markup id for focus tracking.

Generated-by: an AI coding assistant; reviewed and tested by the author.
The autocompleter ran AbstractAutoCompleteRenderer#getOnSelectJavaScriptExpression
by reading the onselect attribute of the chosen item and passing it to eval().
Wicket's default Content Security Policy does not allow 'unsafe-eval', so
under the default settings that eval throws and selecting an item with an
expression does nothing; the only way to use the hook was to weaken the CSP
for the whole application. The hook also invites building JavaScript from
item data by string concatenation, as its own javadoc examples do, which the
markup escaping of the attribute does not protect once eval() decodes it.

The autocompleter now dispatches a cancelable, bubbling
wicket-autocomplete-select event on the input. detail.item is the chosen
element and detail.value the text value; a listener may replace
detail.value, or call preventDefault() to leave the input unchanged (what
returning null from the expression did). Data a listener needs is written
with the new AbstractAutoCompleteRenderer#getChoiceData as escaped data-*
attributes whose names are restricted to lowercase letters, digits and
dashes. The listener is registered from renderHead() and so carries the
CSP nonce; nothing is evaluated.

getOnSelectJavaScriptExpression is deprecated for removal and keeps working
where the application allows 'unsafe-eval': its result becomes the initial
detail.value of the event. Applications that do not override it see no
difference. Migration: move the expression into a wicket-autocomplete-select
listener and expose the values it needs through getChoiceData. Removing the
method and the eval() fallback on master is a separate follow-up.

WICKET-6938

Generated-by: an AI coding assistant; reviewed and tested by the author.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant