[
https://issues.apache.org/jira/browse/WW-5678?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107161#comment-18107161
]
Lukasz Lenart commented on WW-5678:
-----------------------------------
Reviewed against {{main}} at {{74d7ec0d7}} (2026-08-23), after WW-5675, WW-5676
and WW-5677 all landed. Parking this ticket as-is, but recording what has
changed so the next person does not re-derive it.
h3. Item 1 is resolved — no work left
This ticket predicted it: _"WW-5675 may remove this method altogether."_ It
did. The precomputed union of {{ALLOWLIST_REQUIRED_PACKAGES}} and
{{allowlistPackageNames}} replaced the two-set parameter, and both
three-argument overloads are gone. {{SecurityMemberAccess}} now has exactly:
{code:java}public static boolean isClassBelongsToPackages(Class<?> clazz,
Set<String> matchingPackages) // line 428
public static String toPackageName(Class<?> clazz)
// line 400
static boolean isPackageBelongsToPackages(String packageName,
Set<String> packages) // line 449
{code}
There is no longer a name shared across the visibility boundary, so the
reflex-widening hazard Item 1 described cannot occur. Nothing to do.
h3. Items 2 and 3 collapse into a single change
Item 2 (rename) is only source-breaking while the methods are public. Item 3
(narrow) makes them package-private. Done in that order, the rename costs
nothing and needs no deprecation cycle — so this is one edit, not two: narrow
{{isClassBelongsToPackages(Class, Set)}} and {{toPackageName(Class)}}, then
rename all three to something that reads properly ({{classBelongsToAnyPackage}}
/ {{packageMatchesAny}} as suggested, or similar).
h3. Item 3's premise re-verified
Repository-wide search across {{core}}, {{plugins}}, {{apps}} and {{jakarta}}
for {{isClassBelongsToPackages}}, {{isPackageBelongsToPackages}} and
{{toPackageName}}: no caller outside {{SecurityMemberAccess}} itself. The only
hits are javadoc references in {{SecurityMemberAccessArrayTargetTest}} and
{{SecurityMemberAccessTest}}, plus the same-package
{{SecurityMemberAccessPackageMatchingTest}}, which calls all three directly and
will need updating. The unrelated {{ConfigParseUtil.toPackageNamesSet}} is a
different method.
The constraint about the frozen oracles still applies: {{legacyPrefixMatch}}
and {{legacyToPackageName}} in that test are verbatim copies of the pre-WW-5674
implementation kept as differential oracles, and must continue not to delegate
to production code. WW-5677 added a third such oracle,
{{legacyDefaultPackageCondition}}, under the same rule.
h3. Why it stays parked
{{main}} is {{7.4.0-SNAPSHOT}} and there is no 8.0.0 line. Landing the
narrowing now would commit the next release to being a major, immediately after
7.4.0 absorbed WW-5675/5676/5677 — a major release for a visibility tidy-up.
Revisit when an 8.0.0 branch is cut, or when WW-4759 draws the {{struts2-api}}
boundary; as this ticket argues, deciding what in {{SecurityMemberAccess}} is
genuinely API belongs to that same exercise.
A Migration Guide entry is still owed whenever it does land.
> Tidy up the SecurityMemberAccess package-matching helper names and visibility
> -----------------------------------------------------------------------------
>
> Key: WW-5678
> URL: https://issues.apache.org/jira/browse/WW-5678
> Project: Struts 2
> Issue Type: Improvement
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 8.0.0
>
>
> Follow-up to [WW-5674|https://issues.apache.org/jira/browse/WW-5674],
> deferred to 8.0.0 because part of it is a source-breaking change.
> {panel:title=Check WW-5675 before starting}
> Item 1 below may already be gone. WW-5675 carries a design note proposing
> that the two allowlist sets be replaced by a single precomputed union, which
> would delete the three-argument overload outright — and with it the name
> collision. Confirm the state of WW-5675 before planning any work here.
> {panel}
> h2. 1. The package-private overload shares a name with a public method
> WW-5674 left {{SecurityMemberAccess}} with three related static methods:
> {code:java}
> public static boolean isClassBelongsToPackages(Class clazz, Set
> matchingPackages) // public API
> static boolean isClassBelongsToPackages(Class clazz, Set first, Set
> second) // package-private
> static boolean isPackageBelongsToPackages(String packageName, Set
> first, Set second)
> {code}
> (Generics omitted above — the issue tracker mangles them.)
> The two-argument form is public; the three-argument overload deliberately is
> not, because it has a single caller ({{isClassAllowlisted}}) and its only
> test lives in the same package. Publishing it would freeze it as
> {{struts2-core}} API.
> Sharing a name across the visibility boundary is a hazard: a contributor
> tidying up "an inconsistent modifier" can widen the overload to {{public}} by
> reflex and never realise they have added permanent API surface. Giving the
> package-private overload a distinct name removes the trap. This part is not
> source-breaking and could land earlier if convenient.
> As noted above, WW-5675 may remove this method altogether. The two-set
> parameter exists only because a precomputed union of
> {{ALLOWLIST_REQUIRED_PACKAGES}} and {{allowlistPackageNames}} is not
> currently worthwhile under the prototype bean scope; once WW-5675 makes the
> parsed configuration shared, the union becomes the better shape and the
> overload disappears.
> h2. 2. The names are grammatically awkward
> {{isClassBelongsToPackages}} and {{isPackageBelongsToPackages}} read as "is
> class belongs to packages". Something like {{classBelongsToAnyPackage}} /
> {{packageMatchesAny}} would be clearer. The public one cannot be renamed
> before a major release.
> This item stands regardless of what WW-5675 does.
> h2. 3. Two public statics have no callers outside the class
> {{isClassBelongsToPackages(Class, Set)}} and {{toPackageName(Class)}} are
> both {{public static}} on a public class, but a repository-wide search finds
> no caller outside {{SecurityMemberAccess}} itself — not in core, not in any
> plugin, not in the tests. They appear to be public by accident rather than by
> design.
> Narrowing them to package-private would shrink the surface that has to be
> preserved. This _is_ source-breaking for anyone who happens to call them,
> hence 8.0.0. This item also stands regardless of WW-5675.
> h2. Why this matters now
> WW-4759 is drawing the {{struts2-api}} boundary for 8.0.0. Deciding what in
> {{SecurityMemberAccess}} is genuinely API is part of the same exercise, and
> it is cheaper to settle it while that work is open than to inherit these
> three methods into the new artefact unexamined.
> h2. Constraints
> {{SecurityMemberAccessPackageMatchingTest}} calls all three methods directly
> and will need updating. Any rename must not disturb the frozen
> {{legacyPrefixMatch}} and {{legacyToPackageName}} reference oracles in that
> test — they are verbatim copies of the pre-WW-5674 implementation kept as
> differential oracles, and they must continue to _not_ delegate to production
> code.
> Renaming or narrowing anything public needs a migration-guide entry.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
