Finally got around to packages my changes as a patch to the HEAD. As previously
discussed, it should be sufficient for a servlet to implement ContainerServlet and for
the class to be placed in the server/lib directory for it to load and initialize
correctly (in a Context where privileged is true). This is currently not the case,
because in the current implementation being placed in the server/lib directory is not
sufficient for the servlet to be loaded by the catalina class loader. The attached
StandardWrapper patch resolves this issue by attempting to load the servlet class in
the catalina class loader if the context is privileged.
________________________________________________________________________
p l u m b d e s i g n
alan newberger
chief architect
157 chambers st ny ny 10007
p.212.381.0541 | f.212.285.8999
> -----Original Message-----
> From: Alan Newberger
> Sent: Friday, January 11, 2002 1:52 PM
> To: Tomcat Developers List
> Subject: RE: shell connector
>
>
> thanks for your reply, I still feel there is a problem with
> ContainerServlets from arbitrary packages even when placed in
> $CATALINA_HOME/server/lib, see below...
>
> > From: Craig R. McClanahan [mailto:[EMAIL PROTECTED]]
> > I committed some Ant custom tasks yesterday that talk to
> > Manager ... they
> > might be useful to you.
>
> I'll check them out, thanks.
>
>
> > So that one can write servlet-based services that need access
> > to Catalina
> > internals. Existing examples include:
> > * Manager servlet (needs to install, remove, and reload webapps)
> > * Invoker servlet (needs to create new Wrapper instances on the fly)
> > * Administration servlet (will provide web-based user interface for
> > updating pretty much all of the things in server.xml).
> >
> > The third thing is in the "admin" webapp in the HEAD branch.
> >
>
> That also sounds really useful, I'm going to have to take a
> close look at the HEAD branch :) Hopefully it's capable of
> persisting changes back into server.xml...
>
> > In order to execute a ContainerServlet that has access to the
> > internals of
> > Tomcat 4, the following rules must all be followed:
> >
> > * Your servlet must implement the
> org.apache.catalina.ContainerServlet
> > interface.
> >
> > * Your servlet, and all the code it depends on, must be
> > loaded from the
> > "server" classloader. That means it needs to be installed
> > as unpacked
> > classes under $CATALINA_HOME/server/classes or in JAR files in
> > $CATALINA_HOME/server/lib. (Classes in common/classes and
> > common/lib
> > are also visible to these components.)
> >
> > No changes to Tomcat code, or to standard server.xml settings, are
> > required.
> >
>
> IF a servlet which implements ContainerServlet is loaded by
> the Catalina class loader, everything will work fine, i.e. it
> will have its 'setWrapper()' command called by
> StandardWrapper.load(). The problem is in the way
> StandardWrapper decides which classloader to use to load the
> servlet. It uses its private 'isContainerServlet(string)'
> method, which only looks at the String representation of the
> className and checks if it starts with 'org.apache.catalina'.
> So, if I have a servlet implementing ContainerServlet in
> some package, say 'com.foo.servlets', and put it in a jar
> $CATALINA_HOME/server/lib, and create some webapp with a
> web.xml with com.foo.servlets.SomeContainerServlet as a
> 'server-class' for a servlet, things will still fail. The
> StandardWrapper.isContainerServlet(string classname) method
> will return false, so the StandardWrapper.load() method will
> try to load the class from the webapp classloader, which of
> course will not find the class since it is not in the webapp
> classpath. You can put the
> com.foo.servlets.SomeContainerServlet in the common/lib
> folder and it will load correctly, but things will break
> because no catalina classes will be visible to it (an object
> instance cannot see other objects loaded in _child_
> classloaders, only _parent_ classloaders).
>
> The solution to this isn't trivial -- you want all
> ContainerServlets to be loaded in the catalina classloader,
> but how do you know whether a servlet is a ContainerServlet
> by its classname alone, before actually loading it and doing
> something like 'instanceof' or 'isAssignableFrom()'? I'm
> sure this was why the StandardWrapper.isContainerServlet()
> method was written but it just doesn't do the right thing. I
> think the way to a nicer solution is through the 'privileged'
> attribute for a Context in servlet.xml (evaluated in
> ServletWrapper via the isServletAllowed() function). I
> suggest that if a Context is designated as privileged, the
> context's wrapper should attempt to load servlets via the
> catalina classloader. If the class isn't found, catch the
> exception and attempt to load it via the webapp classloader.
> If the class is loaded, check if it implements
> ContainerServlet; if so, call setWrapper() on it and
> continue, if not destroy the class object and attempt to load
> it in the webapp classloader. This is a bit more expensive
> way of loading classes than what is done now, but if only
> performed in Contexts marked 'privileged', most Context will
> be unaffected and privileged Contexts would actually work :)
> I'm fairly sure I'm right about this, I've read the code
> pretty carefully and tested out the above behavior (i.e. I've
> previously tried doing the two things you outline as
> necessary above and things failed, it was a mystery until I
> delved into StandardWrapper source). I'd be happy to code up
> these proposed changes into StandardWrapper if you're interested.
>
> -alan
>
> --
> To unsubscribe, e-mail:
> <mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail:
> <mailto:[EMAIL PROTECTED]>
>
>
Index: StandardWrapper.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core/StandardWrapper.java,v
retrieving revision 1.35
diff -u -r1.35 StandardWrapper.java
--- StandardWrapper.java 11 Dec 2001 18:56:03 -0000 1.35
+++ StandardWrapper.java 22 Jan 2002 18:59:00 -0000
@@ -851,34 +851,44 @@
}
ClassLoader classLoader = loader.getClassLoader();
+ Class classClass = null;
- // Special case class loader for a Catalina internal servlet
- if (isContainerServlet(actualClass)) {
- classLoader = this.getClass().getClassLoader();
- log(sm.getString
- ("standardWrapper.containerServlet", getName()));
- }
+ //if privileged, try loading in catalina classloader
+ if (((Context) getParent()).getPrivileged()) {
+ try {
+ classLoader = this.getClass().getClassLoader();
+ Class sClass = classLoader.loadClass(actualClass);
+ if (ContainerServlet.class.isAssignableFrom(sClass)) {
+ classClass = sClass;
+ }
+
+ log(sm.getString("standardWrapper.containerServlet", getName()));
+ } catch (ClassNotFoundException e) {}
+ }
+
// Load the specified servlet class from the appropriate class loader
- Class classClass = null;
- try {
- if (classLoader != null) {
- classClass = classLoader.loadClass(actualClass);
- } else {
- classClass = Class.forName(actualClass);
+ if (classClass == null) {
+ try {
+ if (classLoader != null) {
+ classClass = classLoader.loadClass(actualClass);
+ } else {
+ classClass = Class.forName(actualClass);
+ }
+ } catch (ClassNotFoundException e) {
+ unavailable(null);
+ throw new ServletException
+ (sm.getString("standardWrapper.missingClass", actualClass),
+ e);
}
- } catch (ClassNotFoundException e) {
- unavailable(null);
- throw new ServletException
- (sm.getString("standardWrapper.missingClass", actualClass),
- e);
}
+
if (classClass == null) {
unavailable(null);
throw new ServletException
(sm.getString("standardWrapper.missingClass", actualClass));
}
-
+
// Instantiate and initialize an instance of the servlet class itself
Servlet servlet = null;
try {
@@ -903,9 +913,10 @@
actualClass));
}
- // Special handling for ContainerServlet instances
+ // Special handling for ContainerServlet instances, if loaded by
+ // this classloader
if ((servlet instanceof ContainerServlet) &&
- isContainerServlet(actualClass)) {
+ classLoader == this.getClass().getClassLoader()) {
((ContainerServlet) servlet).setWrapper(this);
}
@@ -1197,22 +1208,6 @@
protected void addDefaultMapper(String mapperClass) {
; // No need for a default Mapper on a Wrapper
-
- }
-
-
- /**
- * Return <code>true</code> if the specified class name represents a
- * container class that should be loaded by the system class loader.
- *
- * @param name Name of the class to be checked
- */
- private boolean isContainerServlet(String classname) {
-
- if (classname.startsWith("org.apache.catalina."))
- return (true);
- else
- return (false);
}
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
