[
https://issues.apache.org/jira/browse/WW-5827?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5827:
------------------------------
Description:
Three 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}}).
*StrutsPreparerFactory constructs before checking the type, twice on failure,
and logs every failure at ERROR*
{{StrutsPreparerFactory.createPreparer}} builds the preparer through
{{ObjectFactory.buildBean}} and casts to {{ViewPreparer}} afterwards, so the
type check above is not applied on this path. When the cast fails, the {{catch
(Exception)}} falls back to {{super.createPreparer}}, which constructs the
class a second time. The fallback logs at ERROR with a full stack trace; once
failed lookups are no longer cached, a misconfigured preparer name would log
that on every request instead of once.
Expected: check the type before building the bean, construct at most once, and
keep a failed lookup from producing a full ERROR stack trace per request.
was:
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}}).
> 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: 40m
> Remaining Estimate: 0h
>
> Three 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}}).
> *StrutsPreparerFactory constructs before checking the type, twice on failure,
> and logs every failure at ERROR*
> {{StrutsPreparerFactory.createPreparer}} builds the preparer through
> {{ObjectFactory.buildBean}} and casts to {{ViewPreparer}} afterwards, so the
> type check above is not applied on this path. When the cast fails, the
> {{catch (Exception)}} falls back to {{super.createPreparer}}, which
> constructs the class a second time. The fallback logs at ERROR with a full
> stack trace; once failed lookups are no longer cached, a misconfigured
> preparer name would log that on every request instead of once.
> Expected: check the type before building the bean, construct at most once,
> and keep a failed lookup from producing a full ERROR stack trace per request.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
