On Today at 11:06am, RM=>Reinhard Moosauer <[EMAIL PROTECTED]> wrote:
RM> Hi Filip, Haroon, RM> RM> as far as I understand, the problem is a copy operation which copies one file RM> on itself. RM> Haroon showed that this is happing in his case. RM> Unfortunately, it it not proved that this is happening in all cases. RM> Furthermore, the removal of the second copy operation could still cause a RM> regression in other cases. RM> Hi Reinhard, I'm obviously not very familiar with the tomcat code and by no means an expert so I didn't realize that there was copy protection involved. In any case, all of the involved code only works if tag is non-null. To my knowledge, I'm the only one who uses remote war deployment while using tags, and the empirical evidence that I have suggests that the 2nd copy operation is not necessary (it could have even been a copy-and-paste error). RM> RM> Maybe we could fix the bug in a more secure way: RM> How about adding a check to the copy()-method, so that it can no more delete a RM> file by copying on itself? RM> We could: RM> 1. just ignore the copy if the 2 paths are the same RM> 2. throw an IllegalArgumentException if the 2 paths are the same RM> Gotta be careful here. When I said the 2 paths are same, I generalized a little bit. The two paths are almost the same. They are: /www/apache-tomcat-5.5.16/webapps/sws.war /www/tomcat/webapps/sws.war One of them involves a symlink. They are the same as long as you consider the fact that /www/tomcat is a symbolic link to apache-tomcat-5.5.16: lrwxrwxrwx 1 root root 20 Mar 21 14:33 /www/tomcat -> apache-tomcat-5.5.16 So, my (non-binding) vote :-), would be to: 1) Remove the 2nd copy() method all together 2) Clean up copy() method to throw an IllegalArgumentException if the 2 paths are the same while *ALSO* taking into account any symbolic links. RM> RM> Personally, I would prefer the second solution, because it make errors RM> like this one better to find in the future. RM> The two code-lines removed by Haroon could have a try-catch-block to RM> ignore the IllegArgExc here (with a bold FIXME-Message to the log!) RM> RM> *** RM> I suggest to be very careful in changing this part of tomcat. This RM> case proves again, that the test-coverage in the manager code is not RM> pretty good. Mainly because there are many configuration and RM> deployment scenarios, which affect its behavior. (I suffered a similar RM> case some days ago) RM> *** RM> I agree about test-coverage. I fully appreciate and understand the need for caution. However, by their own admission, most tomcat developers do not have the time to test the remote deployment while tagging via ant. To my knowledge, I am the only who uses this in production with tomcat 5.5.12+ (since I'm the only one complaining about it for the last 6 months or so). Combined with the fact that the whole operation is inside a if (tag != null) {} leads me to believe that it is safe to take out the 2nd copy() method. Believe me, with the 2nd copy() method in tact, remote deploying via ant while tagging does *NOT* work. RM> RM> Please tell, if I can help RM> RM> Reinhard RM> You have helped already by replying. Regards, -- Haroon Rafique <[EMAIL PROTECTED]> --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]