Skip to content

Commit

Permalink
[PostgreSQL provider] Add missing namespace escaping in SQL requests
Browse files Browse the repository at this point in the history
  • Loading branch information
rouault committed Feb 5, 2024
1 parent a09b3d0 commit 7ba405d
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 46 deletions.
35 changes: 18 additions & 17 deletions src/providers/postgres/qgspostgresconn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@

const int PG_DEFAULT_TIMEOUT = 30;

static QString quotedString( const QString &v )
{
QString result = v;
result.replace( '\'', QLatin1String( "''" ) );
if ( result.contains( '\\' ) )
return result.replace( '\\', QLatin1String( "\\\\" ) ).prepend( "E'" ).append( '\'' );
else
return result.prepend( '\'' ).append( '\'' );
}

QgsPostgresResult::~QgsPostgresResult()
{
if ( mRes )
Expand Down Expand Up @@ -707,10 +717,10 @@ bool QgsPostgresConn::getTableInfo( bool searchGeometryColumnsOnly, bool searchP
sql += QLatin1String( " AND n.nspname='public'" );

if ( !schema.isEmpty() )
sql += QStringLiteral( " AND %1='%2'" ).arg( schemaName, schema );
sql += QStringLiteral( " AND %1=%2" ).arg( schemaName, quotedString( schema ) );

if ( !name.isEmpty() )
sql += QStringLiteral( " AND %1='%2'" ).arg( tableName, name );
sql += QStringLiteral( " AND %1=%2" ).arg( tableName, quotedString( name ) );

sql += QString( " GROUP BY 1,2,3,4,5,6,7,c.oid,11" );

Expand Down Expand Up @@ -850,10 +860,10 @@ bool QgsPostgresConn::getTableInfo( bool searchGeometryColumnsOnly, bool searchP
sql += QLatin1String( " AND n.nspname='public'" );

if ( !schema.isEmpty() )
sql += QStringLiteral( " AND n.nspname='%1'" ).arg( schema );
sql += QStringLiteral( " AND n.nspname=%1" ).arg( quotedString( schema ) );

if ( !name.isEmpty() )
sql += QStringLiteral( " AND c.relname ='%1'" ).arg( name );
sql += QStringLiteral( " AND c.relname=%1" ).arg( quotedString( name ) );

// skip columns of which we already derived information from the metadata tables
if ( nColumns > 0 )
Expand Down Expand Up @@ -992,10 +1002,11 @@ bool QgsPostgresConn::getTableInfo( bool searchGeometryColumnsOnly, bool searchP
sql += QLatin1String( " AND pg_namespace.nspname='public'" );

if ( !schema.isEmpty() )
sql += QStringLiteral( " AND pg_namespace.nspname='%1'" ).arg( schema );
sql += QStringLiteral( " AND pg_namespace.nspname=%1" ).arg( quotedString( schema ) );

if ( !name.isEmpty() )
sql += QStringLiteral( " AND pg_class.relname='%1'" ).arg( name );
sql += QStringLiteral( " AND pg_class.relname=%1" ).arg( quotedString( name ) );


sql += QLatin1String( " GROUP BY 1,2,3,4" );

Expand Down Expand Up @@ -1339,16 +1350,6 @@ QString QgsPostgresConn::quotedIdentifier( const QString &ident )
return result.prepend( '\"' ).append( '\"' );
}

static QString quotedString( const QString &v )
{
QString result = v;
result.replace( '\'', QLatin1String( "''" ) );
if ( result.contains( '\\' ) )
return result.replace( '\\', QLatin1String( "\\\\" ) ).prepend( "E'" ).append( '\'' );
else
return result.prepend( '\'' ).append( '\'' );
}

static QString doubleQuotedMapValue( const QString &v )
{
QString result = v;
Expand Down Expand Up @@ -2963,7 +2964,7 @@ int QgsPostgresConn::crsToSrid( const QgsCoordinateReferenceSystem &crs )
return -1;
const QString authName = authParts.first();
const QString authId = authParts.last();
QgsPostgresResult result( PQexec( QStringLiteral( "SELECT srid FROM spatial_ref_sys WHERE auth_name='%1' AND auth_srid=%2" ).arg( authName, authId ) ) );
QgsPostgresResult result( PQexec( QStringLiteral( "SELECT srid FROM spatial_ref_sys WHERE auth_name=%1 AND auth_srid=%2" ).arg( quotedString( authName ), authId ) ) );

if ( result.PQresultStatus() == PGRES_TUPLES_OK )
{
Expand Down
2 changes: 1 addition & 1 deletion src/providers/postgres/qgspostgresdataitemguiprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ void QgsPostgresDataItemGuiProvider::deleteSchema( QgsPGSchemaItem *schemaItem,
return;
}

const QString sql = QStringLiteral( "SELECT table_name FROM information_schema.tables WHERE table_schema='%1'" ).arg( schemaItem->name() );
const QString sql = QStringLiteral( "SELECT table_name FROM information_schema.tables WHERE table_schema=%1" ).arg( QgsPostgresConn::quotedValue( schemaItem->name() ) );
QgsPostgresResult result( conn->LoggedPQexec( "QgsPostgresDataItemGuiProvider", sql ) );
if ( result.PQresultStatus() != PGRES_TUPLES_OK )
{
Expand Down
5 changes: 2 additions & 3 deletions src/providers/postgres/qgspostgresproviderconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,11 @@ QList<QgsAbstractDatabaseProviderConnection::TableProperty> QgsPostgresProviderC
{
const QList<QVariantList> pks = executeSqlPrivate( QStringLiteral( R"(
WITH pkrelid AS (
SELECT indexrelid AS idxri FROM pg_index WHERE indrelid='%1.%2'::regclass AND (indisprimary OR indisunique)
SELECT indexrelid AS idxri FROM pg_index WHERE indrelid=%1::regclass AND (indisprimary OR indisunique)
ORDER BY CASE WHEN indisprimary THEN 1 ELSE 2 END LIMIT 1)
SELECT attname FROM pg_index,pg_attribute, pkrelid
WHERE indexrelid=pkrelid.idxri AND indrelid=attrelid AND pg_attribute.attnum=any(pg_index.indkey);
)" ).arg( QgsPostgresConn::quotedIdentifier( pr.schemaName ),
QgsPostgresConn::quotedIdentifier( pr.tableName ) ), false );
)" ).arg( QgsPostgresConn::quotedValue( QString( QgsPostgresConn::quotedIdentifier( pr.schemaName ) + "." + QgsPostgresConn::quotedIdentifier( pr.tableName ) ) ) ), false );
QStringList pkNames;
for ( const QVariantList &pk : std::as_const( pks ) )
{
Expand Down
6 changes: 3 additions & 3 deletions tests/src/python/test_processing_importintopostgis.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def setUpClass(cls):
settings.setValue('database', 'qgis_test')

def test_import(self):
"""Test algorithm with CamelCaseSchema"""
"""Test algorithm with CamelCase'singlequote'Schema"""

alg = self.registry.createAlgorithmById("qgis:importintopostgis")
self.assertIsNotNone(alg)
Expand All @@ -76,7 +76,7 @@ def test_import(self):
'LOWERCASE_NAMES': True,
'OVERWRITE': True,
'PRIMARY_KEY': None,
'SCHEMA': 'CamelCaseSchema',
'SCHEMA': "CamelCase'singlequote'Schema",
'TABLENAME': table_name
}

Expand All @@ -90,7 +90,7 @@ def test_import(self):
# Check that data have been imported correctly
exported = QgsVectorLayer(unitTestDataPath() + '/points.shp', 'exported')
self.assertTrue(exported.isValid())
imported = QgsVectorLayer(f"service='qgis_test' table=\"CamelCaseSchema\".\"{table_name}\" (geom)", 'imported', 'postgres')
imported = QgsVectorLayer(f"service='qgis_test' table=\"CamelCase'singlequote'Schema\".\"{table_name}\" (geom)", 'imported', 'postgres')
self.assertTrue(imported.isValid())
imported_fields = [f.name() for f in imported.fields()]
for f in exported.fields():
Expand Down
12 changes: 6 additions & 6 deletions tests/src/python/test_qgsdatabaseschemacombobox.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ def testCombo(self):
self.assertGreaterEqual(m.comboBox().count(), 3)

text = [m.comboBox().itemText(i) for i in range(m.comboBox().count())]
self.assertIn('CamelCaseSchema', text)
self.assertIn("CamelCase'singlequote'Schema", text)
self.assertIn('qgis_test', text)
self.assertLess(text.index('CamelCaseSchema'), text.index('qgis_test'))
self.assertEqual(m.currentSchema(), 'CamelCaseSchema')
self.assertLess(text.index("CamelCase'singlequote'Schema"), text.index('qgis_test'))
self.assertEqual(m.currentSchema(), "CamelCase'singlequote'Schema")

m.setSchema('qgis_test')
self.assertEqual(m.currentSchema(), 'qgis_test')
Expand Down Expand Up @@ -124,10 +124,10 @@ def testComboWithEmpty(self):

text = [m.comboBox().itemText(i) for i in range(m.comboBox().count())]
self.assertFalse(text[0])
self.assertIn('CamelCaseSchema', text)
self.assertIn("CamelCase'singlequote'Schema", text)
self.assertIn('qgis_test', text)
self.assertLess(text.index('CamelCaseSchema'), text.index('qgis_test'))
self.assertEqual(m.currentSchema(), 'CamelCaseSchema')
self.assertLess(text.index("CamelCase'singlequote'Schema"), text.index('qgis_test'))
self.assertEqual(m.currentSchema(), "CamelCase'singlequote'Schema")

m.setSchema('qgis_test')
self.assertEqual(m.currentSchema(), 'qgis_test')
Expand Down
20 changes: 10 additions & 10 deletions tests/src/python/test_qgsdatabaseschemamodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def testModel(self):
self.assertEqual(model.columnCount(), 1)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertEqual(model.data(model.index(schemas.index('qgis_test'), 0, QModelIndex()), Qt.ItemDataRole.ToolTipRole), 'qgis_test')
self.assertIsNone(model.data(model.index(model.rowCount(), 0, QModelIndex()), Qt.ItemDataRole.DisplayRole))
Expand All @@ -62,7 +62,7 @@ def testModel(self):
self.assertEqual(model.rowCount(), old_count + 1)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertIn('myNewSchema', schemas)

Expand All @@ -72,7 +72,7 @@ def testModel(self):
self.assertEqual(model.rowCount(), old_count + 3)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertIn('myNewSchema', schemas)
self.assertIn('myNewSchema2', schemas)
Expand All @@ -85,7 +85,7 @@ def testModel(self):
self.assertEqual(model.rowCount(), old_count + 2)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertNotIn('myNewSchema', schemas)
self.assertNotIn('myNewSchema2', schemas)
Expand All @@ -98,7 +98,7 @@ def testModel(self):
self.assertEqual(model.rowCount(), old_count)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertNotIn('myNewSchema3', schemas)
self.assertNotIn('myNewSchema4', schemas)
Expand All @@ -115,7 +115,7 @@ def test_model_allow_empty(self):

schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertFalse(model.data(model.index(0, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole))
self.assertTrue(model.data(model.index(0, 0, QModelIndex()), QgsDatabaseSchemaModel.Role.RoleEmpty))
Expand All @@ -131,7 +131,7 @@ def test_model_allow_empty(self):
self.assertEqual(model.rowCount(), old_count + 2)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertIn('myNewSchema', schemas)
self.assertFalse(model.data(model.index(0, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole))
Expand All @@ -154,7 +154,7 @@ def test_model_allow_empty(self):
self.assertEqual(model.rowCount(), old_count + 4)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertIn('myNewSchema', schemas)
self.assertIn('myNewSchema2', schemas)
Expand All @@ -170,7 +170,7 @@ def test_model_allow_empty(self):
self.assertEqual(model.rowCount(), old_count + 3)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertNotIn('myNewSchema', schemas)
self.assertNotIn('myNewSchema2', schemas)
Expand All @@ -186,7 +186,7 @@ def test_model_allow_empty(self):
self.assertEqual(model.rowCount(), old_count + 1)
schemas = [model.data(model.index(r, 0, QModelIndex()), Qt.ItemDataRole.DisplayRole) for r in range(model.rowCount())]
self.assertIn('public', schemas)
self.assertIn('CamelCaseSchema', schemas)
self.assertIn("CamelCase'singlequote'Schema", schemas)
self.assertIn('qgis_test', schemas)
self.assertNotIn('myNewSchema3', schemas)
self.assertNotIn('myNewSchema4', schemas)
Expand Down
12 changes: 6 additions & 6 deletions tests/testdata/provider/testdata_pg.sql
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ ALTER DEFAULT PRIVILEGES IN SCHEMA qgis_test GRANT ALL ON TABLES TO public;
ALTER DEFAULT PRIVILEGES IN SCHEMA qgis_test GRANT ALL ON SEQUENCES TO public;


--- Create "CamelCaseSchema" schema
DROP SCHEMA IF EXISTS "CamelCaseSchema" CASCADE;
CREATE SCHEMA "CamelCaseSchema";
GRANT ALL ON SCHEMA "CamelCaseSchema" TO public;
ALTER DEFAULT PRIVILEGES IN SCHEMA "CamelCaseSchema" GRANT ALL ON TABLES TO public;
ALTER DEFAULT PRIVILEGES IN SCHEMA "CamelCaseSchema" GRANT ALL ON SEQUENCES TO public;
--- Create "CamelCase'singlequote'Schema" schema
DROP SCHEMA IF EXISTS "CamelCase'singlequote'Schema" CASCADE;
CREATE SCHEMA "CamelCase'singlequote'Schema";
GRANT ALL ON SCHEMA "CamelCase'singlequote'Schema" TO public;
ALTER DEFAULT PRIVILEGES IN SCHEMA "CamelCase'singlequote'Schema" GRANT ALL ON TABLES TO public;
ALTER DEFAULT PRIVILEGES IN SCHEMA "CamelCase'singlequote'Schema" GRANT ALL ON SEQUENCES TO public;


SET default_tablespace = '';
Expand Down

0 comments on commit 7ba405d

Please sign in to comment.