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]

Reply via email to