Bug 27009 - Use 301 to redirect from tracklinks.pl
Summary: Use 301 to redirect from tracklinks.pl
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Magnus Enger
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-13 09:14 UTC by Magnus Enger
Modified: 2022-03-10 03:21 UTC (History)
2 users (show)

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


Attachments
Bug 27009 - Use 301 to redirect from tracklinks.pl (1.34 KB, patch)
2020-11-13 09:53 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 27009 - Use 301 to redirect from tracklinks.pl (1.39 KB, patch)
2020-11-15 18:31 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2020-11-13 09:14:30 UTC
In opac/tracklinks.pl we use a simple redirect to send the user on to the actual URL, after tracking:

  print $cgi->redirect($uri);

This sends a "302 Moved Temporarily" to the browser as a default. One of our clients had some SEO work done on their websites, and one of the recommendations was to use "301 Moved Permanently" instead. This makes some sense, the URLs we redirect to do not change very often, I guess. 

According to https://metacpan.org/pod/CGI#Generating-a-redirection-header, the solution is to change the call to redirect() thusly:

  print $q->redirect(
    -uri    => $uri,
    -status => '301 Moved Permanently'
  );

Patch coming.
Comment 1 Magnus Enger 2020-11-13 09:53:49 UTC
Created attachment 113588 [details] [review]
Bug 27009 - Use 301 to redirect from tracklinks.pl

The default for the tracklinks.pl redirect is "302 Moved Temporarily".
It makes more sense to use "301 Moved Permanently".

To test:
1. Set TrackClicks = Track
2. Make sure you have a MARC record with an external link in 856$u
3. In the OPAC, click on the link and use e.g. Developer tools to
   check on the HTTP status code sent by tracklinks.pl. It will
   be 302.
4. Apply this patch and restart all the things
5. Click on the external link in the OPAC again. The status code
   should now be 301.
Comment 2 David Nind 2020-11-15 18:31:40 UTC
Created attachment 113647 [details] [review]
Bug 27009 - Use 301 to redirect from tracklinks.pl

The default for the tracklinks.pl redirect is "302 Moved Temporarily".
It makes more sense to use "301 Moved Permanently".

To test:
1. Set TrackClicks = Track
2. Make sure you have a MARC record with an external link in 856$u
3. In the OPAC, click on the link and use e.g. Developer tools to
   check on the HTTP status code sent by tracklinks.pl. It will
   be 302.
4. Apply this patch and restart all the things
5. Click on the external link in the OPAC again. The status code
   should now be 301.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Julian Maurice 2020-11-16 08:19:39 UTC
HTTP 301 responses are cacheable by default, which means some browsers can cache the redirect indefinitely, and can redirect without doing the request to tracklinks.pl once it is cached. I believe it is not the intended behaviour for a script that is supposed to track redirects.

I don't know what are the modern browsers caching strategies for those redirects, but just to be sure, I think we should add a header 'Cache-Control: no-store'.
Comment 4 Julian Maurice 2020-12-04 13:20:15 UTC
Changing status to Failed QA to get feedback