As I note in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23068#c54 the Koha::Middleware::SetEnv breaks the flow of $env through the Plack middleware wrapper chain. This means that post-processing middleware that happen after Koha::Middleware::SetEnv (e.g. Plack::Middleware::AccessLog) will use the wrong $env variable (ie the $env variable that was being used before Koha::Middleware::SetEnv was run in the pre-process middleware phase). By retaining the original $env hashref in Koha::Middleware::SetEnv, we should be able to preserve the referential integrity. Going to give a patch a go now...
Created attachment 152227 [details] [review] Bug 33967: Fix SetEnv middleware to preserve $env integrity This patch fixes the Koha::Middleware::SetEnv, so that it preserves the referential integrity of the $env hash reference as it passes through the pre-process and post-process stages of middleware wrapped around the core Koha Plack application. Test plan: 0a. Apply the patch 0b. vi /etc/koha/sites/kohadev/koha-conf.xml 0c. Uncomment "koha_trusted_proxies" 0d. Add "172.0.0.0/8" to the end of the "koha_trusted_proxies" 0e. koha-plack --restart kohadev 1. vi /etc/apache2/sites-enabled/kohadev.conf 2. Add the following to the OPAC virtualhost: SetEnv OVERRIDE_SYSPREF_LibraryName "The Best, Truly the Best, Koha Library" RequestHeader add X-Koha-SetEnv "OVERRIDE_SYSPREF_LibraryName The Best\, Truly the Best\, Koha Library" RequestHeader add X-Forwarded-For "192.168.100.100" 3. service apache2 restart 4. tail -f /var/log/koha/kohadev/plack.log 5. Go to http://localhost:8080/ 6. Note that the plack.log lists 192.168.100.100 as the client IP address 7. Note that the title of the OPAC webpage is "The Best, Truly the Best, Koha Library Library catalog"
Created attachment 152228 [details] [review] Bug 33967: Fix SetEnv middleware to preserve $env integrity This patch fixes the Koha::Middleware::SetEnv, so that it preserves the referential integrity of the $env hash reference as it passes through the pre-process and post-process stages of middleware wrapped around the core Koha Plack application. Test plan: 0a. Apply the patch 0b. vi /etc/koha/sites/kohadev/koha-conf.xml 0c. Uncomment "koha_trusted_proxies" 0d. Add "172.0.0.0/8" to the end of the "koha_trusted_proxies" 0e. koha-plack --restart kohadev 1. vi /etc/apache2/sites-enabled/kohadev.conf 2. Add the following to the OPAC virtualhost: SetEnv OVERRIDE_SYSPREF_LibraryName "The Best, Truly the Best, Koha Library" RequestHeader add X-Koha-SetEnv "OVERRIDE_SYSPREF_LibraryName The Best\, Truly the Best\, Koha Library" RequestHeader add X-Forwarded-For "192.168.100.100" 3. service apache2 restart 4. tail -f /var/log/koha/kohadev/plack.log 5. Go to http://localhost:8080/ 6. Note that the plack.log lists 192.168.100.100 as the client IP address 7. Note that the title of the OPAC webpage is "The Best, Truly the Best, Koha Library Library catalog" Signed-off-by: David Nind <david@davidnind.com>
Created attachment 152229 [details] [review] Bug 33967: Add unit test to ensure SetEnv doesn't compromise $env This patch includes a unit test which checks the memory address of the $env hashref in middlewares before and after Koha::Middleware::Env is applied. It succeeds when the same $env is maintained before and after this middleware. It fails if the $env has been overwritten with a new hashref (as evidenced by the new memory address for the hash).
Created attachment 152230 [details] [review] Bug 33967: Add unit test to ensure SetEnv doesn't compromise $env This patch includes a unit test which checks the memory address of the $env hashref in middlewares before and after Koha::Middleware::Env is applied. It succeeds when the same $env is maintained before and after this middleware. It fails if the $env has been overwritten with a new hashref (as evidenced by the new memory address for the hash).
If you want to verify the unit test works, you can either run it before applying the first patch, or you can compromise Koha/Middleware/SetEnv.pm by adding the following: $env = {}; right before the final return in the call sub.
Created attachment 152241 [details] [review] Bug 33967: Fix SetEnv middleware to preserve $env integrity This patch fixes the Koha::Middleware::SetEnv, so that it preserves the referential integrity of the $env hash reference as it passes through the pre-process and post-process stages of middleware wrapped around the core Koha Plack application. Test plan: 0a. Apply the patch 0b. vi /etc/koha/sites/kohadev/koha-conf.xml 0c. Uncomment "koha_trusted_proxies" 0d. Add "172.0.0.0/8" to the end of the "koha_trusted_proxies" 0e. koha-plack --restart kohadev 1. vi /etc/apache2/sites-enabled/kohadev.conf 2. Add the following to the OPAC virtualhost: SetEnv OVERRIDE_SYSPREF_LibraryName "The Best, Truly the Best, Koha Library" RequestHeader add X-Koha-SetEnv "OVERRIDE_SYSPREF_LibraryName The Best\, Truly the Best\, Koha Library" RequestHeader add X-Forwarded-For "192.168.100.100" 3. service apache2 restart 4. tail -f /var/log/koha/kohadev/plack.log 5. Go to http://localhost:8080/ 6. Note that the plack.log lists 192.168.100.100 as the client IP address 7. Note that the title of the OPAC webpage is "The Best, Truly the Best, Koha Library Library catalog" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 152242 [details] [review] Bug 33967: Add unit test to ensure SetEnv doesn't compromise $env This patch includes a unit test which checks the memory address of the $env hashref in middlewares before and after Koha::Middleware::Env is applied. It succeeds when the same $env is maintained before and after this middleware. It fails if the $env has been overwritten with a new hashref (as evidenced by the new memory address for the hash). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 23.11. Nice work everyone, thanks!
Thanks for all the hard work! Pushed to 23.05.x for the next release
Nice work everyone! Pushed to oldstable for 22.11.x