Skip to content

Commit

Permalink
Fix #569 Only skip subdomains for non-www domains
Browse files Browse the repository at this point in the history
The logic to skip the reversing functionality for subdomains was also
affecting domains starting with `www`. Now it correctly leaves `www`
domains alone and only skips other subdomains.
  • Loading branch information
swalkinshaw committed Apr 17, 2016
1 parent 79fdb34 commit 3878399
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
### HEAD
* Fix #569 - Only skip subdomains for non-www domains ([#570](https://github.com/roots/trellis/pull/570))
* Enable Let's Encrypt to transition http sites to https ([#565](https://github.com/roots/trellis/pull/565))

### 0.9.7: April 10th, 2016
Expand Down
8 changes: 4 additions & 4 deletions lib/trellis/plugins/filter/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ def reverse_www(hosts, enabled=True, append=True):
elif isinstance(hosts, string_types):
host = hosts

if len(host.split('.')) > 2:
return host

if host.startswith('www.'):
return host[4:]
else:
return 'www.{0}'.format(host)
if len(host.split('.')) > 2:
return host
else:
return 'www.{0}'.format(host)

# Handle invalid input type
else:
Expand Down

3 comments on commit 3878399

@jdebuchy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swalkinshaw, sorry if it's not the way to mention this bug. Watch out for line 32

if len(host.split('.')) > 2:

when we have international domains in our hosts (like com.ar, co.uk, etc) www is not added because the conditional returns true.

So if you have a domain like domain.com.ar you will not have the www redirect because the condition think it is a subdomain.

If I get to a solution I'll post it asap.

Thanks!

@swalkinshaw
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdebuchy thanks for pointing this out. Unfortunately there's no easy way of solving this problem. The only reliable way to do it (in code) is to have a list of TLDs and check them all. There's some python libraries which handle it but Ansible doesn't make it easy to use external dependencies.

@jdebuchy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the answer @swalkinshaw, and sad to hear there is no easy solution for this. If I find a walk-around to it, I'll let you know. Unfortunately, I work with many clients who uses these TLDs :(.

Please sign in to comment.