Decode attributes pulled from URI.parse #7593

Merged
merged 1 commit into from Oct 26, 2012

Projects

None yet

5 participants

@veader
Contributor
veader commented Sep 10, 2012

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.

@al2o3cr
Contributor
al2o3cr commented Sep 10, 2012

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

@veader
Contributor
veader commented Sep 11, 2012

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

@frodsan
Contributor
frodsan commented Oct 26, 2012

Could you squash your commits? Thanks.

@veader
Contributor
veader commented Oct 26, 2012

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

@frodsan
Contributor
frodsan commented Oct 26, 2012
@rafaelfranca rafaelfranca and 2 others commented on an outdated diff Oct 26, 2012
...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
@rafaelfranca
rafaelfranca Oct 26, 2012 Member

When the Parser is not available?

@veader
veader Oct 26, 2012 Contributor

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

@frodsan
frodsan Oct 26, 2012 Contributor

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

@rafaelfranca rafaelfranca and 2 others commented on an outdated diff Oct 26, 2012
...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) }
@rafaelfranca
rafaelfranca Oct 26, 2012 Member

val.is_a?(String)

@rafaelfranca
rafaelfranca Oct 26, 2012 Member

|key, value|

@veader
veader Oct 26, 2012 Contributor

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.

@steveklabnik
steveklabnik Oct 26, 2012 Member

is_a? and kind_of? are aliases, yes.

@veader
veader Oct 26, 2012 Contributor

I do believe they are.

@rafaelfranca
Member

Could you add a CHANGELOG entry?

@veader
Contributor
veader commented Oct 26, 2012

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

@veader
Contributor
veader commented Oct 26, 2012

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

@rafaelfranca rafaelfranca commented on an outdated diff Oct 26, 2012
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*
@rafaelfranca
rafaelfranca Oct 26, 2012 Member

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

@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
@veader
Contributor
veader commented Oct 26, 2012

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

@rafaelfranca
Member

Thank you so much for the pull request.

@rafaelfranca rafaelfranca merged commit 35f2a09 into rails:master Oct 26, 2012
@frodsan
Contributor
frodsan commented Oct 26, 2012

👍 Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment