Repository navigation
Let autocomplete selections and ChoiceFilter auto-submit work under the default CSP (WICKET-6938) - #1667
Open
aeonframework wants to merge 2 commits into
Open
Let autocomplete selections and ChoiceFilter auto-submit work under the default CSP (WICKET-6938)#1667aeonframework wants to merge 2 commits into
aeonframework wants to merge 2 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves WICKET-6938.
Problem
Wicket's default Content Security Policy (
strict():script-src 'strict-dynamic' 'nonce-…') blocks two things wicket-extensions still relies on:AbstractAutoCompleteRenderer#getOnSelectJavaScriptExpressionis written into anonselectattribute of each choice, andwicket-autocomplete.jsruns it witheval(). 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 onceeval()decodes it.autoSubmitthe select is rendered withonchange="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:
OnEventHeaderItem, the wayFilterForm#enableFocusTrackingalready does, so the script carries the CSP nonce. The markup only loses the attribute.wicket-autocomplete-selectCustomEventon the input.detail.itemis the chosen element anddetail.valuethe text value; a listener may replacedetail.value, or callpreventDefault()to leave the input unchanged (what returningnullfrom the expression did). Data a listener needs comes from the newAbstractAutoCompleteRenderer#getChoiceData(T), written as markup-escapeddata-*attributes whose names must match[a-z][a-z0-9-]*. The listener is registered fromrenderHead(), so it carries the nonce and nothing is evaluated. The class javadoc shows a complete listener.getOnSelectJavaScriptExpressionis@Deprecated(since = "11.0.0", forRemoval = true)and keeps working where'unsafe-eval'is allowed: its result becomes the initialdetail.value, and the expression still sees the text value asinputandarguments[0]. Applications that do not override it see no change.Migration: move the expression into a
wicket-autocomplete-selectlistener and expose the values it needs throughgetChoiceData.Tests
mvn clean verify -Pjs-test -pl wicket-extensions,testing/wicket-js-tests -amd(JDK 21, aftermvn 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.ChoiceFilterTest: checks there is no inlineonchangeand that the header registers the change handler. It fails on the base code.AbstractAutoCompleteRendererTest: checksgetChoiceDataescaping, skipping ofnullvalues, and rejection of invalid attribute names.autocomplete-test.js+autocomplete.html, wired intoGruntfile.jsfor both engines and listed inJAVASCRIPTTESTING.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 asarguments[0]. Against the basewicket-autocomplete.jsits 3 event assertions fail.Follow-ups
getOnSelectJavaScriptExpressionand theeval()fallback.new Function(), which also needs'unsafe-eval'.This change was prepared with AI assistance and reviewed and tested by me.