https://issues.apache.org/bugzilla/show_bug.cgi?id=54007
--- Comment #2 from Konstantin Kolinko <knst.koli...@gmail.com> --- Concerns about the patch 1) Messy. The new methods have to be documented better and to be made protected instead of private. 2) Several concerns about getContextPathFromFileName() method: - It needs better API. In the patch it combines context name calculation, canonicalization check and update of invalidWars map. Originally the check was supposed to be performed for war files only. - Typo in getContextPathFromFileName() JavaDoc: a Cyrillic char instead of trailing '.' - A bug: The "appbase" argument is not used because of a typo. The method calls validateContextPath(appBase,..) using this.appBase instead of "appbase" argument. - The validation check that uses canonicalization is expensive. It could be improved by reordering the checks. Reordering cannot be done if the check is encapsulated in this method. See http://svn.apache.org/viewvc?view=revision&revision=1380891 3) HostConfig.invalidWars is a HashSet. I think it has to be made final and access to it has to be made synchonized. 4) As noted by kfujino in STATUS.txt of Tomcat 6 where this patch was proposed: Question of if host.addChild(context) threw IllegalStateException. E.g. case of deployDirectory. If META-INF/context.xml exist in Directory, context.xml is copied to configBase. If host.addChild(context) threw IllegalStateException, only a directory is registered into redeployResources. Context.xml does not register into redeployResources. If manager app execute undeploy(or delete directory), directory is deleted but context.xml isn't deleted. Should (conf/Catalina/localhost/)context.xml be registered into redeployResources? Or need to delete context.xml manually? I think context.xml should be registered into redeployResources. A better patch is needed. 5) Regarding the change to ContainerBase.addChild() It seems too wide (concerns all containers), but I do not see a better solution here. (In reply to comment #0) > 2) The input stream for the broken context xml file is not properly closed. > When running on Windows the file cannot be deleted without stopping Tomcat. I do not have a solution for the locked context xml file issue. Tomcat 7 is affected as well. The issue seems to occur inside XmlReader created by Digester. We could patch out copy of Digester, but I do not see an API there to explicitly close the file. The workaround is to force full GC (e.g. to press the "Find Leaks" button in the Manager). After that the file is unlocked. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
