Skip to content

Commit

Permalink
[9.x] Fix parsing config('database.connections.pgsql.search_path')
Browse files Browse the repository at this point in the history 
The given PostgresConnector regex doesn't consider the full range of
characters allowed in a schema or variable name - specifically '-' and
accented characters.

e.g., 'test-db' was being parsed as `set search_path to "test", "db"`
instead of `set search_path to "test-db"`

Replace the 'search_path' regex allowlist of characters with a blocklist
of  delimiters when config('database.connections.pgsql.search_path') is
a string value.

Technically Postgres _does_ allow our config delimiter characters
(spaces, comma, quotes) in symbols so an array configuration can
instead be used for such schema names. However single/double quote
characters in such array configs aren't supported.

---

* Roll methods testPostgresSearchPathCommaSeparatedValueSupported() &
  testPostgresSearchPathVariablesSupported() into
  testPostgresSearchPathIsSet() with the provideSearchPaths() data set.
* testPostgresSearchPathArraySupported() is repurposed to show
  config('database.connections.pgsql.schema') from versions < 9.x is
  used when the 'search_path' config key is absent.
* Fix PostgresConnector::quoteSearchPath() docblock since passing in a
  string value would throw an exception for being un-Countable. Its only
  use is being given parseSearchPath()'s return value which is an array.
  • Loading branch information
@derekmd
derekmd committed Feb 17, 2022
1 parent e4c93c4 commit e1a9467
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/Illuminate/Database/Connectors/PostgresConnector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ protected function configureSearchPath($connection, $config)
protected function parseSearchPath($searchPath)
{
if (is_string($searchPath)) {
preg_match_all('/[a-zA-z0-9$]{1,}/i', $searchPath, $matches);
preg_match_all('/[^\s,"\']+/', $searchPath, $matches);

$searchPath = $matches[0];
}
Expand All @@ -144,7 +144,7 @@ protected function parseSearchPath($searchPath)
/**
* Format the search path for the DSN.
*
* @param array|string $searchPath
* @param array $searchPath
* @return string
*/
protected function quoteSearchPath($searchPath)
Expand Down
118 changes: 81 additions & 37 deletions tests/Database/DatabaseConnectorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,27 +89,103 @@ public function testPostgresConnectCallsCreateConnectionWithProperArguments()
$this->assertSame($result, $connection);
}

public function testPostgresSearchPathIsSet()
/**
* @dataProvider provideSearchPaths
*
* @param string $searchPath
* @param string $expectedSql
*/
public function testPostgresSearchPathIsSet($searchPath, $expectedSql)
{
$dsn = 'pgsql:host=foo;dbname=\'bar\'';
$config = ['host' => 'foo', 'database' => 'bar', 'search_path' => 'public', 'charset' => 'utf8'];
$config = ['host' => 'foo', 'database' => 'bar', 'search_path' => $searchPath, 'charset' => 'utf8'];
$connector = $this->getMockBuilder(PostgresConnector::class)->onlyMethods(['createConnection', 'getOptions'])->getMock();
$connection = m::mock(stdClass::class);
$connector->expects($this->once())->method('getOptions')->with($this->equalTo($config))->willReturn(['options']);
$connector->expects($this->once())->method('createConnection')->with($this->equalTo($dsn), $this->equalTo($config), $this->equalTo(['options']))->willReturn($connection);
$statement = m::mock(PDOStatement::class);
$connection->shouldReceive('prepare')->once()->with('set names \'utf8\'')->andReturn($statement);
$connection->shouldReceive('prepare')->once()->with('set search_path to "public"')->andReturn($statement);
$connection->shouldReceive('prepare')->once()->with($expectedSql)->andReturn($statement);
$statement->shouldReceive('execute')->twice();
$result = $connector->connect($config);

$this->assertSame($result, $connection);
}

public function testPostgresSearchPathArraySupported()
public function provideSearchPaths()
{
return [
'all-lowercase' => [
'public',
'set search_path to "public"',
],
'case-sensitive' => [
'Public',
'set search_path to "Public"',
],
'special characters' => [
'¡foo_bar-Baz!.Áüõß',
'set search_path to "¡foo_bar-Baz!.Áüõß"',
],
'single-quoted' => [
"'public'",
'set search_path to "public"',
],
'double-quoted' => [
'"public"',
'set search_path to "public"',
],
'variable' => [
'$user',
'set search_path to "$user"',
],
'delimit space' => [
'public user',
'set search_path to "public", "user"',
],
'delimit newline' => [
"public\nuser\r\n\ttest",
'set search_path to "public", "user", "test"',
],
'delimit comma' => [
'public,user',
'set search_path to "public", "user"',
],
'delimit comma and space' => [
'public, user',
'set search_path to "public", "user"',
],
'single-quoted many' => [
"'public', 'user'",
'set search_path to "public", "user"',
],
'double-quoted many' => [
'"public", "user"',
'set search_path to "public", "user"',
],
'quoted space is unsupported in string' => [
'"public user"',
'set search_path to "public", "user"',
],
'array' => [
['public', 'user'],
'set search_path to "public", "user"',
],
'array with variable' => [
['public', '$user'],
'set search_path to "public", "$user"',
],
'array with delimiter characters' => [
['public', '"user"', "'test'", 'spaced schema'],
'set search_path to "public", "user", "test", "spaced schema"',
],
];
}

public function testPostgresSearchPathFallbackToConfigKeySchema()
{
$dsn = 'pgsql:host=foo;dbname=\'bar\'';
$config = ['host' => 'foo', 'database' => 'bar', 'search_path' => ['public', '"user"'], 'charset' => 'utf8'];
$config = ['host' => 'foo', 'database' => 'bar', 'schema' => ['public', '"user"'], 'charset' => 'utf8'];
$connector = $this->getMockBuilder(PostgresConnector::class)->onlyMethods(['createConnection', 'getOptions'])->getMock();
$connection = m::mock(stdClass::class);
$connector->expects($this->once())->method('getOptions')->with($this->equalTo($config))->willReturn(['options']);
Expand All @@ -123,38 +199,6 @@ public function testPostgresSearchPathArraySupported()
$this->assertSame($result, $connection);
}

public function testPostgresSearchPathCommaSeparatedValueSupported()
{
$dsn = 'pgsql:host=foo;dbname=\'bar\'';
$config = ['host' => 'foo', 'database' => 'bar', 'search_path' => 'public, "user"', 'charset' => 'utf8'];
$connector = $this->getMockBuilder('Illuminate\Database\Connectors\PostgresConnector')->setMethods(['createConnection', 'getOptions'])->getMock();
$connection = m::mock('stdClass');
$connector->expects($this->once())->method('getOptions')->with($this->equalTo($config))->willReturn(['options']);
$connector->expects($this->once())->method('createConnection')->with($this->equalTo($dsn), $this->equalTo($config), $this->equalTo(['options']))->willReturn($connection);
$connection->shouldReceive('prepare')->once()->with('set names \'utf8\'')->andReturn($connection);
$connection->shouldReceive('prepare')->once()->with('set search_path to "public", "user"')->andReturn($connection);
$connection->shouldReceive('execute')->twice();
$result = $connector->connect($config);

$this->assertSame($result, $connection);
}

public function testPostgresSearchPathVariablesSupported()
{
$dsn = 'pgsql:host=foo;dbname=\'bar\'';
$config = ['host' => 'foo', 'database' => 'bar', 'search_path' => '"$user", public, user', 'charset' => 'utf8'];
$connector = $this->getMockBuilder('Illuminate\Database\Connectors\PostgresConnector')->setMethods(['createConnection', 'getOptions'])->getMock();
$connection = m::mock('stdClass');
$connector->expects($this->once())->method('getOptions')->with($this->equalTo($config))->willReturn(['options']);
$connector->expects($this->once())->method('createConnection')->with($this->equalTo($dsn), $this->equalTo($config), $this->equalTo(['options']))->willReturn($connection);
$connection->shouldReceive('prepare')->once()->with('set names \'utf8\'')->andReturn($connection);
$connection->shouldReceive('prepare')->once()->with('set search_path to "$user", "public", "user"')->andReturn($connection);
$connection->shouldReceive('execute')->twice();
$result = $connector->connect($config);

$this->assertSame($result, $connection);
}

public function testPostgresApplicationNameIsSet()
{
$dsn = 'pgsql:host=foo;dbname=\'bar\'';
Expand Down

0 comments on commit e1a9467

Please sign in to comment.