[jira] [Commented] (LOG4J2-604) Audit use of ClassLoader, Class.forName, etc.

["Matt Sicker (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LOG4J2-604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13969776#comment-13969776 ] 

Matt Sicker commented on LOG4J2-604:
------------------------------------

I went through and did a bunch of work on this last night. The thing is, now I have a question as to how we want to proceed with this.

In the original {{Loader.loadClass}} implementation, it would use {{getTCCL().loadClass(foo)}} by default (which loads the class but does not initialize it). If the TCCL was disabled, it would use {{Class.forName(foo)}} which *does* initialize the class (you'd have to use {{Class.forName(foo, false, someClassLoader)}} to get the same behavior as {{someClassLoader.loadClass(foo)}}). Essentially, you get different behavior besides which class loader you want!

What I've changed is now if you use the TCCL, it uses {{Class.forName(foo, true, getTCCL()}} to make the behavior consistent with the default {{Class.forName(foo)}} method. The thing about specifying initialization is that on init, it might need to load more classes that haven't been loaded already. If this should happen under *that* thread context (and not some later one which might be different if the class is initialized in another thread), then you open up pandoras box with incompatible class casts and such.

Anyway, I think the new behavior is more correct, so I've also updated all the usages of Class.forName to use an appropriate method from Loader. I've also added a few methods to Loader for convenience.

> Audit use of ClassLoader, Class.forName, etc.
> ---------------------------------------------
>
>                 Key: LOG4J2-604
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-604
>             Project: Log4j 2
>          Issue Type: Epic
>          Components: API, Core
>    Affects Versions: 2.0-rc2
>            Reporter: Matt Sicker
>            Priority: Blocker
>
> The idiom {{Class.forName}} is almost always a bad idea if it's called without a classloader to go along with it. The only acceptable place to put it is in something like Loader.loadClass as a last resort.
> To make sure everything works as expected in non-trivial environments (e.g., multiple LoggerContexts associated to completely different ClassLoaders like in webapps or bundles), all usage of dynamic class loading should be audited for correctness. The appropriate neighbour class can be used for getting a class loader in most cases (i.e., another already loaded class that should be from the same JAR).
> I'll try to add some integration tests that create sub-classloaders that isolate contexts from one another to ensure correctness.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org