[ 
https://issues.apache.org/jira/browse/WW-5827?focusedWorklogId=1051620&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1051620
 ]

ASF GitHub Bot logged work on WW-5827:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 31/Oct/26 13:35
            Start Date: 31/Oct/26 13:35
    Worklog Time Spent: 10m 
      Work Description: arunmanni-ai commented on PR #2084:
URL: https://github.com/apache/struts/pull/2084#issuecomment-6544053820

   Hi @lukaszlenart ,
   
   One scope question. In a Struts deployment 
`StrutsPreparerFactory.createPreparer` builds the preparer through the 
`ObjectFactory` and casts to `ViewPreparer` afterwards, so the type check in 
this PR is not reached there — and when the cast fails, `catch (Exception)` 
falls back to `super.createPreparer`, which constructs the class a second time. 
That is the double construction you mentioned; it is not in the ticket 
description.
   
   Two things would fold into the same change. The fallback logs at ERROR with 
a full stack trace on every call, and now that failed lookups are no longer 
cached, a misconfigured preparer name logs that per request rather than once.
   
   Should both go in this PR, or would you rather have a separate ticket? Happy 
either way — I just didn't want to widen this one without asking.




Issue Time Tracking
-------------------

    Worklog Id:     (was: 1051620)
    Time Spent: 20m  (was: 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: 20m
>  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)

Reply via email to