[
https://issues.apache.org/jira/browse/WW-5674?focusedWorklogId=1033445&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1033445
]
ASF GitHub Bot logged work on WW-5674:
--------------------------------------
Author: ASF GitHub Bot
Created on: 03/Aug/26 11:46
Start Date: 03/Aug/26 11:46
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1830:
URL: https://github.com/apache/struts/pull/1830
Fixes [WW-5674](https://issues.apache.org/jira/browse/WW-5674), a sub-task
of [WW-5667](https://issues.apache.org/jira/browse/WW-5667).
## Background
WW-5667 reports OGNL security checks consuming 9% of RUNNABLE CPU samples in
a 2-minute JFR profile under load. The report contains two stack samples
pointing at two independent problems, so it was split:
- **WW-5674 (this PR)** — the per-OGNL-access cost in
`isClassBelongsToPackages` (sample 1).
- **[WW-5675](https://issues.apache.org/jira/browse/WW-5675)** — repeated
config re-parsing caused by `SecurityMemberAccess` being a `Scope.PROTOTYPE`
bean (sample 2). This is the dominant cost and is **not** addressed here.
Note that the fix proposed on WW-5667 (caching the parsed `Set` in a
`SecurityMemberAccess` field) addresses neither problem — it does not touch
this hot path, and it cannot help sample 2 because the instance holding the
field is discarded and rebuilt on each container lookup.
## What changed
`isClassBelongsToPackages` ran on the OGNL member-access path and, for an
N-segment package, allocated a `String[]` plus its N substrings from `split`, a
`List.of` wrapper, an `IntStream` pipeline, N `subList` views, and N
`StringJoiner`-built strings. It is invoked up to four times per
`isAccessible()` call.
1. `toPackageName` now uses the cached `Class.getPackageName()` instead of
`Class.getPackage().getName()`, which resolves through the defining
classloader's package map on every call.
2. The prefix construction is replaced by an in-place index walk, extracted
into a package-private `isPackageBelongsToPackages` so the logic is testable
against package-name shapes no real `Class` can produce.
3. `isClassAllowlisted` walked the same package name twice, once per
allowlist set. A package-private two-set overload now probes both sets at each
prefix, halving that work.
## Behaviour is unchanged
This is the OGNL security gate, so the change is a pure optimisation with
**zero** semantic change, proven rather than asserted:
- The test suite keeps a frozen, verbatim copy of the replaced
implementation as a differential oracle and asserts old and new agree across a
matrix of package-name shapes and candidate sets.
- Package-boundary safety is pinned explicitly: `org.apache.struts2x` must
not match an `org.apache.struts2` entry.
- The obscure default-package edge is pinned:
`struts.excludedPackageNames="."` strips to `""`, which still excludes
default-package classes.
- Equivalence was additionally verified exhaustively over all 3280 strings
on `{a, b, .}` up to length 7. The only divergences are package names ending in
`.`, which `Class.getPackageName()` cannot produce.
- `toPackageName` equivalence was verified over 26 class shapes on the Java
17 target, including hidden classes, JDK proxies, and classes defined by a
classloader that never calls `definePackage()`.
**Array and primitive package semantics are deliberately unchanged.**
`getPackageName()` would resolve `String[]` to `java.lang` and `java.io.File[]`
to `java.io`, where the current code yields `""`. That change is bidirectional
— it tightens the exclusion list but *loosens* the allowlist, since arrays of
allowlisted-package types would become reachable without an explicit
`struts.allowlist.classes` entry. As the allowlist is the primary OGNL defence
in 7.x and is on by default, that question is deferred to its own ticket. The
`isArray() || isPrimitive()` guard preserves current behaviour exactly.
## Testing
Full `core` module suite green: **3158 tests, 0 failures, 0 errors**.
`SecurityMemberAccessTest` passes unmodified — no existing assertion was
changed.
Issue Time Tracking
-------------------
Worklog Id: (was: 1033445)
Remaining Estimate: 0h
Time Spent: 10m
> Make SecurityMemberAccess.isClassBelongsToPackages allocation-free
> ------------------------------------------------------------------
>
> Key: WW-5674
> URL: https://issues.apache.org/jira/browse/WW-5674
> Project: Struts 2
> Issue Type: Sub-task
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Sub-task of WW-5667.
> {{SecurityMemberAccess.isClassBelongsToPackages(Class, Set)}} runs on the
> OGNL member-access hot path and allocates heavily on every call. It splits
> the package name with a regex-based {{String.split("\\.")}}, wraps the result
> in a {{List}}, then streams over the prefixes and builds a brand-new joined
> {{String}} for every one of them before probing the set. For a class in
> {{org.apache.struts2.ognl}} that is one array plus four {{String.join}}
> allocations.
> It is invoked up to four times per {{isAccessible()}} call — once for the
> excluded-package check and once for the allowlist check, for both the
> member's declaring class and the target class (see
> {{isExcludedPackageNames}}, {{isClassAllowlisted}}).
> This matches JFR sample 1 on the parent ticket:
> {noformat}
> java.lang.String.split(String)
> SecurityMemberAccess.isClassBelongsToPackages(Class, Set) :390
> SecurityMemberAccess.isExcludedPackageNames(Class) :386
> SecurityMemberAccess.isPackageExcluded(Class) :371
> {noformat}
> The same prefix-matching can be done by walking the package name from each
> {{'.'}} and probing the set with a substring, with no array, stream, or join
> allocations.
> This is the OGNL security gate, so the change must preserve exact allow/deny
> semantics. Equivalence tests are required for: the default package (empty
> package name), single-segment packages, exact matches, sub-package matches,
> and near-misses that share a character prefix but not a package boundary
> (e.g. {{org.apache.struts2x}} must not match {{org.apache.struts2}}).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
