Skip to content

feat: read event data property paths without compiling them - #26916

Merged
mcollovati merged 8 commits into
mainfrom
csp-event-data-property-paths
Nov 9, 2026
Merged

mcollovati merged 8 commits into
mainfrom
csp-event-data-property-paths

Conversation

@totally-not-ai

@totally-not-ai totally-not-ai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Part of #26152

Behavior deviation · flow-client, flow-server · apps running with a Content Security Policy without unsafe-eval, or with the strict CSP mode

Background — event data expressions. A DOM listener can ask the browser to send values with the event, written as JavaScript expressions such as event.detail.value. The browser compiled each one with new Function, which a Content Security Policy without unsafe-eval blocks.

Event data and filters that are plain property paths, like event.detail, element.value or event.detail?.value, are now read in the browser without compiling anything. They work without unsafe-eval and are no longer reported in the warn and strict CSP modes. Together with the built-in navigation events no longer being reported, a UI now starts in strict mode.

Risks:

  • ⚠️ Behaviour change: in the warn and strict modes, event data and filters that are property paths are no longer reported. Other expressions are still reported and compiled as before.

  • ✅ No API change, no protocol change: the expression string stays the key of the value sent to the server. Same values as the compiled expression: a .name segment read from null/undefined throws a TypeError, and a ?.name segment makes the whole path undefined.

  • Added a client resolver for event or element followed by .name or ?.name segments, tried before new Function for listener event data, filters, mapped elements and @EventData of template event handlers.

  • The server uses the same pattern for event data, mapped elements (addEventDataElement) and setFilter.

  • The UI navigation, refresh and web component connect events no longer report their @EventData values, which are keys of data that client code builds and sends with sendEventMessage, never compiled expressions. They are marked with the internal @PrebuiltEventData. An application @DomEvent with such a key is still reported.

  • Updated the Javadoc of addEventData(String), addEventData(List) and setFilter(String).

  • Added client tests for the resolver and for reading paths with new Function blocked, and strict mode tests for paths and look-alike non-paths.

Fixes #26152

The client reads event data such as event.detail.value by walking the
properties of the event or the element instead of compiling the
expression with new Function, so it works under a Content Security
Policy without unsafe-eval. The server no longer reports such paths as
JavaScript sent as a string. The expression stays the key of the value
in the event data sent to the server.

Part of #26152
Document which event data and filters work without unsafe-eval, and
cover paths with another root in the strict mode tests.
@totally-not-ai

totally-not-ai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Type of change

  • Feature
Test coverage
  • Client: the resolver reads paths from the event and the element, including inherited properties and getters, throws a TypeError past null/undefined, and rejects look-alikes such as event.x(), event['x'], event.x[0], events.x, event..x.
  • Client: with new Function blocked, listener event data (also a mapped event.target) and template @EventData paths are still read.
  • Server: in strict mode, paths in addEventData, addEventDetail, addEventDataElement and setFilter, and creating a UI, do not throw. Non-paths and paths with another root (window.location, document.head) still throw.
  • All flow-client unit tests pass.

@github-actions github-actions Bot added the +0.0.1 label Nov 9, 2026
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

 1 677 files  ± 0   1 767 suites  +1   2h 14m 41s ⏱️ + 1m 58s
13 365 tests +33  13 297 ✅ +33  68 💤 ±0  0 ❌ ±0 
13 999 runs  +33  13 931 ✅ +33  68 💤 ±0  0 ❌ ±0 

Results for commit a9d3c2d. ± Comparison against base commit a381afa.

♻️ This comment has been updated with latest results.

Paths such as event.detail?.value are read without compiling them too,
and return undefined when an optional segment is read from null or
undefined, as the compiled expression does.
The navigation, refresh and web component connect events carry data
that client code builds and sends with sendEventMessage, so their
@eventdata values are keys and are never compiled. Mark these events
with the internal @PrebuiltEventData and add their keys without a CSP
report, so a UI starts in the strict mode.
@github-actions github-actions Bot added +0.1.0 and removed +0.0.1 labels Nov 9, 2026
@mcollovati
mcollovati marked this pull request as ready for review November 9, 2026 15:47

@vaadin-review-bot vaadin-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changes — collected 1 finding below that could not be attached to the diff.

Finding
📝 The STRICT note now contradicts this PR: it still says click listener event data throws and that the mode can't be used yet.

📝 The STRICT note now contradicts this PR: it still says click listener event data throws and that the mode can't be used yet.
The note says Vaadin's own event data and filter expressions, "such as those of a click listener", still throw, so the mode "is not yet usable for applications". After this change every @EventData of ClickEvent and KeyDownEvent is a property path and is no longer reported. The PR description also says a UI now starts in strict mode. Please narrow the note to what still throws, such as filters like the double-click event.detail == 2.
CspMode.java:59 · documentation

@totally-not-ai

totally-not-ai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Updated the CspMode.STRICT note: it now says only that some of Vaadin's own filters still need eval, such as those of double and single click listeners and keyboard shortcuts, and that event data that is a property path, like that of a click listener, does not. The remaining flow-maven-plugin unit-test failure is the SBOM validation error that #26915 addresses; it is not related to this change.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Property paths skip both the server's CSP report and the client's new Function compile

sequenceDiagram
    participant R as DomListenerRegistration addEventData or setFilter
    participant M as ElementListenerMap isCompiledByClient
    participant C as CspViolationReporter
    participant G as getOrCreateExpression client
    participant P as PropertyPathExpression createPropertyPathExpression

    R->>M: registers expression, e.g. event.detail.value
    M->>M: isPropertyPath(expression) (new check)
    alt expression is a property path
        Note over M: no longer reported (new)
    else expression needs compiling
        M->>C: reportStringJavaScript()
    end
    G->>P: try createPropertyPathExpression(expression) (new, tried first)
    alt matches event or element path
        P-->>G: reader function, no compile
    else
        G->>G: new Function(event, element, expression)
    end
Loading

The figure shows the one mechanism the pull request adds on both sides of the wire: ElementListenerMap.isCompiledByClient (used by addEventData/setFilter) now calls isPropertyPath, a regex check mirrored on the client in PropertyPathExpression.createPropertyPathExpression, which getOrCreateExpression in ServerEventObject.ts and SimpleElementBindingStrategy.ts now tries before compiling with new Function. Per the PR description, expressions like event.detail.value are read without compiling and are no longer reported in the warn/strict CSP modes, while other expressions keep the old compiled path and report as before.

Diagram Bot draws the mechanism this pull request touches; it does not review the change. Verify it against the diff.

Generated by Diagram Bot for #26916 · agent · 47.1 AIC · ⊞ 9.2K · ◷

@vaadin-review-bot vaadin-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Nothing to flag — the changes look good.

@mcollovati
mcollovati enabled auto-merge November 9, 2026 17:04
@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@mcollovati
mcollovati added this pull request to the merge queue Nov 9, 2026
@totally-not-ai

totally-not-ai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

One of these findings is real and comes from this PR: the server's property-path pattern uses a repeated group, which Java's regex engine matches recursively, so a very long path (e.g. event followed by 10 000 .a segments) throws StackOverflowError. The paths come from application code, not from the browser, so this is not reachable from a request. I have a fix that matches one .name/?.name segment at a time, with tests for a long path and for look-alike roots such as events.x, but this PR is already in the merge queue, so the branch can't be updated. I'll open it as a small follow-up PR once this one is merged, unless you'd rather dequeue this one and take it here.

The other findings stay as they are:

  • The unused key parameter in CspModeTest is the @EventData constructor parameter of a test event; it declares the event data the test needs.
  • The rest (fields that could be final, record patterns, the hidden entry name, final on ExpressionSettings) are on lines this PR doesn't change.

Merged via the queue into main with commit f608bef Nov 9, 2026
52 of 53 checks passed
@mcollovati
mcollovati deleted the csp-event-data-property-paths branch November 9, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read event data property paths without compiling JavaScript

2 participants