Skip to content

Commit

Permalink
Fix improperly configured host in generated urls
Browse files Browse the repository at this point in the history
If the host in `default_url_options` is accidentally set with a protocol such as 

```
host: "http://example.com"
``` 

then the generated url will have the protocol twice `http://http://example.com` which is not what the user intended. Likely they wanted to define a host `host: "example.com"` and a `protocol: "http://"` but did not know the convention.

This may not the most common problem, but when it happens it can go undetected for a while. I accidentally added `http://` out of habit recently only to find all the links in my emails were broken after deploying a demo site to production. Rather than allow this accident go undetected, we can fix the problem in line by properly setting the protocol and host.


I was able to find this related question on stack overflow: http://stackoverflow.com/questions/5878329/rails-3-devise-how-do-i-make-the-email-confirmation-links-use-secure-https-n where the answer was highly upvoted.

This is based off of work in rails#7415 cc/ @pixeltrix

back port of rails#9794

ATP Action Mailer and Action Pack
  • Loading branch information
schneems committed Mar 20, 2013
1 parent dc2bc38 commit 3f11317
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
8 changes: 8 additions & 0 deletions actionpack/CHANGELOG.md
@@ -1,5 +1,13 @@
## unreleased ##

* Allow default url options to accept host with protocol such as `http://`

config.action_mailer.default_url_options = { host: "http://mydomain.com" }

Backport #9794

*Richard Schneeman*

* `ActiveSupport::NumberHelper#number_to_human` returns the number unaltered when
the units hash does not contain the needed key, e.g. when the number provided is less
than the largest key proivided.
Expand Down
13 changes: 12 additions & 1 deletion actionpack/lib/action_dispatch/http/url.rb
Expand Up @@ -28,8 +28,9 @@ def url_for(options = {})
rewritten_url = ""

unless options[:only_path]
protocol = extract_protocol(options)
unless options[:protocol] == false
rewritten_url << (options[:protocol] || "http")
rewritten_url << protocol
rewritten_url << ":" unless rewritten_url.match(%r{:|//})
end
rewritten_url << "//" unless rewritten_url.match("//")
Expand Down Expand Up @@ -67,6 +68,16 @@ def rewrite_authentication(options)
end
end

# Extracts protocol http:// or https:// from options[:host]
# needs to be called whether the :protocol is being used or not
def extract_protocol(options)
if options[:host] && match = options[:host].match(/(^.*:\/\/)(.*)/)
options[:protocol] ||= match[1]
options[:host] = match[2]
end
options[:protocol] || "http"
end

def host_or_subdomain_and_domain(options)
return options[:host] if !named_host?(options[:host]) || (options[:subdomain].nil? && options[:domain].nil?)

Expand Down
8 changes: 8 additions & 0 deletions actionpack/test/dispatch/url_generation_test.rb
Expand Up @@ -39,6 +39,14 @@ def app
https!
assert_equal "http://www.example.com/foo", foo_url(:protocol => "http")
end

test "extracting protocol from host when protocol not present" do
assert_equal "httpz://www.example.com/foo", foo_url(host: "httpz://www.example.com", protocol: nil)
end

test "formatting host when protocol is present" do
assert_equal "http://www.example.com/foo", foo_url(host: "httpz://www.example.com", protocol: "http://")
end
end
end

0 comments on commit 3f11317

Please sign in to comment.