Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Decode attributes pulled from URI.parse #7593

Merged
merged 1 commit into from

5 participants

Shawn Veader Matt Jones Francesco Rodríguez Rafael Mendonça França Steve Klabnik
Shawn Veader

The RFC indicates that username and passwords may be encoded.
http://tools.ietf.org/html/rfc2396#section-3.2.2

Found this trying to use the mysql://username:password@host:port/db and having special characters in the password which needed to be URI encoded.

Matt Jones

A test case would be helpful - probably best to add it in test/cases/connection_specification/resolver_test.rb.

Shawn Veader

Tests included. Also silenced some 1.9.x deprecation warning for URI.unescape

Francesco Rodríguez

Could you squash your commits? Thanks.

Shawn Veader

@frodsan done. looks like i botched my commit message in the squash. :/

...ecord/connection_adapters/connection_specification.rb
@@ -73,6 +73,8 @@ def connection_url_to_hash(url) # :nodoc:
:database => config.path.sub(%r{^/},""),
:host => config.host }
spec.reject!{ |_,value| value.blank? }
+ uri_parser = URI.const_defined?(:Parser) ? URI::Parser.new : URI
Rafael Mendonça França Owner

When the Parser is not available?

Shawn Veader
veader added a note

Was trying to make this safe for 1.8. Not sure it's needed at this point. Thoughts?

Francesco Rodríguez
frodsan added a note

You don't have to worry about 1.8. Master branch version only supports >= 1.9.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ecord/connection_adapters/connection_specification.rb
@@ -73,6 +73,8 @@ def connection_url_to_hash(url) # :nodoc:
:database => config.path.sub(%r{^/},""),
:host => config.host }
spec.reject!{ |_,value| value.blank? }
+ uri_parser = URI.const_defined?(:Parser) ? URI::Parser.new : URI
+ spec.map { |key,val| spec[key] = uri_parser.unescape(val) if val.kind_of?(String) }
Rafael Mendonça França Owner

val.is_a?(String)

Rafael Mendonça França Owner

|key, value|

Shawn Veader
veader added a note

I can definitely change the val to value. is_a? and kind_of? are the same, correct? Either works for me, I'm not picky.

Steve Klabnik Collaborator

is_a? and kind_of? are aliases, yes.

Shawn Veader
veader added a note

I do believe they are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Rafael Mendonça França

Could you add a CHANGELOG entry?

Shawn Veader

@rafaelfranca sure, do you want me to incorporate the changes above as well?

Shawn Veader

@rafaelfranca Done. Squashed with previous and incorporated comments from above review. Thanks!

activerecord/CHANGELOG.md
@@ -36,9 +36,13 @@
implementation is very small and it's probably not worth putting users
through lots of annoying deprecation warnings.
+ *Shawn Veader*
Rafael Mendonça França Owner

This CHANGELOG entry is wrong. It should be at top of the file and the author name is below the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Shawn Veader veader Decode attributes pulled from URI.parse
The RFC indicates that username and passwords may be encoded.
http://tools.ietf.org/html/rfc2396#section-3.2.2

Found this trying to use the mysql://username:password@host:port/db and having special characters in the password which needed to be URI encoded.
f96b410
Shawn Veader

@rafaelfranca Apologies... fixed. (Someone needs sleep and/or more coffee.)

Rafael Mendonça França

Thank you so much for the pull request.

Rafael Mendonça França rafaelfranca merged commit 35f2a09 into from
Francesco Rodríguez

:+1: Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 26, 2012
  1. Shawn Veader

    Decode attributes pulled from URI.parse

    veader authored
    The RFC indicates that username and passwords may be encoded.
    http://tools.ietf.org/html/rfc2396#section-3.2.2
    
    Found this trying to use the mysql://username:password@host:port/db and having special characters in the password which needed to be URI encoded.
This page is out of date. Refresh to see the latest.
6 activerecord/CHANGELOG.md
View
@@ -1,5 +1,9 @@
## Rails 4.0.0 (unreleased) ##
+* Decode URI encoded attributes on database connection URLs.
+
+ *Shawn Veader*
+
* Add `find_or_create_by`, `find_or_create_by!` and
`find_or_initialize_by` methods to `Relation`.
@@ -38,7 +42,7 @@
*Jon Leighton*
-* Fix bug with presence validation of associations. Would incorrectly add duplicated errors
+* Fix bug with presence validation of associations. Would incorrectly add duplicated errors
when the association was blank. Bug introduced in 1fab518c6a75dac5773654646eb724a59741bc13.
*Scott Willson*
2  activerecord/lib/active_record/connection_adapters/connection_specification.rb
View
@@ -73,6 +73,8 @@ def connection_url_to_hash(url) # :nodoc:
:database => config.path.sub(%r{^/},""),
:host => config.host }
spec.reject!{ |_,value| value.blank? }
+ uri_parser = URI::Parser.new
+ spec.map { |key,value| spec[key] = uri_parser.unescape(value) if value.is_a?(String) }
if config.query
options = Hash[config.query.split("&").map{ |pair| pair.split("=") }].symbolize_keys
spec.merge!(options)
8 activerecord/test/cases/connection_specification/resolver_test.rb
View
@@ -36,6 +36,14 @@ def test_url_port
:host => "foo",
:encoding => "utf8" }, spec)
end
+
+ def test_encoded_password
+ skip "only if mysql is available" unless defined?(MysqlAdapter)
+ password = 'am@z1ng_p@ssw0rd#!'
+ encoded_password = URI.encode_www_form_component(password)
+ spec = resolve "mysql://foo:#{encoded_password}@localhost/bar"
+ assert_equal password, spec[:password]
+ end
end
end
end
Something went wrong with that request. Please try again.