Always use https #5
Conversation
|
fyi @ossarga |
There was a problem hiding this comment.
I have read the relevant sections of the htaccess and mod_rewrite to understand what is going on here.
I have tested the regex in bash inside a Ubuntu docker container
root@ff2f9e8ca396:~# echo "/test/path" | sed -r 's/^\/?(.*)/https:\/\/www.example.com\/\1/'
https://www.example.com/test/path
I am confident that the RewriteRule and RewriteRule Flags are correct. I think this is ok to merge. There are two questions I have, both of which are non-blocking for this PR to be merged.
| @@ -1 +1,4 @@ | |||
| RewriteEngine On | |||
|
|
|||
| RewriteCond %{HTTPS} !=on | |||
There was a problem hiding this comment.
In the Apache [mod_rewrite spec|https://httpd.apache.org/docs/current/mod/mod_rewrite.html#RewriteCond] it says
The string comparison operator is part of the CondPattern argument and must be included in the quotes if those are used. Eg.
RewriteCond %{HTTP_USER_AGENT} "=This Robot/1.0"
Should the !=on be enclosed in quotes i.e. "!=on"?
There was a problem hiding this comment.
I don't think so. Numerous places, including apache's own wiki, gives examples without the quotes.
There was a problem hiding this comment.
Cool. Sounds fine to me then.
| @@ -1 +1,4 @@ | |||
| RewriteEngine On | |||
|
|
|||
| RewriteCond %{HTTPS} !=on | |||
| RewriteRule ^/?(.*) https://%{SERVER_NAME}/$1 [R,L] | |||
There was a problem hiding this comment.
The SERVER_NAME Server-Variable maps to the ServerName Directive. The syntax for this directive says that it can optionally include the scheme. This may cause problems with the rewrite rule if a scheme is included in the server name. Is it worth using the HTTP_HOST instead? The downside I see to this is that HTTP_HOST is the host name only where as the SERVER_NAME can include the port number.
There was a problem hiding this comment.
have tested it. SERVER_NAME works fine in different scenarios.
|
Tested on http://cassandra.staged.apache.org |
patch by Chris Thornett, Erick Ramirez; reviewed by Erick Ramirez, Anthony Grasso for CASSANDRA-16496
No description provided.