Bug 15005 - (Plack) Use of SCRIPT_NAME makes Koha generate wrong URIs
Summary: (Plack) Use of SCRIPT_NAME makes Koha generate wrong URIs
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 15015 15016 (view as bug list)
Depends on:
Blocks: 15019 15816
  Show dependency treegraph
 
Reported: 2015-10-13 13:10 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2019-06-27 09:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Wrong links generated (54.93 KB, image/png)
2015-10-13 13:11 UTC, Tomás Cohen Arazi (tcohen)
Details
Bug 15005: Replace CGI->url with the corresponding url (7.04 KB, patch)
2015-10-13 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Remove CGI->url calls from pl scripts (3.25 KB, patch)
2015-10-13 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Fix pagination_bar calls (1.66 KB, patch)
2015-10-13 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Use ReverseProxyPath on plack.psgi (2.29 KB, patch)
2015-10-15 18:06 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 15005: Use ReverseProxyPath on plack.psgi (2.89 KB, patch)
2015-10-15 18:10 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 15005: Replace CGI->url with the corresponding url (7.12 KB, patch)
2015-10-16 06:50 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15005: Remove CGI->url calls from pl scripts (3.33 KB, patch)
2015-10-16 06:50 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15005: Fix pagination_bar calls (1.74 KB, patch)
2015-10-16 06:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths (6.75 KB, patch)
2015-10-16 08:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Replace CGI->url with the corresponding url (7.09 KB, patch)
2015-10-16 08:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Remove CGI->url calls from pl scripts (3.30 KB, patch)
2015-10-16 08:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Fix pagination_bar calls (1.71 KB, patch)
2015-10-16 08:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths (7.03 KB, patch)
2015-10-16 08:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths (7.14 KB, patch)
2015-10-16 09:36 UTC, Josef Moravec
Details | Diff | Splinter Review
[PASSED QA] Bug 15005: Replace CGI->url with the corresponding url (7.00 KB, patch)
2015-10-16 16:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 15005: Remove CGI->url calls from pl scripts (3.38 KB, patch)
2015-10-16 16:43 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 15005: Fix pagination_bar calls (1.79 KB, patch)
2015-10-16 16:43 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths (7.23 KB, patch)
2015-10-16 16:43 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 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 (tcohen) 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 (tcohen) 2015-10-15 18:06:54 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi (tcohen) 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 (khall) 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 (khall) 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 (khall) 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 (khall) 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 (tcohen) 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.