[
https://issues.apache.org/jira/browse/WW-5827?focusedWorklogId=1051619&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1051619
]
ASF GitHub Bot logged work on WW-5827:
--------------------------------------
Author: ASF GitHub Bot
Created on: 31/Oct/26 13:33
Start Date: 31/Oct/26 13:33
Worklog Time Spent: 10m
Work Description: arunmanni-ai opened a new pull request, #2084:
URL: https://github.com/apache/struts/pull/2084
Fixes [WW-5827](https://issues.apache.org/jira/browse/WW-5827)
Addresses the two items in the ticket.
### ClassUtil instantiates before checking the type
`getClass(String, Class)` resolved with `initialize=true`, so the named
class ran its static initialiser on lookup, and `instantiate(String, boolean)`
passed `Object.class` as the base type, which left the `asSubclass` check
unable to fail. `BasicPreparerFactory` cast to `ViewPreparer` only afterwards,
so a configured name that does not denote a `ViewPreparer` was loaded,
initialised and constructed before it was rejected.
The class is now resolved without being initialised, and a new
`instantiate(String, Class<T>, boolean)` applies the caller's base type before
the constructor runs, so a name that does not conform is rejected without
anything being constructed.
A name that resolves but does not conform is reported as
`CannotInstantiateObjectException` rather than returned as `null`. Returning
`null` here would be swallowed by the `ignoreMissing` path in
`BasicTilesContainer.prepare`, turning a misconfigured preparer into a silent
no-op; the exception keeps the failure as visible as the `ClassCastException`
it replaces. `returnNull` continues to mean "class not found" only. Resolution
and construction are caught separately, so an exception thrown by the
constructor itself is never reported as a type mismatch.
The untyped `instantiate` methods are unchanged; `TilesDispatchServlet` is
the only other caller and is migrated to the typed one.
### BasicPreparerFactory caches failed lookups in a non-thread-safe map
`getPreparer` stored whatever `createPreparer` returned, including `null`,
which `containsKey` then reported as resolved. A single failure became
permanent for the lifetime of the factory even when its cause was transient.
Failed lookups are no longer recorded, and the map is a `ConcurrentHashMap`,
since it is read and written by request threads. `get`/`putIfAbsent` rather
than `computeIfAbsent`, so class loading does not run while a bin of the map is
locked.
### Tests
`mvn test -DskipAssembly -pl plugins/tiles` — 572 tests, 0 failures.
New `BasicPreparerFactoryTypeCheckTest` covers rejection before
initialisation and construction, non-retention of a name that cannot be
resolved, and that a genuine `ViewPreparer` is still created and cached. The
evidence flags live in a separate holder class, because reading a static member
of the marker class would itself initialise it. Four cases added to
`ClassUtilTest` for the typed overload.
`PatternUtilTest` is unchanged.
Issue Time Tracking
-------------------
Worklog Id: (was: 1051619)
Remaining Estimate: 0h
Time Spent: 10m
> Tiles plugin: check the preparer type before instantiating it and do not
> cache failed preparer lookups
> ------------------------------------------------------------------------------------------------------
>
> Key: WW-5827
> URL: https://issues.apache.org/jira/browse/WW-5827
> Project: Struts 2
> Issue Type: Improvement
> Components: Plugin - Tiles
> Reporter: Lukasz Lenart
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Two issues in how the Tiles plugin resolves view preparers.
> *ClassUtil instantiates before checking the type*
> {{ClassUtil.getClass(String, Class)}} loads the class with
> {{Class.forName(className, true, classLoader)}}, so static initialisation
> runs on lookup. {{ClassUtil.instantiate(String, boolean)}} passes
> {{Object.class}} as the base type and calls {{newInstance()}}; the
> {{ViewPreparer}} check happens only afterwards, as a cast in
> {{BasicPreparerFactory.createPreparer}}. A configured preparer name that is
> not a {{ViewPreparer}} is therefore loaded, initialised and constructed
> before it is rejected.
> Expected: resolve the class without initialising it, apply the expected base
> type ({{ViewPreparer}}) before instantiation, and fail without constructing
> anything when it does not match.
> *BasicPreparerFactory caches failed lookups in a non-thread-safe map*
> {{BasicPreparerFactory.getPreparer}} stores the result of {{createPreparer}}
> in a plain {{HashMap}}, including the {{null}} returned when the class cannot
> be found. A failed lookup stays cached for the lifetime of the factory, and
> the map is read and written concurrently by request threads.
> Expected: do not cache failed lookups, and use a thread-safe map (e.g.
> {{ConcurrentHashMap}}).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
