https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36304
David Cook <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #5 from David Cook <[email protected]> --- (In reply to Nicholas van Oudtshoorn from comment #4) > Created attachment 163973 [details] [review] > Support for externally updated proxy lists > > Thanks for the feedback, David! Thanks for being receptive! > It makes sense to move it into the koha-conf.xml file, and this updated > patch has done so. It includes a sample bash script that can be used to keep > an external list file up to date. This can also easily be extended to > automatically update other providers, hence I have taken away the cloudflare > name, although cloudflare is referenced in a comment in koha-conf.xml You'll want to add the config block to "debian/templates/koha-conf-site.xml.in" as well. (I think it could be worthwhile to say in the comment that the list is a plain-text with an IP (range) per line, but not a blocker.) > Currently, RealIP was re-generating the trusted_proxies list on every call > to get_real_ip. This patch also uses Koha's caching system to prevent > reloading from the config file and from the external list on every call, > which should speed things up just a little bit. Oh yikes. I remember reading through this recently, and I saw "$self->trusted_proxy()" in prepare_app(), and that list is passed to get_real_ip(), so I figured it was just determining the list at startup time, but it looks like it's unused currently and it really is fetching it on every call! Double yikes... I think we'd want to split the changes here. I think a new ticket to fix RealIP.pm to use the $self->trusted_proxy() set up in "prepare_app", and we could also set that up to use the cache to further improve performance. The change to RealIP.pm looks like it has some copy/pasted code. I'd suggest adding the list IPs to @trusted_proxies() rather than having 2 separate blocks calling Net::Netmask->new2(). > The sample bash script will restart koha-common if it is running, and also > clear the caches of all koha-instances. Since this one is Cloudflare specific, I think it would be worthwhile including that in the script name (e.g. "update_cloudflare_proxies_list.sh). I think it might be overkill restarting the whole koha-common service, since we really just need to restart the Plack in this case. You'd want to clear the cache first before running koha-plack --reload/--restart as well I believe. -- Overall, I think it's looking pretty good though. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
