added domain white list feature, error-image and context.xml#12
added domain white list feature, error-image and context.xml#12
Conversation
|
I've forgotten I made timeouts configurable via deepzoomer.cfg |
There was a problem hiding this comment.
Not easy to review, because there is no corresponding issue. Just some remarks while walking through. Maybe also some misunderstandings/lack of understanding on my side.
- Whitelist
Whitelist implementation is too verbose in my opinion. Just load whitelist to static hashmap and query it before entering critical tasks. I think the right place is at the controller and not within the utility classes.if(!hashmap.containsKey(ip)){ ...return HTTP FORBIDDEN }. (Also, I don't think this should be an exceptional case, just normal data flow. But this is maybe a matter of taste.) BTW, why is classDomainWhiteListCheckerabstract?
For future reference I think you should add at least some hints to provide answers to the following questions.
- What are the changes in web.xml, context.xml and deepzoomer.cfg for?
- What are the changes in DeepZoomerUrlService.java for?
- What are the changes in ZoomifyUrlService.java for?
Maybe the answers are quite obvious to you, but for example, why is an additional entry in context.xml needed? I totally missed that point.
|
Thank you for review. As discussed yesterday F2F there seems to be a different behavior/access rights implementation for tomcat at Ubuntu and openSuSE: OpenSuSE requires a context path to be set in context.xml in order to allow tomcat webapps access to directories outside the webapps directory. If Ubuntu doesn't, does that mean we have to take extra provisions to prevent tomcat webapps to do this on Ubuntu - and is this something you've already done in your last commit ;-) ? I'm not quite sure about this. I agree with most of your other remarks. Decision to implement Whitelist within FileUtil was made due to maintenance reasons - and also kind of pressue to find a solution in a hurry. I will review this decision as it has some issues. |
I've added an white list feature to allow requests only from specific domains. If either an unaccepted domain is choosen or something goes wrong with the image generation an error-image is displayed now.
Amid the works I've been in some trouble to set up tilesCache outside the webapp. As fare as I understand a context.xml is needed to be copied into $CATALINA_BASE/CATALINE/localhost/