Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[gdal] Allow storing credential key/value credential options in uris #57801

Closed
wants to merge 3 commits into from
Closed

[gdal] Allow storing credential key/value credential options in uris #57801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b852ea8
[gdal] Ensure cloud vsi prefixes are correctly decoded
nyalldawson Jun 19, 2024
0ebf240
[gdal] Allow storing credential key/value credential options in uris
nyalldawson Jun 19, 2024
dbe9d3d
Fix warning on older GDAL builds
nyalldawson Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 67 additions & 2 deletions src/core/providers/gdal/qgsgdalproviderbase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "qgsgdalproviderbase.h"
#include "qgsgdalutils.h"
#include "qgssettings.h"
#include "qgsmessagelog.h"

#include <mutex>
#include <QRegularExpression>
Expand Down Expand Up @@ -290,6 +291,32 @@ GDALDatasetH QgsGdalProviderBase::gdalOpen( const QString &uri, unsigned int nOp
option.toUtf8().constData() );
}

const QString vsiPrefix = parts.value( QStringLiteral( "vsiPrefix" ) ).toString();
const QString vsiSuffix = parts.value( QStringLiteral( "vsiSuffix" ) ).toString();

const QVariantMap credentialOptions = parts.value( QStringLiteral( "credentialOptions" ) ).toMap();
parts.remove( QStringLiteral( "credentialOptions" ) );
if ( !credentialOptions.isEmpty() && !vsiPrefix.isEmpty() )
{
const thread_local QRegularExpression bucketRx( QStringLiteral( "^(.*?)/" ) );
const QRegularExpressionMatch bucketMatch = bucketRx.match( parts.value( QStringLiteral( "path" ) ).toString() );
if ( bucketMatch.hasMatch() )
{
const QString bucket = vsiPrefix + bucketMatch.captured( 1 );
for ( auto it = credentialOptions.constBegin(); it != credentialOptions.constEnd(); ++it )
{
#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3, 6, 0)
VSISetPathSpecificOption( bucket.toLocal8Bit().constData(), it.key().toLocal8Bit().constData(), it.value().toString().toLocal8Bit().constData() );
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rouault does this approach look valid to you?

#elif GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3, 5, 0)
VSISetCredential( bucket.toLocal8Bit().constData(), it.key().toLocal8Bit().constData(), it.value().toString().toLocal8Bit().constData() );
Comment on lines +309 to +311
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VSISetPathSpecificOption( bucket.toLocal8Bit().constData(), it.key().toLocal8Bit().constData(), it.value().toString().toLocal8Bit().constData() );
#elif GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3, 5, 0)
VSISetCredential( bucket.toLocal8Bit().constData(), it.key().toLocal8Bit().constData(), it.value().toString().toLocal8Bit().constData() );
VSISetPathSpecificOption( bucket.toUtf8().constData(), it.key().toUtf8().constData(), it.value().toString().toUtf8().constData() );
#elif GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3, 5, 0)
VSISetCredential( bucket.toUtf8().constData(), it.key().toUtf8().constData(), it.value().toString().toUtf8().constData() );

#else
( void )bucket;
QgsMessageLog::logMessage( QObject::tr( "Cannot use VSI credential options on GDAL versions earlier than 3.5" ), QStringLiteral( "GDAL" ), Qgis::MessageLevel::Critical );
#endif
}
}
}

const bool modify_OGR_GPKG_FOREIGN_KEY_CHECK = !CPLGetConfigOption( "OGR_GPKG_FOREIGN_KEY_CHECK", nullptr );
if ( modify_OGR_GPKG_FOREIGN_KEY_CHECK )
{
Expand All @@ -301,8 +328,6 @@ GDALDatasetH QgsGdalProviderBase::gdalOpen( const QString &uri, unsigned int nOp

if ( !hDS )
{
const QString vsiPrefix = parts.value( QStringLiteral( "vsiPrefix" ) ).toString();
const QString vsiSuffix = parts.value( QStringLiteral( "vsiSuffix" ) ).toString();
if ( vsiSuffix.isEmpty() && QgsGdalUtils::isVsiArchivePrefix( vsiPrefix ) )
{
// in the case that a direct path to a vsi supported archive was specified BUT
Expand Down Expand Up @@ -391,6 +416,7 @@ QVariantMap QgsGdalProviderBase::decodeGdalUri( const QString &uri )
QString layerName;
QString authcfg;
QStringList openOptions;
QVariantMap credentialOptions;

const thread_local QRegularExpression authcfgRegex( " authcfg='([^']+)'" );
QRegularExpressionMatch match;
Expand Down Expand Up @@ -451,13 +477,39 @@ QVariantMap QgsGdalProviderBase::decodeGdalUri( const QString &uri )
break;
}
}

const thread_local QRegularExpression credentialOptionRegex( QStringLiteral( "\\|credential:([^|]*)" ) );
const thread_local QRegularExpression credentialOptionKeyValueRegex( QStringLiteral( "(.*?)=(.*)" ) );
while ( true )
{
const QRegularExpressionMatch match = credentialOptionRegex.match( path );
if ( match.hasMatch() )
{
const QRegularExpressionMatch keyValueMatch = credentialOptionKeyValueRegex.match( match.captured( 1 ) );
if ( keyValueMatch.hasMatch() )
{
credentialOptions.insert( keyValueMatch.captured( 1 ), keyValueMatch.captured( 2 ) );
}
else
{
credentialOptions.insert( match.captured( 1 ), QString() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not completely sure if we need to handle a credential that is a key without value. I can't think of a case where it is needed currently. Unless this is meant to remove a path specific option. But in that case VSISetPathSpecificOption() should be called with a nullptr value and not an empty string

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll remove this support. (I wasn't sure if in future there'd be valueless options added in gdal)

}
path = path.remove( match.capturedStart( 0 ), match.capturedLength( 0 ) );
}
else
{
break;
}
}
}

QVariantMap uriComponents;
uriComponents.insert( QStringLiteral( "path" ), path );
uriComponents.insert( QStringLiteral( "layerName" ), layerName );
if ( !openOptions.isEmpty() )
uriComponents.insert( QStringLiteral( "openOptions" ), openOptions );
if ( !credentialOptions.isEmpty() )
uriComponents.insert( QStringLiteral( "credentialOptions" ), credentialOptions );
if ( !vsiPrefix.isEmpty() )
uriComponents.insert( QStringLiteral( "vsiPrefix" ), vsiPrefix );
if ( !vsiSuffix.isEmpty() )
Expand Down Expand Up @@ -494,6 +546,19 @@ QString QgsGdalProviderBase::encodeGdalUri( const QVariantMap &parts )
uri += openOption;
}

const QVariantMap credentialOptions = parts.value( QStringLiteral( "credentialOptions" ) ).toMap();
for ( auto it = credentialOptions.constBegin(); it != credentialOptions.constEnd(); ++it )
{
if ( !it.value().toString().isEmpty() )
{
uri += QStringLiteral( "|credential:%1=%2" ).arg( it.key(), it.value().toString() );
}
else
{
uri += QStringLiteral( "|credential:%1" ).arg( it.key() );
}
}

if ( !authcfg.isEmpty() )
uri += QStringLiteral( " authcfg='%1'" ).arg( authcfg );

Expand Down
9 changes: 4 additions & 5 deletions src/core/qgsgdalutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,11 +711,10 @@ QString QgsGdalUtils::vsiPrefixForPath( const QString &path )
{
const QStringList vsiPrefixes = QgsGdalUtils::vsiArchivePrefixes();

for ( const QString &vsiPrefix : vsiPrefixes )
{
if ( path.startsWith( vsiPrefix, Qt::CaseInsensitive ) )
return vsiPrefix;
}
const thread_local QRegularExpression vsiRx( QStringLiteral( "^(/vsi.+?/)" ), QRegularExpression::PatternOption::CaseInsensitiveOption );
const QRegularExpressionMatch vsiMatch = vsiRx.match( path );
if ( vsiMatch.hasMatch() )
return vsiMatch.captured( 1 );

if ( path.endsWith( QLatin1String( ".shp.zip" ), Qt::CaseInsensitive ) )
{
Expand Down
52 changes: 51 additions & 1 deletion tests/src/core/testqgsgdalprovider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class TestQgsGdalProvider : public QgsTest
void testGdalProviderQuerySublayersFastScan();
void testGdalProviderQuerySublayersFastScan_NetCDF();
void testGdalProviderAbsoluteRelativeUri();
void testVsiCredentialOptions();

private:
QString mTestDataDir;
Expand Down Expand Up @@ -138,8 +139,15 @@ void TestQgsGdalProvider::decodeUri()
// test authcfg with vsicurl URI
uri = QStringLiteral( "/vsicurl/https://www.qgis.org/dataset.tif authcfg='1234567'" );
components = QgsProviderRegistry::instance()->decodeUri( QStringLiteral( "gdal" ), uri );
QCOMPARE( components.value( QStringLiteral( "path" ) ).toString(), QString( "/vsicurl/https://www.qgis.org/dataset.tif" ) );
QCOMPARE( components.value( QStringLiteral( "path" ) ).toString(), QString( "https://www.qgis.org/dataset.tif" ) );
QCOMPARE( components.value( QStringLiteral( "vsiPrefix" ) ).toString(), QString( "/vsicurl/" ) );
QCOMPARE( components.value( QStringLiteral( "authcfg" ) ).toString(), QString( "1234567" ) );

// vsis3
uri = QStringLiteral( "/vsis3/nz-elevation/auckland/auckland-north_2016-2018/dem_1m/2193/AY30_10000_0405.tiff" );
components = QgsProviderRegistry::instance()->decodeUri( QStringLiteral( "gdal" ), uri );
QCOMPARE( components.value( QStringLiteral( "path" ) ).toString(), QString( "nz-elevation/auckland/auckland-north_2016-2018/dem_1m/2193/AY30_10000_0405.tiff" ) );
QCOMPARE( components.value( QStringLiteral( "vsiPrefix" ) ).toString(), QString( "/vsis3/" ) );
}

void TestQgsGdalProvider::encodeUri()
Expand All @@ -162,6 +170,17 @@ void TestQgsGdalProvider::encodeUri()
parts.insert( QStringLiteral( "path" ), QStringLiteral( "/vsicurl/https://www.qgis.org/dataset.tif" ) );
parts.insert( QStringLiteral( "authcfg" ), QStringLiteral( "1234567" ) );
QCOMPARE( QgsProviderRegistry::instance()->encodeUri( QStringLiteral( "gdal" ), parts ), QStringLiteral( "/vsicurl/https://www.qgis.org/dataset.tif authcfg='1234567'" ) );
parts.clear();
parts.insert( QStringLiteral( "path" ), QStringLiteral( "https://www.qgis.org/dataset.tif" ) );
parts.insert( QStringLiteral( "vsiPrefix" ), QStringLiteral( "/vsicurl/" ) );
parts.insert( QStringLiteral( "authcfg" ), QStringLiteral( "1234567" ) );
QCOMPARE( QgsProviderRegistry::instance()->encodeUri( QStringLiteral( "gdal" ), parts ), QStringLiteral( "/vsicurl/https://www.qgis.org/dataset.tif authcfg='1234567'" ) );

// vsis3
parts.clear();
parts.insert( QStringLiteral( "vsiPrefix" ), QStringLiteral( "/vsis3/" ) );
parts.insert( QStringLiteral( "path" ), QStringLiteral( "nz-elevation/auckland/auckland-north_2016-2018/dem_1m/2193/AY30_10000_0405.tiff" ) );
QCOMPARE( QgsProviderRegistry::instance()->encodeUri( QStringLiteral( "gdal" ), parts ), QStringLiteral( "/vsis3/nz-elevation/auckland/auckland-north_2016-2018/dem_1m/2193/AY30_10000_0405.tiff" ) );
}

void TestQgsGdalProvider::scaleDataType()
Expand Down Expand Up @@ -888,5 +907,36 @@ void TestQgsGdalProvider::testGdalProviderAbsoluteRelativeUri()
QCOMPARE( mGdalMetadata->relativeToAbsoluteUri( relativeUri, context ), absoluteUri );
}

void TestQgsGdalProvider::testVsiCredentialOptions()
{
#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3, 6, 0)
// test that credential options are correctly set when layer URI specifies them
std::unique_ptr< QgsRasterLayer > rl = std::make_unique< QgsRasterLayer >( QStringLiteral( "/vsis3/testbucket/test|credential:AWS_NO_SIGN_REQUEST=YES|credential:AWS_REGION=eu-central-1|credential:AWS_S3_ENDPOINT=localhost" ), QStringLiteral( "test" ), QStringLiteral( "gdal" ) );

// confirm that GDAL VSI configuration options are set
QString noSign( VSIGetPathSpecificOption( "/vsis3/testbucket", "AWS_NO_SIGN_REQUEST", nullptr ) );
QCOMPARE( noSign, QStringLiteral( "YES" ) );
QString region( VSIGetPathSpecificOption( "/vsis3/testbucket", "AWS_REGION", nullptr ) );
QCOMPARE( region, QStringLiteral( "eu-central-1" ) );

// different bucket
noSign = QString( VSIGetPathSpecificOption( "/vsis3/another", "AWS_NO_SIGN_REQUEST", nullptr ) );
QCOMPARE( noSign, QString() );
region = QString( VSIGetPathSpecificOption( "/vsis3/another", "AWS_REGION", nullptr ) );
QCOMPARE( region, QString() );

// credentials should be bucket specific
std::unique_ptr< QgsRasterLayer > rl2 = std::make_unique< QgsRasterLayer >( QStringLiteral( "/vsis3/another/test|credential:AWS_NO_SIGN_REQUEST=NO|credential:AWS_REGION=eu-central-2|credential:AWS_S3_ENDPOINT=localhost" ), QStringLiteral( "test" ), QStringLiteral( "gdal" ) );
noSign = QString( VSIGetPathSpecificOption( "/vsis3/testbucket", "AWS_NO_SIGN_REQUEST", nullptr ) );
QCOMPARE( noSign, QStringLiteral( "YES" ) );
region = QString( VSIGetPathSpecificOption( "/vsis3/testbucket", "AWS_REGION", nullptr ) );
QCOMPARE( region, QStringLiteral( "eu-central-1" ) );
noSign = QString( VSIGetPathSpecificOption( "/vsis3/another", "AWS_NO_SIGN_REQUEST", nullptr ) );
QCOMPARE( noSign, QStringLiteral( "NO" ) );
region = QString( VSIGetPathSpecificOption( "/vsis3/another", "AWS_REGION", nullptr ) );
QCOMPARE( region, QStringLiteral( "eu-central-2" ) );
#endif
}

QGSTEST_MAIN( TestQgsGdalProvider )
#include "testqgsgdalprovider.moc"
17 changes: 17 additions & 0 deletions tests/src/python/test_provider_gdal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,23 @@ def testDecodeEncodeUriOptions(self):
encodedUri = QgsProviderRegistry.instance().encodeUri('gdal', parts)
self.assertEqual(encodedUri, uri)

def testDecodeEncodeUriCredentialOptions(self):
"""Test decodeUri/encodeUri credential options support"""

uri = '/my/raster.pdf|option:AN=OPTION|credential:ANOTHER|credential:SOMEKEY=AAAAA'
parts = QgsProviderRegistry.instance().decodeUri('gdal', uri)
self.assertEqual(parts, {
'path': '/my/raster.pdf',
'layerName': None,
'credentialOptions': {
'ANOTHER': None,
'SOMEKEY': 'AAAAA'
},
'openOptions': ['AN=OPTION']
})
encodedUri = QgsProviderRegistry.instance().encodeUri('gdal', parts)
self.assertEqual(encodedUri, uri)

def testDecodeEncodeUriVsizip(self):
"""Test decodeUri/encodeUri for /vsizip/ prefixed URIs"""

Expand Down
Loading