2012/2/2 <ma...@apache.org>: > Author: markt > Date: Thu Feb 2 10:32:34 2012 > New Revision: 1239522 > > URL: http://svn.apache.org/viewvc?rev=1239522&view=rev > Log: > Extend clean-up to Server > > Modified: > tomcat/trunk/java/org/apache/catalina/Server.java > tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java > tomcat/trunk/java/org/apache/catalina/core/StandardEngine.java > tomcat/trunk/java/org/apache/catalina/core/StandardServer.java > tomcat/trunk/java/org/apache/catalina/mbeans/MBeanFactory.java > tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java >
Several comments regarding the changes in Tomcat.initBaseDir(): 0. Trivial note: one typo was already fixed in http://svn.apache.org/viewvc?view=revision&revision=1239784 1. Tomcat#setBaseDir() Javadoc says that if system properties are unset the value used is $HOME/tomcat.$PORT. Though further it says "TODO: better default ? Maybe current dir ?" The actual implementation of iniBaseDir() uses System.getProperty("user.dir") which is $PWD. The home directory property name would be "user.home". 2. Old implementation of initBaseDir() updated Tomcat#basedir field. > basedir = home.getCanonicalPath(); That is no longer done. It could be basedir = baseFile.getPath(); though honestly that field has no getters (but it has protected visibility). It was String because the property that it was used to set was String. 3. There are two further bugs in setting catalinaHome besides the one fixed by r1239784: 1) s/setCatalinaBase()/setCatalinaHome()/ 2) If catalina home value is null it should fallback to the value of catalina base. In the old code it was: > System.setProperty(Globals.CATALINA_HOME_PROP, basedir); 4. if (!homeFile.isAbsolute()) checks when setting base and home: I wonder whether "isAbsolute()" check is needed. Though it matches with what the old code was doing. The Bootstrap class (as updated in r1239527) always converts both paths to canonical form unconditionally. The old code in Catalina#initDirs() (removed in r1239527) did conversion conditionally. I think it is safer to convert it to canonical form here as well, and that would be more consistent with bootstrap. I wonder whether there is a use case that requires absolute but non-canonical value here. Anyway we are already using canonical paths in many places internally. If someone expects that he can change absolute->canonical mapping while tomcat is running (e.g. change a symlink) he wouldn't go far with that already. SecurityManager also operates on canonical paths IIRC. So I think it is OK to convert to canonical form unconditionally, though it is slight change in behaviour. Best regards, Konstantin Kolinko > Modified: tomcat/trunk/java/org/apache/catalina/Server.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Server.java?rev=1239522&r1=1239521&r2=1239522&view=diff > > --- tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java (original) > +++ tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java Thu Feb 2 > 10:32:34 2012 > @@ -459,11 +459,12 @@ public class Tomcat { > return server; > } > > - initBaseDir(); > - > System.setProperty("catalina.useNaming", "false"); > > server = new StandardServer(); > + > + initBaseDir(); > + > server.setPort( -1 ); > > service = new StandardService(); > @@ -591,20 +592,31 @@ public class Tomcat { > // Create a temp dir. > basedir = System.getProperty("user.dir") + > "/tomcat." + port; > - File home = new File(basedir); > - home.mkdir(); > - if (!home.isAbsolute()) { > + } > + > + File baseFile = new File(basedir); > + baseFile.mkdirs(); > + if (!baseFile.isAbsolute()) { > + try { > + baseFile = baseFile.getCanonicalFile(); > + } catch (IOException e) { > + baseFile = baseFile.getAbsoluteFile(); > + } > + } > + server.setCatalinaBase(baseFile); > + > + if (catalinaHome == null) { > + File homeFile = new File(catalinaHome); > + homeFile.mkdirs(); > + if (!homeFile.isAbsolute()) { > try { > - basedir = home.getCanonicalPath(); > + homeFile = homeFile.getCanonicalFile(); > } catch (IOException e) { > - basedir = home.getAbsolutePath(); > + homeFile = homeFile.getAbsoluteFile(); > } > } > + server.setCatalinaBase(homeFile); > } > - if (catalinaHome == null) { > - System.setProperty(Globals.CATALINA_HOME_PROP, basedir); > - } > - System.setProperty(Globals.CATALINA_BASE_PROP, basedir); > } > > static final String[] silences = new String[] { > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
