You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/03/03 15:47:13 UTC
svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
ExpandWar.java HostConfig.java LocalStrings.properties
Author: markt
Date: Tue Mar 3 14:47:12 2015
New Revision: 1663715
URL: http://svn.apache.org/r1663715
Log:
In directly fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57251
Enable Tomcat to detect when a WAR file has been changed while Tomcat is not running. Note Tomcat does this by setting the last modified time of the expanded directory to the last modified time of the WAR.
Modified:
tomcat/trunk/java/org/apache/catalina/startup/ExpandWar.java
tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java
tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties
Modified: tomcat/trunk/java/org/apache/catalina/startup/ExpandWar.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/ExpandWar.java?rev=1663715&r1=1663714&r2=1663715&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/ExpandWar.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/ExpandWar.java Tue Mar 3 14:47:12 2015
@@ -55,8 +55,7 @@ public class ExpandWar {
/**
* Expand the WAR file found at the specified URL into an unpacked
- * directory structure, and return the absolute pathname to the expanded
- * directory.
+ * directory structure.
*
* @param host Host war is being installed for
* @param war URL of the web application archive to be expanded
@@ -67,31 +66,62 @@ public class ExpandWar {
* WAR file is invalid
* @exception IOException if an input/output error was encountered
* during expansion
+ *
+ * @return The absolute path to the expanded directory foe the given WAR
*/
public static String expand(Host host, URL war, String pathname)
throws IOException {
- // Make sure that there is no such directory already existing
- File docBase = new File(host.getAppBaseFile(), pathname);
- if (docBase.exists()) {
- // War file is already installed
- return (docBase.getAbsolutePath());
- }
-
- // Create the new document base directory
- if(!docBase.mkdir() && !docBase.isDirectory())
- throw new IOException(sm.getString("expandWar.createFailed", docBase));
-
- // Expand the WAR into the new document base directory
- String canonicalDocBasePrefix = docBase.getCanonicalPath();
- if (!canonicalDocBasePrefix.endsWith(File.separator)) {
- canonicalDocBasePrefix += File.separator;
- }
+ // Open the connection to the WAR. There is no explicit close method.
+ // You have to get the JarFile and close that.
JarURLConnection juc = (JarURLConnection) war.openConnection();
juc.setUseCaches(false);
+ // Set up the variables used in the finally block of the following try
boolean success = false;
+ File docBase = new File(host.getAppBaseFile(), pathname);
+
try (JarFile jarFile = juc.getJarFile()) {
+
+ // Get the last modified time for the WAR
+ long warLastModified = juc.getContentLengthLong();
+
+ // Check to see of the WAR has been expanded previously
+ if (docBase.exists()) {
+ // A WAR was expanded. Tomcat will have set the last modified
+ // time of the expanded directory to the last modified time of
+ // the WAR so changes to the WAR while Tomcat is stopped can be
+ // detected
+ long dirLastModified = docBase.lastModified();
+
+ if (dirLastModified == warLastModified) {
+ // No changes to the WAR
+ return (docBase.getAbsolutePath());
+ }
+
+ log.info(sm.getString("expandWar.deleteOld", docBase));
+
+ // WAR must have been modified. Remove expanded directory.
+ if (!delete(docBase)) {
+ throw new IOException(sm.getString("expandWar.deleteFailed", docBase));
+ }
+ }
+
+ // Create the new document base directory
+ if(!docBase.mkdir() && !docBase.isDirectory()) {
+ throw new IOException(sm.getString("expandWar.createFailed", docBase));
+ }
+
+ // Align the last modified time of the directory with the WAR so
+ // changes to the WAR while Tomcat is stopped can be detected
+ docBase.setLastModified(warLastModified);
+
+ // Expand the WAR into the new document base directory
+ String canonicalDocBasePrefix = docBase.getCanonicalPath();
+ if (!canonicalDocBasePrefix.endsWith(File.separator)) {
+ canonicalDocBasePrefix += File.separator;
+ }
+
Enumeration<JarEntry> jarEntries = jarFile.entries();
while (jarEntries.hasMoreElements()) {
JarEntry jarEntry = jarEntries.nextElement();
@@ -119,7 +149,6 @@ public class ExpandWar {
continue;
}
-
try (InputStream input = jarFile.getInputStream(jarEntry)) {
if (null == input)
throw new ZipException(sm.getString("expandWar.missingJarEntry",
Modified: tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java?rev=1663715&r1=1663714&r2=1663715&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java Tue Mar 3 14:47:12 2015
@@ -789,9 +789,17 @@ public class HostConfig
entry = null;
}
+ // If there is an expanded directory then any xml in that directory
+ // should only be used if the directory is not out of date and
+ // unpackWARs is true. Note the code below may apply further limits
+ boolean useXml = false;
+ if (xml.exists() && unpackWARs && xml.lastModified() == war.lastModified()) {
+ useXml = true;
+ }
+
Context context = null;
try {
- if (deployXML && xml.exists() && unpackWARs && !copyXML) {
+ if (deployXML && useXml && !copyXML) {
synchronized (digesterLock) {
try {
context = (Context) digester.parse(xml);
Modified: tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties?rev=1663715&r1=1663714&r2=1663715&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties Tue Mar 3 14:47:12 2015
@@ -76,6 +76,7 @@ engineConfig.stop=EngineConfig: Processi
expandWar.copy=Error copying {0} to {1}
expandWar.createFailed=Unable to create the directory [{0}]
expandWar.deleteFailed=[{0}] could not be completely deleted. The presence of the remaining files may cause problems
+expandWar.deleteOld=An expanded directory [{0}] was found with a last modified time that did not match the associated WAR. It will be deleted.
expandWar.illegalPath=The archive [{0}] is malformed and will be ignored: an entry contains an illegal path [{1}] which was not expanded to [{2}] since that is outside of the defined docBase [{3}]
expandWar.missingJarEntry=Cannot get input stream for JarEntry "{0}" - broken WAR file?
failedContext.start=Failed to process either the global, per-host or context-specific context.xml file therefore the [{0}] Context cannot be started.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
ExpandWar.java HostConfig.java LocalStrings.properties
Posted by Mark Thomas <ma...@apache.org>.
On 03/03/2015 19:14, Christopher Schultz wrote:
> Chuck,
>
> On 3/3/15 1:20 PM, Caldarale, Charles R wrote:
>>> From: Mark Thomas [mailto:markt@apache.org]
>>> Subject: Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
>>> ExpandWar.java HostConfig.java LocalStrings.properties
>>
>>> A bigger issue is anything writing files into the docBase (e.g. some people write
>>> logs here despite it being a bad idea) is going to change the last modified time
>>> of the directory. As much as I like the simplicity of this approach, I think an
>>> alternative is required.
>>
>> As Konstantin noted, the timestamp on a directory is rather ephemeral, especially on Windows or NAS boxes.
>>
>> Can you simply record the deployment occurrence by writing a file with some appropriately Tomcat-specific name into the chosen deployment directory and set the timestamp on that file to match the .war?
>
> Like touching the timestamp of the context.xml file?
That can be modified by the user.
> Or maybe META-INF/context.xml.stamp or whatever.
I like that idea. I was thinking about the work directory but having it
inside the directory of the expanded WAR simplifies undeployment. I'll
probably tweak the file name but I do like the general idea.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1663715 - in
/tomcat/trunk/java/org/apache/catalina/startup: ExpandWar.java
HostConfig.java LocalStrings.properties
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Christopher Schultz [mailto:chris@christopherschultz.net]
> Subject: Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
> ExpandWar.java HostConfig.java LocalStrings.properties
> > Can you simply record the deployment occurrence by writing a file with some
> > appropriately Tomcat-specific name into the chosen deployment directory and
> > set the timestamp on that file to match the .war?
> Like touching the timestamp of the context.xml file? Or maybe
> META-INF/context.xml.stamp or whatever.
Better to use a unique file name that's not part of the current webapp configuration. A simple touch is not appropriate, but setting the timestamp to the .war modification time should work.
- Chuck
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1663715 - in
/tomcat/trunk/java/org/apache/catalina/startup: ExpandWar.java HostConfig.java
LocalStrings.properties
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Chuck,
On 3/3/15 1:20 PM, Caldarale, Charles R wrote:
>> From: Mark Thomas [mailto:markt@apache.org]
>> Subject: Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
>> ExpandWar.java HostConfig.java LocalStrings.properties
>
>> A bigger issue is anything writing files into the docBase (e.g. some people write
>> logs here despite it being a bad idea) is going to change the last modified time
>> of the directory. As much as I like the simplicity of this approach, I think an
>> alternative is required.
>
> As Konstantin noted, the timestamp on a directory is rather ephemeral, especially on Windows or NAS boxes.
>
> Can you simply record the deployment occurrence by writing a file with some appropriately Tomcat-specific name into the chosen deployment directory and set the timestamp on that file to match the .war?
Like touching the timestamp of the context.xml file? Or maybe
META-INF/context.xml.stamp or whatever.
-chris
RE: svn commit: r1663715 - in
/tomcat/trunk/java/org/apache/catalina/startup: ExpandWar.java
HostConfig.java LocalStrings.properties
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Mark Thomas [mailto:markt@apache.org]
> Subject: Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
> ExpandWar.java HostConfig.java LocalStrings.properties
> A bigger issue is anything writing files into the docBase (e.g. some people write
> logs here despite it being a bad idea) is going to change the last modified time
> of the directory. As much as I like the simplicity of this approach, I think an
> alternative is required.
As Konstantin noted, the timestamp on a directory is rather ephemeral, especially on Windows or NAS boxes.
Can you simply record the deployment occurrence by writing a file with some appropriately Tomcat-specific name into the chosen deployment directory and set the timestamp on that file to match the .war?
- Chuck
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
ExpandWar.java HostConfig.java LocalStrings.properties
Posted by Mark Thomas <ma...@apache.org>.
On 03/03/2015 17:30, Mark Thomas wrote:
> On 03/03/2015 15:48, Konstantin Kolinko wrote:
>> 2015-03-03 17:47 GMT+03:00 <ma...@apache.org>:
>>> Author: markt
>>> Date: Tue Mar 3 14:47:12 2015
>>> New Revision: 1663715
>>>
>>> URL: http://svn.apache.org/r1663715
>>> Log:
>>> In directly fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57251
>>
>> You meant "Indirectly".
>
> I did.
>
>> 1. The above ordering of setLastModified vs unpacking is wrong.
>>
>> Adding files to the directory while unpacking the war should change
>> its modification time. Your "setLastModified" call will be invalidated
>> by later changes.
>
> I'll check that and fix it.
>
>> 2. I wonder why I do not see any check for directory time stamp in
>> this commit. It only compares time stamps of xml and war files.
>
> ExpndWar should be correct. HostConfig needs fixing.
>
>> 3. Generally, I am not sure how well setting modification time of a
>> directory works cross-platform. There may be some edge cases here. I
>> am more used to file modification times.
>>
>> Probably modern OSes are OK, but there may be some issues with drives
>> accessed over network. (Just a guess. I do not have an actual
>> example.)
>>
>> This can be solved by allowing to opt-out from this feature - see 4. below.
>
> Absent an actual problem I'd rather not add another configuration option.
>
> I should be able to test if setting the timestamp works and skip the
> test if it doesn't. The only catch is that the code that checks the
> timestamp and the code that sets the timestamp are widely separated at
> the moment. A little refactoring may be required.
>
>> 4. If you are going to backport this,
>
> I intend to backport it to 8 and no further unless there is a demand for
> it from the users.
>
>> I think we need to allow to opt out of last modified check on the
>> expanded directory.
>>
>> Setting the date is OK - we are ignoring the boolean return value of
>> that method, checking the date has to be configurable.
>>
>> Use case:
>> The expanded directory is created via some other legacy method.
>>
>> E.g. by unpacking the war by some configuration tool. There may be
>> some legacy scripts doing such unpacking.
>> Maybe chef recipes, maybe RPMs. Personally I would just pack an
>> expanded directory into the RPM, but I do not know what people will be
>> doing.
>>
>> Someone can do unpacking from a script to save time on subsequent
>> Tomcat startup,
>> or if the same appbase is shared by several Tomcat instances.
>>
>> The script may be not smart enough to read timestamp of a file and set
>> timestamp on the directory. (As a future help: "touch -r reffile"
>> can be used to copy a timestamp from another file).
>>
>>
>> One more example: the war and expanded directory are stored in svn or
>> git. Those do not track time stamps of directories.
>
> Disabling unpackWARs should suffice (or is necessary for correct
> operation in some cases) for those use cases.
>
> The change needs to be clearly documented in the docs, migration guide
> and the release announcement.
Hmm. A bigger issue is anything writing files into the docBase (e.g.
some people write logs here despite it being a bad idea) is going to
change the last modified time of the directory. As much as I like the
simplicity of this approach, I think an alternative is required.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
ExpandWar.java HostConfig.java LocalStrings.properties
Posted by Mark Thomas <ma...@apache.org>.
On 03/03/2015 15:48, Konstantin Kolinko wrote:
> 2015-03-03 17:47 GMT+03:00 <ma...@apache.org>:
>> Author: markt
>> Date: Tue Mar 3 14:47:12 2015
>> New Revision: 1663715
>>
>> URL: http://svn.apache.org/r1663715
>> Log:
>> In directly fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57251
>
> You meant "Indirectly".
I did.
> 1. The above ordering of setLastModified vs unpacking is wrong.
>
> Adding files to the directory while unpacking the war should change
> its modification time. Your "setLastModified" call will be invalidated
> by later changes.
I'll check that and fix it.
> 2. I wonder why I do not see any check for directory time stamp in
> this commit. It only compares time stamps of xml and war files.
ExpndWar should be correct. HostConfig needs fixing.
> 3. Generally, I am not sure how well setting modification time of a
> directory works cross-platform. There may be some edge cases here. I
> am more used to file modification times.
>
> Probably modern OSes are OK, but there may be some issues with drives
> accessed over network. (Just a guess. I do not have an actual
> example.)
>
> This can be solved by allowing to opt-out from this feature - see 4. below.
Absent an actual problem I'd rather not add another configuration option.
I should be able to test if setting the timestamp works and skip the
test if it doesn't. The only catch is that the code that checks the
timestamp and the code that sets the timestamp are widely separated at
the moment. A little refactoring may be required.
> 4. If you are going to backport this,
I intend to backport it to 8 and no further unless there is a demand for
it from the users.
> I think we need to allow to opt out of last modified check on the
> expanded directory.
>
> Setting the date is OK - we are ignoring the boolean return value of
> that method, checking the date has to be configurable.
>
> Use case:
> The expanded directory is created via some other legacy method.
>
> E.g. by unpacking the war by some configuration tool. There may be
> some legacy scripts doing such unpacking.
> Maybe chef recipes, maybe RPMs. Personally I would just pack an
> expanded directory into the RPM, but I do not know what people will be
> doing.
>
> Someone can do unpacking from a script to save time on subsequent
> Tomcat startup,
> or if the same appbase is shared by several Tomcat instances.
>
> The script may be not smart enough to read timestamp of a file and set
> timestamp on the directory. (As a future help: "touch -r reffile"
> can be used to copy a timestamp from another file).
>
>
> One more example: the war and expanded directory are stored in svn or
> git. Those do not track time stamps of directories.
Disabling unpackWARs should suffice (or is necessary for correct
operation in some cases) for those use cases.
The change needs to be clearly documented in the docs, migration guide
and the release announcement.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1663715 - in /tomcat/trunk/java/org/apache/catalina/startup:
ExpandWar.java HostConfig.java LocalStrings.properties
Posted by Konstantin Kolinko <kn...@gmail.com>.
2015-03-03 17:47 GMT+03:00 <ma...@apache.org>:
> Author: markt
> Date: Tue Mar 3 14:47:12 2015
> New Revision: 1663715
>
> URL: http://svn.apache.org/r1663715
> Log:
> In directly fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57251
You meant "Indirectly".
> Enable Tomcat to detect when a WAR file has been changed while Tomcat is not running. Note Tomcat does this by setting the last modified time of the expanded directory to the last modified time of the WAR.
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/startup/ExpandWar.java
> tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java
> tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties
> + // Align the last modified time of the directory with the WAR so
> + // changes to the WAR while Tomcat is stopped can be detected
> + docBase.setLastModified(warLastModified);
> +
> + // Expand the WAR into the new document base directory
1. The above ordering of setLastModified vs unpacking is wrong.
Adding files to the directory while unpacking the war should change
its modification time. Your "setLastModified" call will be invalidated
by later changes.
2. I wonder why I do not see any check for directory time stamp in
this commit. It only compares time stamps of xml and war files.
3. Generally, I am not sure how well setting modification time of a
directory works cross-platform. There may be some edge cases here. I
am more used to file modification times.
Probably modern OSes are OK, but there may be some issues with drives
accessed over network. (Just a guess. I do not have an actual
example.)
This can be solved by allowing to opt-out from this feature - see 4. below.
4. If you are going to backport this,
I think we need to allow to opt out of last modified check on the
expanded directory.
Setting the date is OK - we are ignoring the boolean return value of
that method, checking the date has to be configurable.
Use case:
The expanded directory is created via some other legacy method.
E.g. by unpacking the war by some configuration tool. There may be
some legacy scripts doing such unpacking.
Maybe chef recipes, maybe RPMs. Personally I would just pack an
expanded directory into the RPM, but I do not know what people will be
doing.
Someone can do unpacking from a script to save time on subsequent
Tomcat startup,
or if the same appbase is shared by several Tomcat instances.
The script may be not smart enough to read timestamp of a file and set
timestamp on the directory. (As a future help: "touch -r reffile"
can be used to copy a timestamp from another file).
One more example: the war and expanded directory are stored in svn or
git. Those do not track time stamps of directories.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org