Bug 9683 - Allow disable rewrite apache mod
Summary: Allow disable rewrite apache mod
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-21 14:25 UTC by Fridolin Somers
Modified: 2017-05-25 04:29 UTC (History)
4 users (show)

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


Attachments
Proposed patch (3.01 KB, patch)
2013-02-21 14:29 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 9683: Allow disable rewrite apache mod (3.06 KB, patch)
2013-03-18 09:54 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9683: Allow disable rewrite apache mod (3.15 KB, patch)
2013-04-02 22:05 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-02-21 14:25:53 UTC
In Apache config koha-httpd.conf, URL-rewriting is enabled and does not allow to disable mod_rewrite.
Also, first rewriting rule "RewriteRule (.+) $1?%1%2 [N,R,NE]" is enabled by default. This rule rewrites nearly every URL. I propose to comment it in sources so that is must be intentionally enabled.
Comment 1 Fridolin Somers 2013-02-21 14:29:54 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2013-02-21 14:32:24 UTC
Could you address what the ramifications of this change would be? Do we lose any functionality?
Comment 3 Fridolin Somers 2013-02-21 14:37:04 UTC
I propose to disable first rewriting rule "RewriteRule (.+) $1?%1%2 [N,R,NE]" because we notices that it can cause some problems.

For example, a library has defined some search links in OPACNav : 
"/cgi-bin/koha/opac-search.pl?idx=kw&idx=kw&idx=kw&limit=mc-ccode%3ADVD" (%3A means ":").
When clicking on this URL, it is rewritten and espaced characters are re-escaped : 
"mc-ccode%3ADVD" becomes "mc-ccode%253ADVD", witch is wrong.

I've searched but I could not find what is the purpose of this rule.
Does someone know ?
Comment 4 Fridolin Somers 2013-02-21 14:39:27 UTC
(In reply to comment #2)
> Could you address what the ramifications of this change would be? Do we lose
> any functionality?

Setting rewrite conditional does not change anything.
If we can find wat is the purpose of first rewrite rule, it will be disabled but only for new installs.
Comment 5 Chris Cormack 2013-03-18 09:52:05 UTC
http://lists.koha-community.org/pipermail/koha-devel/2007-December/029821.html

Considering how wrong the author of that email was about pretty much everything else, I vote for removing it (empty params are harmless, much more harmless than rewriting urls we don't need to)
Comment 6 Chris Cormack 2013-03-18 09:54:33 UTC Comment hidden (obsolete)
Comment 7 Mason James 2013-04-02 22:05:23 UTC
Created attachment 17151 [details] [review]
Bug 9683: Allow disable rewrite apache mod

In Apache config koha-httpd.conf, URL-rewriting is enabled and does not allow to disable mod_rewrite.
Also, first rewriting rule "RewriteRule (.+) $1?%1%2 [N,R,NE]" is enabled by default. This rule rewrites nearly every URL. I propose to comment it in sources so that is must be intentionally enabled.

This patch sets rewriting options into a conditional tag.

Test plan :
Test OPAC and intranet with and without mod_rewrite activated.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Comment 8 Mason James 2013-04-02 22:06:50 UTC
(In reply to comment #7)
> Created attachment 17151 [details] [review] [review]
> Bug 9683: Allow disable rewrite apache mod
> This patch sets rewriting options into a conditional tag.
> 
> Test plan :
> Test OPAC and intranet with and without mod_rewrite activated.
> 
> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
> Signed-off-by: Mason James <mtj@kohaaloha.com>

passing QA

patch looks fine and works as expected
Comment 9 Jared Camins-Esakov 2013-04-03 11:04:03 UTC
This patch has been pushed to master.
Comment 10 David Cook 2017-05-25 04:17:41 UTC
Looks like we should've commented out the following as well:

RewriteCond %{QUERY_STRING} (.*?)(?:[A-Za-z0-9_-]+)=&(.*)

In the absence of RewriteRule (.+) $1?%1%2 [N,R,NE], it's being applied to RewriteRule ^/bib/([^\/]*)/?$ /cgi-bin/koha/opac-detail\.pl?bib=$1 [PT].

I'm opening a new bug for that though.
Comment 11 David Cook 2017-05-25 04:22:58 UTC
Also, this patch isn't implemented in the Debian Apache templates, although that doesn't apply to my current situation...