|
»
« Return to Thread: Patch - Don't follow symlinks for deletion
Re: Patch - Don't follow symlinks for deletion
by jasonb
::
Rate this Message:
Michael:
It sounds like a good change. I have not tried the patch (BTW: I
believe that FarmWarDeployer is now considered
unsupported/deprecated), but the theory here sounds right. I think it
makes sense to consolidate the implementations of the code that
removes the files, if they are basically the same (it appears that
they are). I also would expect Tomcat to just remove the symlink to
an external file or directory, instead of following the symlink and
deleting the external file or dir as well. That was probably
unintentional behavior.
Thanks for the investigation and the patch!
--
Jason Brittain
On Tue, Jul 15, 2008 at 10:06 AM, Michael Bailey
<michaelbailey0@...> wrote:
> Can someone in the know comment on this? Is it intended behavior for tomcat
> to follow symlinks when deleting the exploded directory during undeploy? Or
> should I file a bug and attached this patch to it?
>
> Michael Bailey
>
> On Thu, Jul 3, 2008 at 11:27 AM, Michael Bailey <michaelbailey0@...>
> wrote:
>
>> Patch pasted in case the attachement didn't make throught the list:
>>
>> Index: java/org/apache/tomcat/util/FileUtils.java
>> ===================================================================
>> --- java/org/apache/tomcat/util/FileUtils.java (revision 0)
>> +++ java/org/apache/tomcat/util/FileUtils.java (revision 0)
>> @@ -0,0 +1,55 @@
>> +package org.apache.tomcat.util;
>> +
>> +import java.io.File;
>> +import java.io.IOException;
>> +
>> +
>> +public class FileUtils {
>> +
>> + /**
>> + * Delete dir recursively never following symlinks
>> + * @param dir
>> + */
>> + public static boolean deleteDir(File dir) {
>> + boolean mightBeSymlink;
>> + try {
>> + mightBeSymlink = isSymLink(dir);
>> + } catch (IOException e) {
>> + mightBeSymlink = true;
>> + }
>> +
>> + if (mightBeSymlink) {
>> + return dir.delete();
>> + } else {
>> + String files[] = dir.list();
>> + if (files == null) {
>> + files = new String[0];
>> + }
>> + for (int i = 0; i < files.length; i++) {
>> + File file = new File(dir, files[i]);
>> + if (file.isDirectory()) {
>> + deleteDir(file);
>> + } else {
>> + file.delete();
>> + }
>> + }
>> + return dir.delete();
>> + }
>> +
>> + }
>> +
>> + /**
>> + * Trys to determine if the last path component is a symlink
>> + *
>> + * @param file
>> + * @return true if it is a symlink
>> + * @throws IOException - if we run into trouble examining the
>> file, the file may or may not be a symlink
>> + * if this exception is thrown.
>> + */
>> + public static boolean isSymLink(File file) throws IOException {
>> + String path1 =
>> (file.getAbsoluteFile().getParentFile().getCanonicalPath() +
>> File.separatorChar + file.getName());
>> + String path2 = (file.getAbsoluteFile().getCanonicalPath());
>> + return !path1.equals(path2);
>> + }
>> +
>> +}
>> Index: java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
>> ===================================================================
>> --- java/org/apache/catalina/ha/deploy/FarmWarDeployer.java (revision
>> 672932)
>> +++ java/org/apache/catalina/ha/deploy/FarmWarDeployer.java (working
>> copy)
>> @@ -36,6 +36,7 @@
>> import org.apache.catalina.ha.ClusterMessage;
>> import org.apache.catalina.tribes.Member;
>> import org.apache.tomcat.util.modeler.Registry;
>> +import org.apache.tomcat.util.FileUtils;
>>
>>
>> /**
>> @@ -580,21 +581,7 @@
>> * File object representing the directory to be deleted
>> */
>> protected void undeployDir(File dir) {
>> -
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - undeployDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - dir.delete();
>> -
>> + FileUtils.deleteDir(dir);
>> }
>>
>> /*
>> Index: java/org/apache/catalina/startup/ExpandWar.java
>> ===================================================================
>> --- java/org/apache/catalina/startup/ExpandWar.java (revision 672932)
>> +++ java/org/apache/catalina/startup/ExpandWar.java (working copy)
>> @@ -35,6 +35,7 @@
>> import org.apache.catalina.util.StringManager;
>> import org.apache.juli.logging.Log;
>> import org.apache.juli.logging.LogFactory;
>> +import org.apache.tomcat.util.FileUtils;
>>
>> /**
>> * Expand out a WAR in a Host's appBase.
>> @@ -274,21 +275,7 @@
>> * @param dir File object representing the directory to be deleted
>> */
>> public static boolean deleteDir(File dir) {
>> -
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - deleteDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - return dir.delete();
>> -
>> + return FileUtils.deleteDir(dir);
>> }
>>
>>
>> Index: java/org/apache/catalina/loader/WebappClassLoader.java
>> ===================================================================
>> --- java/org/apache/catalina/loader/WebappClassLoader.java (revision
>> 672932)
>> +++ java/org/apache/catalina/loader/WebappClassLoader.java (working
>> copy)
>> @@ -63,6 +63,7 @@
>> import org.apache.naming.resources.Resource;
>> import org.apache.naming.resources.ResourceAttributes;
>> import org.apache.tomcat.util.IntrospectionUtils;
>> +import org.apache.tomcat.util.FileUtils;
>>
>> /**
>> * Specialized web application class loader.
>> @@ -2330,21 +2331,8 @@
>> * @param dir File object representing the directory to be deleted
>> */
>> protected static void deleteDir(File dir) {
>> + FileUtils.deleteDir(dir);
>>
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - deleteDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - dir.delete();
>> -
>> }
>>
>>
>> Index: java/org/apache/catalina/manager/ManagerServlet.java
>> ===================================================================
>> --- java/org/apache/catalina/manager/ManagerServlet.java (revision
>> 672932)
>> +++ java/org/apache/catalina/manager/ManagerServlet.java (working
>> copy)
>> @@ -59,6 +59,7 @@
>> import org.apache.catalina.util.ServerInfo;
>> import org.apache.catalina.util.StringManager;
>> import org.apache.tomcat.util.modeler.Registry;
>> +import org.apache.tomcat.util.FileUtils;
>>
>>
>> /**
>> @@ -1508,21 +1509,7 @@
>> * @param dir File object representing the directory to be deleted
>> */
>> protected void undeployDir(File dir) {
>> -
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - undeployDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - dir.delete();
>> -
>> + FileUtils.deleteDir(dir);
>> }
>>
>>
>>
>>
>> On Thu, Jul 3, 2008 at 11:17 AM, Michael Bailey
>> <michaelbailey0@...> wrote:
>> > By default tomcat, when tomcat undeploys a war it deletes the exploded
>> > directory and follows symlinked directories recursively, whether
>> > allowLinking="true" or not. Is this the intended behavior? I have a
>> > use case in which I want to have allowLinking="true" for my context,
>> > but when the exploded directory gets deleted during undeploy I don't
>> > want it to delete the target of the symlink only the symlink itself.
>> > The recursive delete code was found in 4 places in the code. I
>> > replaced it with a call to a utility method. I have attached a patch
>> > that disables following of symlinks for deletion. It has only been
>> > tested with JDK 5 on Linux.
>> >
>> > I am curious to know the details for the use case in which you want
>> > tomcat to follow symlinks for deletion to be the default behavior.
>> >
>> > Michael Bailey
>> >
>>
>
--
Jason Brittain
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...
It sounds like a good change. I have not tried the patch (BTW: I
believe that FarmWarDeployer is now considered
unsupported/deprecated), but the theory here sounds right. I think it
makes sense to consolidate the implementations of the code that
removes the files, if they are basically the same (it appears that
they are). I also would expect Tomcat to just remove the symlink to
an external file or directory, instead of following the symlink and
deleting the external file or dir as well. That was probably
unintentional behavior.
Thanks for the investigation and the patch!
--
Jason Brittain
On Tue, Jul 15, 2008 at 10:06 AM, Michael Bailey
<michaelbailey0@...> wrote:
> Can someone in the know comment on this? Is it intended behavior for tomcat
> to follow symlinks when deleting the exploded directory during undeploy? Or
> should I file a bug and attached this patch to it?
>
> Michael Bailey
>
> On Thu, Jul 3, 2008 at 11:27 AM, Michael Bailey <michaelbailey0@...>
> wrote:
>
>> Patch pasted in case the attachement didn't make throught the list:
>>
>> Index: java/org/apache/tomcat/util/FileUtils.java
>> ===================================================================
>> --- java/org/apache/tomcat/util/FileUtils.java (revision 0)
>> +++ java/org/apache/tomcat/util/FileUtils.java (revision 0)
>> @@ -0,0 +1,55 @@
>> +package org.apache.tomcat.util;
>> +
>> +import java.io.File;
>> +import java.io.IOException;
>> +
>> +
>> +public class FileUtils {
>> +
>> + /**
>> + * Delete dir recursively never following symlinks
>> + * @param dir
>> + */
>> + public static boolean deleteDir(File dir) {
>> + boolean mightBeSymlink;
>> + try {
>> + mightBeSymlink = isSymLink(dir);
>> + } catch (IOException e) {
>> + mightBeSymlink = true;
>> + }
>> +
>> + if (mightBeSymlink) {
>> + return dir.delete();
>> + } else {
>> + String files[] = dir.list();
>> + if (files == null) {
>> + files = new String[0];
>> + }
>> + for (int i = 0; i < files.length; i++) {
>> + File file = new File(dir, files[i]);
>> + if (file.isDirectory()) {
>> + deleteDir(file);
>> + } else {
>> + file.delete();
>> + }
>> + }
>> + return dir.delete();
>> + }
>> +
>> + }
>> +
>> + /**
>> + * Trys to determine if the last path component is a symlink
>> + *
>> + * @param file
>> + * @return true if it is a symlink
>> + * @throws IOException - if we run into trouble examining the
>> file, the file may or may not be a symlink
>> + * if this exception is thrown.
>> + */
>> + public static boolean isSymLink(File file) throws IOException {
>> + String path1 =
>> (file.getAbsoluteFile().getParentFile().getCanonicalPath() +
>> File.separatorChar + file.getName());
>> + String path2 = (file.getAbsoluteFile().getCanonicalPath());
>> + return !path1.equals(path2);
>> + }
>> +
>> +}
>> Index: java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
>> ===================================================================
>> --- java/org/apache/catalina/ha/deploy/FarmWarDeployer.java (revision
>> 672932)
>> +++ java/org/apache/catalina/ha/deploy/FarmWarDeployer.java (working
>> copy)
>> @@ -36,6 +36,7 @@
>> import org.apache.catalina.ha.ClusterMessage;
>> import org.apache.catalina.tribes.Member;
>> import org.apache.tomcat.util.modeler.Registry;
>> +import org.apache.tomcat.util.FileUtils;
>>
>>
>> /**
>> @@ -580,21 +581,7 @@
>> * File object representing the directory to be deleted
>> */
>> protected void undeployDir(File dir) {
>> -
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - undeployDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - dir.delete();
>> -
>> + FileUtils.deleteDir(dir);
>> }
>>
>> /*
>> Index: java/org/apache/catalina/startup/ExpandWar.java
>> ===================================================================
>> --- java/org/apache/catalina/startup/ExpandWar.java (revision 672932)
>> +++ java/org/apache/catalina/startup/ExpandWar.java (working copy)
>> @@ -35,6 +35,7 @@
>> import org.apache.catalina.util.StringManager;
>> import org.apache.juli.logging.Log;
>> import org.apache.juli.logging.LogFactory;
>> +import org.apache.tomcat.util.FileUtils;
>>
>> /**
>> * Expand out a WAR in a Host's appBase.
>> @@ -274,21 +275,7 @@
>> * @param dir File object representing the directory to be deleted
>> */
>> public static boolean deleteDir(File dir) {
>> -
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - deleteDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - return dir.delete();
>> -
>> + return FileUtils.deleteDir(dir);
>> }
>>
>>
>> Index: java/org/apache/catalina/loader/WebappClassLoader.java
>> ===================================================================
>> --- java/org/apache/catalina/loader/WebappClassLoader.java (revision
>> 672932)
>> +++ java/org/apache/catalina/loader/WebappClassLoader.java (working
>> copy)
>> @@ -63,6 +63,7 @@
>> import org.apache.naming.resources.Resource;
>> import org.apache.naming.resources.ResourceAttributes;
>> import org.apache.tomcat.util.IntrospectionUtils;
>> +import org.apache.tomcat.util.FileUtils;
>>
>> /**
>> * Specialized web application class loader.
>> @@ -2330,21 +2331,8 @@
>> * @param dir File object representing the directory to be deleted
>> */
>> protected static void deleteDir(File dir) {
>> + FileUtils.deleteDir(dir);
>>
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - deleteDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - dir.delete();
>> -
>> }
>>
>>
>> Index: java/org/apache/catalina/manager/ManagerServlet.java
>> ===================================================================
>> --- java/org/apache/catalina/manager/ManagerServlet.java (revision
>> 672932)
>> +++ java/org/apache/catalina/manager/ManagerServlet.java (working
>> copy)
>> @@ -59,6 +59,7 @@
>> import org.apache.catalina.util.ServerInfo;
>> import org.apache.catalina.util.StringManager;
>> import org.apache.tomcat.util.modeler.Registry;
>> +import org.apache.tomcat.util.FileUtils;
>>
>>
>> /**
>> @@ -1508,21 +1509,7 @@
>> * @param dir File object representing the directory to be deleted
>> */
>> protected void undeployDir(File dir) {
>> -
>> - String files[] = dir.list();
>> - if (files == null) {
>> - files = new String[0];
>> - }
>> - for (int i = 0; i < files.length; i++) {
>> - File file = new File(dir, files[i]);
>> - if (file.isDirectory()) {
>> - undeployDir(file);
>> - } else {
>> - file.delete();
>> - }
>> - }
>> - dir.delete();
>> -
>> + FileUtils.deleteDir(dir);
>> }
>>
>>
>>
>>
>> On Thu, Jul 3, 2008 at 11:17 AM, Michael Bailey
>> <michaelbailey0@...> wrote:
>> > By default tomcat, when tomcat undeploys a war it deletes the exploded
>> > directory and follows symlinked directories recursively, whether
>> > allowLinking="true" or not. Is this the intended behavior? I have a
>> > use case in which I want to have allowLinking="true" for my context,
>> > but when the exploded directory gets deleted during undeploy I don't
>> > want it to delete the target of the symlink only the symlink itself.
>> > The recursive delete code was found in 4 places in the code. I
>> > replaced it with a call to a utility method. I have attached a patch
>> > that disables following of symlinks for deletion. It has only been
>> > tested with JDK 5 on Linux.
>> >
>> > I am curious to know the details for the use case in which you want
>> > tomcat to follow symlinks for deletion to be the default behavior.
>> >
>> > Michael Bailey
>> >
>>
>
--
Jason Brittain
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...
« Return to Thread: Patch - Don't follow symlinks for deletion
| Free embeddable forum powered by Nabble | Forum Help |
