Bug 15005

Summary: (Plack) Use of SCRIPT_NAME makes Koha generate wrong URIs
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: gwilliams, josef.moravec, kyle.m.hall, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15015
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15045
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15945
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 15019, 15816    
Attachments: Wrong links generated
Bug 15005: Replace CGI->url with the corresponding url
Bug 15005: Remove CGI->url calls from pl scripts
Bug 15005: Fix pagination_bar calls
Bug 15005: Use ReverseProxyPath on plack.psgi
Bug 15005: Use ReverseProxyPath on plack.psgi
Bug 15005: Replace CGI->url with the corresponding url
Bug 15005: Remove CGI->url calls from pl scripts
Bug 15005: Fix pagination_bar calls
Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths
Bug 15005: Replace CGI->url with the corresponding url
Bug 15005: Remove CGI->url calls from pl scripts
Bug 15005: Fix pagination_bar calls
Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths
Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths
[PASSED QA] Bug 15005: Replace CGI->url with the corresponding url
[PASSED QA] Bug 15005: Remove CGI->url calls from pl scripts
[PASSED QA] Bug 15005: Fix pagination_bar calls
[PASSED QA] Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths

Description Tomás Cohen Arazi 2015-10-13 13:10:38 UTC
The current plack-on-packages schema points the apache vhost (for example for the intranet) like this:

  /cgi-bin/koha/ => /intranet/

So the code that uses ENV{SCRIPT_NAME} "sees" it is being called for example like /intranet/mainpage.pl. This gets problematic in some scenarios, because it is used to then redirect the browser, or to create links to things.

Steps to reproduce:
- Check out an old DBRev in the koha repo (1 step older is ok)
- Pick the ansible branch from the kohadevbox [1]
- Launch it, on jessie:
  $ vagrant up jessie
- Log into it
  $ vagrant ssh jessie
- Enable Plack for the automatically created instance
  $ sudo koha-plack --enable kohadev
  $ sudo koha-plack --start  kohadev
  $ sudo service apache2 restart
- Get the DB password to be able to login
  $ koha-pass
- Point your browser to the staff interface:
  http://localhost:8081
- Login with koha_kohadev and the retrieved password
=> FAIL: You cannot login
- Disable Plack for the instance
  $ sudo koha-plack --disable kohadev
  $ sudo koha-plack --stop  kohadev
  $ sudo service apache2 restart
- Point your browser to the staff interface:
  http://localhost:8081
- Login with koha_kohadev and the retrieved password
=> SUCCESS: You can login
- Proceed to install with the default data (you can try enabling plack and you will notice it works)
- Once you are in, enable plack
  $ sudo koha-plack --enable kohadev
  $ sudo koha-plack --start  kohadev
  $ sudo service apache2 restart
- Checkout the latest master (so it triggers an update)
- Go somewhere inside (about?)
- You are required to login (which is expected)
=> FAIL: the links back are pointing to /intranet/ so they are shortcircuited to the installer cyclically (see screenshot)

[1] https://github.com/digibib/kohadevbox/tree/ansible
Comment 1 Tomás Cohen Arazi 2015-10-13 13:11:15 UTC
Created attachment 43383 [details]
Wrong links generated
Comment 2 Jonathan Druart 2015-10-13 16:01:43 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-10-13 16:01:46 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-10-13 16:01:52 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-10-14 12:56:00 UTC
Breaking news:

Add this line to /etc/koha/sites/kohadev/plack.psgi
  enable "ReverseProxyPath";

And this one to /etc/koha/apache-shared-intranet-plack.conf
  RequestHeader set "X-Forwarded-Script-Name" /cgi-bin/koha

fix the $ENV{SCRIPT_NAME}!

($cgi->url still does not work)
Comment 6 Tomás Cohen Arazi 2015-10-15 18:06:54 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2015-10-15 18:10:28 UTC Comment hidden (obsolete)
Comment 8 Josef Moravec 2015-10-16 06:50:49 UTC Comment hidden (obsolete)
Comment 9 Josef Moravec 2015-10-16 06:50:58 UTC Comment hidden (obsolete)
Comment 10 Josef Moravec 2015-10-16 06:51:06 UTC Comment hidden (obsolete)
Comment 11 Josef Moravec 2015-10-16 06:53:08 UTC
The patches works as expected.
Comment 12 Jonathan Druart 2015-10-16 08:07:46 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-10-16 08:09:28 UTC
(In reply to Tomás Cohen Arazi from comment #7)
> Created attachment 43444 [details] [review] [review]
> Bug 15005: Use ReverseProxyPath on plack.psgi
> 
> This patch makes plack.psgi use ReverseProxyPath so %ENV{SCRIPT_NAME} is
> correctly set when running Koha behind a reverse proxy that changes
> the URI path (which is done on the Plack integration into packages).
> 
> The apache includes for plack are tweaked so the needed headers are set
> before passing the request to the Plack backend.
> 
> Edit: added missing dependency
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Tomas,
Unfortunately, this does not work, the path generated is now /cgi-bin/koha/intranet/modules/script.pl (note the '/intranet').
The last patch I have proposed should fix the issue on a silly way (but working...).
Comment 14 Jonathan Druart 2015-10-16 08:15:47 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-10-16 08:15:51 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2015-10-16 08:15:54 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-10-16 08:15:57 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2015-10-16 08:58:08 UTC
*** Bug 15014 has been marked as a duplicate of this bug. ***
Comment 19 Jonathan Druart 2015-10-16 09:00:10 UTC
*** Bug 15015 has been marked as a duplicate of this bug. ***
Comment 20 Jonathan Druart 2015-10-16 09:06:54 UTC
*** Bug 15016 has been marked as a duplicate of this bug. ***
Comment 21 Josef Moravec 2015-10-16 09:36:39 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2015-10-16 16:42:58 UTC
Created attachment 43534 [details] [review]
[PASSED QA] Bug 15005: Replace CGI->url with the corresponding url

CGI->url does not return the correct url on install using packages.

Test plan:
1/ Try to reproduce the bug from the description of bug 15005.
You should be able to login to the intranet and the OPAC
2/ Send a basket and a list from the intranet and the OPAC.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Kyle M Hall 2015-10-16 16:43:09 UTC
Created attachment 43535 [details] [review]
[PASSED QA] Bug 15005: Remove CGI->url calls from pl scripts

Same as previous patch for calls in pl scripts.

Test plan:
- delete an entry of your search history from intranet and opac
- with IndependentBranches=OFF, go on the waiting holds and confirm that the
link next to 'Holds awaiting pickup for your library on' is correct.
- search for items, the pagination should work correctly

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Kyle M Hall 2015-10-16 16:43:12 UTC
Created attachment 43536 [details] [review]
[PASSED QA] Bug 15005: Fix pagination_bar calls

Test plan:
Create an order for an existing biblio, confirm that the pagination links work correctly.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall 2015-10-16 16:43:16 UTC
Created attachment 43537 [details] [review]
[PASSED QA] Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths

Since I don't manage to make SCRIPT_NAME works directly with RequestHeader and/or
ReverseProxyPath, the easier way it to fix all the different occurrences.

Test plan:
On the import patrons page, confirm that the "Import" button does not redirect to a 'Not found' page.
You should stay on the import patrons page.

Note that if this change works, all others should work too.
Have a look at the diff of this patch and confirm there is no typo.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Tested in patron import and item batch modification

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2015-10-16 16:45:16 UTC
Note: SCRIPT_NAME is referenced in a number of templates and a few perl scripts. In some cases it is passed to the template as a parameter. In others it is not, which makes the form action submit to the current url. As far as I'm aware this works fine and is non-detrimental, but I did want to make a note of it.
Comment 27 Tomás Cohen Arazi 2015-10-19 13:15:53 UTC
Patches pushed to master.

Thanks Jonathan!

We need to fill an enhancement bug for letting Koha stand in whatever
endpoint we want instead of the legacy URI path.