Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistencies with add www / strip www #312

Closed
retlehs opened this issue Aug 24, 2015 · 10 comments
Closed

Inconsistencies with add www / strip www #312

retlehs opened this issue Aug 24, 2015 · 10 comments
Labels
Milestone

Comments

@retlehs
Copy link
Member

retlehs commented Aug 24, 2015

via @fullyint:

(untested)
               DOES THE FORWARDING SUCCEED?

-- START URL --  ------------- GOAL URL ----------------------
                     A         B           C            D
                  http://   https://   http://www   https://www
1   http://          -         Y           N            N
2  https://          N         -           N            N
3   http://www       Y         Y           -            Y
4  https://www       N         Y           N            -

Existing Features       Missing Features
  - add_s                 - strip_s
  - strip_www             - add_www
@kalenjohnson
Copy link
Contributor

fullyint++ for nice graph

@austinpray
Copy link
Contributor

image

@louim
Copy link
Contributor

louim commented Aug 24, 2015

I think we want the strip_www to act like a toggle, either forcing no www or forcing the www. I don't think we want to redirect from https to http. The bold N are the case we would like to work.

http:// https:// http://www https://www
http:// - Y N N
https:// N - by design - N - by design N
http://www Y Y - Y
https://www N - by design Y N - by design -

@fullyint
Copy link
Contributor

I hadn't thought we'd want any N - by design because it seems like a site would simply fail to load under conditions such as these:

  • we use the N - by design for https:// --> http://
  • someone has their site listening only on port 80 at http://example.com
  • a user erroneously navigates to https://example.com but we're not listening on port 443

I like the idea of thinking of it like toggles.
www = true/false forces www or non-www
wordpress_sites[site].ssl.enabled = true/false forces https or http

Edit: I know there's plenty I don't understand about SSL, Strict-Transport-Security, etc.

@louim
Copy link
Contributor

louim commented Aug 24, 2015

I still think we should be pushing our users towards HTTPS, and not allow the opposite. With https://letsencrypt.org/ around the corner, I think offering to downgrade connexions is not a good idea.

Also, you would still need a Certificate to listen on the HTTPS port, because the redirect kick in only after the connexion is established. So it's a bad idea IMHO.

@swalkinshaw
Copy link
Member

Yeah they key to redirecting from https -> http is that a cert is needed anyway as @louim said. So we don't care about that.

The www variable (with a better name) should be moved into a WP site wordpress_sites[site] as well.

@fullyint
Copy link
Contributor

I see, thanks! Downgrading connections is bad practice and brings logistical problems with certs.

@fullyint
Copy link
Contributor

We could automatically redirect the opposite of each item in wordpress_sites[site].site_hosts (referring to www and non-www as opposites). This avoids requiring users to toggle anything.

host: www.example.com
has_www: "{{ host | match('^www\\.(.*)') }}" # returns true/false
www_removed: "{{ host | regex_replace('^www\\.(.*)', '\\\\1') }}" # returns hostname without www
host_to_redirect: "{{ has_www | bool | ternary(www_removed, 'www.' + host ) }}"

We can't just put those variables in roles/wordpress-setup/default/main.yml because host will not be defined. So, if you string all of them into one long logic, it's not pretty but it works. It should redirect all the target bold Ns from above.

# roles/wordpress-setup/templates/wordpress-site.conf.j2

- {% for host in item.value.site_hosts if strip_www %}
+ {% for host in item.value.site_hosts if www_redirect %}
  server {
    {% if item.value.ssl is defined and item.value.ssl.enabled | default(false) -%}
      listen 443 ssl spdy;
    {% else -%}
      listen 80;
    {% endif -%}
-     server_name www.{{ host }};
+     server_name {{ host | match('^www\\.(.*)') | ternary(host | regex_replace('^www\\.(.*)', '\\1'), 'www.' + host ) }};
      return 301 $scheme://{{ host }}$request_uri;
  }
  {% endfor %}

Users can turn off the automatic redirect by setting www_redirect: false
Backreferences are escaped differently in the two examples (\\\\1 vs. \\1). Also, bool filter is not needed in 2nd example.

@retlehs
Copy link
Member Author

retlehs commented Sep 29, 2015

add_www/strip_www should also be ignored for subdomains

@swalkinshaw
Copy link
Member

Closed via #452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants