Skip to content

Commit

Permalink
Reliably parse query params from connection string (#485)
Browse files Browse the repository at this point in the history
The previous regex used the match query options was incorrect since it only
captured words (`\w+`) values from query params. This would cause params with
values such as `&host=/path/to/my/socket` to be parsed as `nil`, which is
incorrect and will cause pain on some hosting providers (i.e. Google Cloud
requires a path to a socket).

Parsing query params can be brittle, so we opt to let `CGI.parse` to do the
heavy lifting since it is more battle-tested.
  • Loading branch information
ianks authored and mereghost committed Jul 14, 2018
1 parent 75c6141 commit a5edb7e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
9 changes: 7 additions & 2 deletions lib/hanami/model/migrator/connection.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'cgi'

module Hanami
module Model
class Migrator
Expand Down Expand Up @@ -153,8 +155,11 @@ def table(name)
#
# @since 0.5.0
# @api private
def parsed_opt(option)
parsed_uri.to_s.match(/[\?|\&]#{ option }=(\w+)\&?/).to_a.last
def parsed_opt(option, query: parsed_uri.query)
return if query.nil?

@parsed_query_opts ||= CGI.parse(query)
@parsed_query_opts[option].to_a.last
end
end
end
Expand Down
32 changes: 30 additions & 2 deletions spec/unit/hanami/model/migrator/connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,36 @@
end

describe '#host' do
it 'returns configured host' do
expect(connection.host).to eq('127.0.0.1')
describe 'when the host is only specified in the URI' do
let(:url) { "postgresql://127.0.0.1/database" }

it 'returns configured host' do
expect(connection.host).to eq('127.0.0.1')
end
end

describe 'when the host is only specified in the query' do
let(:url) { "postgresql:///database?host=0.0.0.0" }

it 'returns the host specified in the query param' do
expect(connection.host).to eql('0.0.0.0')
end
end

describe 'when the host is specified as a socket' do
let(:url) { "postgresql:///database?host=/path/to/my/sock" }

it 'returns the path to the socket specified in the query param' do
expect(connection.host).to eql('/path/to/my/sock')
end
end

describe 'when the host is specified in both the URI and query' do
let(:url) { "postgresql://127.0.0.1/database?host=0.0.0.0" }

it 'prefers the host from the URI' do
expect(connection.host).to eql('127.0.0.1')
end
end
end

Expand Down

0 comments on commit a5edb7e

Please sign in to comment.