mirror of
https://github.com/apache/struts.git
synced 2026-09-11 16:49:40 +00:00
Lukasz Lenart
81b34c295f
* WW-5674 docs(ognl): add design for allocation-free isClassBelongsToPackages
Covers sample 1 of WW-5667: the per-OGNL-access split/stream/join cost in
SecurityMemberAccess.isClassBelongsToPackages. Sample 2 (config re-parsing
caused by the PROTOTYPE bean scope) is tracked separately as WW-5675.
Records the current prefix-matching semantics verified against JDK 17,
including the default-package contains("") edge reachable via
struts.excludedPackageNames="." and the unreachable trailing-dot divergence.
Keeps array and primitive package semantics unchanged, since adopting
getPackageName() there tightens the exclusion list but loosens the allowlist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5674 docs(ognl): add implementation plan for allocation-free package matching
Four TDD tasks: characterise current behaviour, swap toPackageName to the
cached getPackageName() behind an array/primitive guard, replace the
split/stream prefix construction with an index walk, then collapse the
allowlist path's two walks into one.
Task 1 is a characterisation suite that must pass against unmodified code;
a failure there means the spec's semantic claims are wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5674 test(ognl): characterise SecurityMemberAccess package matching
Pins the current behaviour of isClassBelongsToPackages and toPackageName
before the WW-5674 rewrite, including the default-package empty-string edge
reachable via struts.excludedPackageNames="." and the package-boundary case
where org.apache.struts2x must not match org.apache.struts2.
* WW-5674 perf(ognl): resolve package names via cached Class.getPackageName
getPackage() performs a classloader package-map lookup on every call; the name
returned by getPackageName() is computed once and cached on the Class. The
isArray()/isPrimitive() guard covers exactly the inputs for which getPackage()
returns null, so results are unchanged for every class shape.
* WW-5674 perf(ognl): walk package names in place instead of building prefixes
Replaces the split/IntStream/String.join prefix construction with an index walk,
extracted into a pure package-private helper so it can be tested against package
name shapes no real Class can produce. Per call this drops a String[], a list
wrapper, a stream pipeline, N sublist views and N joined strings, leaving one
substring per package level.
Equivalence with the replaced implementation is asserted over a matrix of
package name shapes and candidate sets.
* WW-5674 perf(ognl): match both allowlist package sets in one walk
isClassAllowlisted walked the class's package name twice, once for
ALLOWLIST_REQUIRED_PACKAGES and once for the configured allowlist. A two-set
overload probes both sets at each prefix, halving the work on a path that runs
for every OGNL member access.
Asserted equivalent to OR-ing the two single-set calls across a matrix of class
shapes and candidate sets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5674 test(ognl): assert package-boundary case against the live gate
The sibling-package test asserted only against the test-local copy of the
replaced implementation, so it would have stayed green even if the production
walk were gutted. It now asserts on both the live helper and the frozen oracle.
Also narrows the three-argument isClassBelongsToPackages overload to
package-private: it has a single caller and its test is in the same package,
and public static on a public class is frozen API until the next major release.
Adds a candidate set that makes the consecutive-dot prefix the deciding probe,
and corrects two inaccuracies in the design document.
* WW-5674 docs(ognl): align the design doc with the package-private overload
The three-argument isClassBelongsToPackages was narrowed to package-private
during the final review, but section 3 still showed it as public static and
still carried the superseded justification for publishing it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5674 docs(ognl): correct overstated claims and record the trailing-dot direction
Copilot's review is right that "allocation-free" overclaims: the walk still
creates one substring per package level. What it removes is everything around
that. Retitles the spec and plan accordingly and softens the goal statement.
Documents on isPackageBelongsToPackages that its one divergence from the
replaced implementation is directional. A package name ending in '.' probes one
prefix more, which tightens exclusion but loosens the allowlist. No caller can
produce one today, but the helper is a package-private pure String function, so
a future caller routing some other string through it would inherit the problem.
Also aligns the plan with the package-private overload it now ships, and lists
WW-5676 and WW-5677 as filed rather than pending, including checkDefaultPackageAccess
which the spec previously omitted from its out-of-scope list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5674 docs(ognl): note the naming cleanup deferred to WW-5678
Records why the three-argument overload is package-private while sharing a name
with a public method, and that renaming it — plus narrowing the two public
statics with no external callers — is tracked against 8.0.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>