lukaszlenart opened a new pull request, #1859: URL: https://github.com/apache/struts/pull/1859
Fixes [WW-5677](https://issues.apache.org/jira/browse/WW-5677) Sub-task of WW-5667. WW-5674 replaced `Class.getPackage()` with the cached `Class.getPackageName()` inside `toPackageName`, but deliberately left two neighbouring call sites alone as out of scope. Both sit on the same per-access path, within twenty lines of it. This finishes the job. **No behaviour change.** ## 1. `checkDefaultPackageAccess` made four `getPackage()` calls ```diff -if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) { +if (toPackageName(memberClass).isEmpty()) { ``` …and the same for `targetClass`. `Class.getPackage()` resolves through the defining classloader's package map on every call, and the old condition evaluated it twice per class, for up to two classes per access. The two forms agree for every class shape: `getPackage()` returns `null` for arrays, primitives and `void`, and names the unnamed package with the empty string — all of which `toPackageName` reports as empty. ## 2. `isExcludedPackageNamePatterns` recomputed the package name per pattern ```diff -return excludedPackageNamePatterns.stream().anyMatch(pattern -> pattern.matcher(toPackageName(clazz)).matches()); +String packageName = toPackageName(clazz); +return excludedPackageNamePatterns.stream().anyMatch(pattern -> pattern.matcher(packageName).matches()); ``` `toPackageName` was evaluated inside the lambda, so it ran once per configured pattern. Now once per call. ## Testing This is the OGNL security gate, so per the ticket the equivalence is **asserted rather than argued**: - **`defaultPackageConditionMatchesLegacyAcrossClassShapes`** runs the replaced condition — frozen verbatim as `legacyDefaultPackageCondition`, which calls `getPackage()` directly and never delegates to production code — against the new one over the existing `classShapes()` matrix: arrays, primitives, `void`, a default-package class, a lambda and a JDK proxy. - **`testDefaultPackageAccessPermitsNamedPackageClass`** — a named-package class still passes the gate with `struts.disallowDefaultPackageAccess=true`. - **`testDefaultPackageAccessBlocksArrayTarget`** — an array target, the shape most likely to break the equivalence, stays blocked. The member declares in `java.lang`, so only the target branch can block. All three were mutation-checked rather than assumed non-vacuous: | mutation | fails | |---|---| | arrays/primitives resolve to `java.lang` | `defaultPackageConditionMatchesLegacyAcrossClassShapes`, `testDefaultPackageAccessBlocksArrayTarget` (plus the two WW-5674 equivalence tests) | | member-class condition inverted | `testDefaultPackageAccessPermitsNamedPackageClass` (plus the existing `testDefaultPackageExclusionSetting`) | `mvn test -DskipAssembly -pl core` — 3190 tests, 0 failures, 0 errors. ## Scope deliberately not taken - The `LOG.warn` calls in `checkExclusionList` also call `getPackage()`, but only on the deny path, so there is no hot-path value — and switching them would change the log text from `package java.io` to `java.io`. - Not threading a single package name through `isPackageExcluded` into both helpers. That would remove the last redundant call, but `isExcludedPackageNames`/`isExcludedPackageNamePatterns` are `protected` and subclasses may override them, so changing their signatures is source-breaking. WW-5678 owns that cleanup for 8.0.0. `toPackageName` is now a cached field read plus a branch, so calling it twice is negligible. ## Impact Neither path runs by default: `checkDefaultPackageAccess` only when `struts.disallowDefaultPackageAccess` is enabled, and the pattern loop only when `struts.excludedPackageNamePatterns` is configured — both pattern constants are commented out in `struts-excluded-classes.xml`. This is a consistency fix for deployments that do enable them, not where the WW-5667 9% lives; that was WW-5675. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
