[
https://issues.apache.org/jira/browse/JXPATH-127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12676537#action_12676537
]
John Trimble commented on JXPATH-127:
-------------------------------------
Thank you for taking the time to respond to my post. I understand and
appreciate your concern regarding the first patch I attached, and I apologize
for the trouble. I looked over Roubtsov's code and the only thing taken from
him code-wise is the use of a dummy SecurityManager to get the call stack. This
isn't necessary so I removed it and uploaded a new patch. Of course, the patch
still uses the same approach to class loading as described in the article
(which may be the root of your IP concern). If you still feel there are IP
issues, I can contact Roubtsov about an IP grant. However, I've included 2
additional patches, based on your suggestion of using [lang]'s code, one of
which might work better anyways.
The patch jxpath-14-classloading-lang-port.patch is, for the most part, a
verbatim port of [lang]'s class loading code. I made some modifications to
remove dependence on StringUtil. It also includes a test case. My concern with
using [lang]'s class loading code is that, while it solves the original
problem, it may cause backwards compatibility issues. Specifically, anyone
currently using [jxpath] in an application where the context class loader is
set will now have jxpath attempting to load their classes through the context
class loader (which may not have access to them) instead of the class loader
that loaded jxpath as it used to (the jxpath-14-classloading-widest-scope.patch
patch solves this problem by choosing the class loader with the widest scope).
The patch jxpath-14-classloading-lang-port-with-fallback.patch is also a port
of [lang]'s code but modified to use the current class loader (the class loader
that loaded ClassLoaderUtil) if the context class loader throws a
ClassNotFoundException. This should make the class loading change to [jxpath]
transparent to those already using it, and essentially achieves the same goal
as the jxpath-14-classloading-widest-scope.patch patch. So either patch
jxpath-14-classloading-widest-scope.patch or
jxpath-14-classloading-lang-port-with-fallback.patch would be best, in my
opinion, but any of them should solve the problem. I have not marked any of
them for inclusion due to the IP concerns with the first and the other two
being ports of code, not written by me, from [lang]. I can change that, but I
wanted to err on the side of caution this time. Let me know what you think and
thanks again.
> Change dynamic class loading to consult context class loader.
> -------------------------------------------------------------
>
> Key: JXPATH-127
> URL: https://issues.apache.org/jira/browse/JXPATH-127
> Project: Commons JXPath
> Issue Type: Improvement
> Reporter: John Trimble
> Fix For: post-1.3
>
> Attachments: classloading_fix_jxpath14.patch,
> jxpath-14-classloading-lang-port-with-fallback.patch,
> jxpath-14-classloading-lang-port.patch,
> jxpath-14-classloading-widest-scope.patch
>
>
> For dynamically loading classes, JXPath currently uses Class.forName(...).
> This means all classes loaded by JXPath must be visible to whatever class
> loader loaded JXPath. This is restrictive in large frameworks and web
> applications where multiple class loaders are in use. In particular, in an
> EJB3 application deployed on Weblogic, the shared jars between different EJB
> modules are loaded with a separate class loader than the modules themselves.
> Consequently, if the jxpath.jar is among the general libraries for the
> application, then a specific EJB module will be unable to reference static
> methods of classes within its own module in an xpath expression. For example,
> consider the following lines of code in an EJB module class:
> JXPathContext context = JXPathContext.newContext(new Object());
> Object value = context.getValue("SomeClass.getSomething()");
> This will fail, with a ClassNotFoundException, if SomeClass is a class in the
> module, but jxpath is a general library for the application.
> Attached (or will be once I figure out how) is a patch for JXPath that gets
> around this issue. The patch changes the class loading strategy to use the
> context class loader, when it appears appropriate, to dynamically load
> classes. The strategy is based on suggestions and example code from:
> http://www.javaworld.com/javaworld/javaqa/2003-06/01-qa-0606-load.html
> Assuming all class loaders involved delegate to their parent class loader (as
> recommended in the API), currently working code, that uses JXPath, should
> only be affected by this change if the current class loader (the class loader
> that loaded JXPath) is not in an ancestor/descendant relationship with the
> context class loader.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
