Skip to content

Commit

Permalink
[server] Make restoration of original layer subsetStrings much more
Browse files Browse the repository at this point in the history 
robust (there were many code paths where the original filters
were not being correctly restored)
  • Loading branch information
@nyalldawson
nyalldawson committed Feb 22, 2016
1 parent a46dca5 commit f264799
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 43 deletions.
2 changes: 1 addition & 1 deletion src/server/qgsowsserver.cpp
View file
Expand Up @@ -49,7 +49,7 @@ void QgsOWSServer::applyAccessControlLayerFilters( QgsMapLayer* mapLayer, QHash<
#endif #endif


/** Restore layer filter as original */ /** Restore layer filter as original */
void QgsOWSServer::restoreLayerFilters( const QHash<QgsMapLayer*, QString>& filterMap ) const void QgsOWSServer::restoreLayerFilters( const QHash<QgsMapLayer*, QString>& filterMap )
{ {
QHash<QgsMapLayer*, QString>::const_iterator filterIt = filterMap.constBegin(); QHash<QgsMapLayer*, QString>::const_iterator filterIt = filterMap.constBegin();
for ( ; filterIt != filterMap.constEnd(); ++filterIt ) for ( ; filterIt != filterMap.constEnd(); ++filterIt )
Expand Down
35 changes: 32 additions & 3 deletions src/server/qgsowsserver.h
View file
Expand Up @@ -23,6 +23,8 @@
#include "qgsaccesscontrol.h" #include "qgsaccesscontrol.h"
#endif #endif


#include <QHash>

class QgsOWSServer class QgsOWSServer
{ {
public: public:
Expand All @@ -45,6 +47,11 @@ class QgsOWSServer


virtual void executeRequest() = 0; virtual void executeRequest() = 0;


/** Restores the original layer filters
* @param filterMap the original layer filter
*/
static void restoreLayerFilters( const QHash < QgsMapLayer*, QString >& filterMap );

private: private:
QgsOWSServer() {} QgsOWSServer() {}


Expand All @@ -64,10 +71,32 @@ class QgsOWSServer
void applyAccessControlLayerFilters( QgsMapLayer* layer, QHash<QgsMapLayer*, QString>& originalLayerFilters ) const; void applyAccessControlLayerFilters( QgsMapLayer* layer, QHash<QgsMapLayer*, QString>& originalLayerFilters ) const;
#endif #endif


/** Restores the original layer filters };
* @param filterMap the original layer filter
/** RAII class to restore layer filters on destruction
*/
class QgsOWSServerFilterRestorer
{
public:

QgsOWSServerFilterRestorer() {}

//! Destructor. When object is destroyed all original layer filters will be restored.
~QgsOWSServerFilterRestorer()
{
QgsOWSServer::restoreLayerFilters( mOriginalLayerFilters );
}

/** Returns a reference to the object's hash of layers to original subsetString filters.
* Original layer subsetString filters MUST be inserted into this hash before modifying them.
*/ */
void restoreLayerFilters( const QHash < QgsMapLayer*, QString >& filterMap ) const; QHash<QgsMapLayer*, QString>& originalFilters() { return mOriginalLayerFilters; }

private:
QHash<QgsMapLayer*, QString> mOriginalLayerFilters;

QgsOWSServerFilterRestorer( const QgsOWSServerFilterRestorer& rh );
QgsOWSServerFilterRestorer& operator=( const QgsOWSServerFilterRestorer& rh );
}; };


#endif // QGSOWSSERVER_H #endif // QGSOWSSERVER_H
21 changes: 8 additions & 13 deletions src/server/qgswfsserver.cpp
View file
Expand Up @@ -422,7 +422,11 @@ int QgsWFSServer::getFeature( QgsRequestHandler& request, const QString& format


QDomDocument doc; QDomDocument doc;
QString errorMsg; QString errorMsg;
QHash<QgsMapLayer*, QString> originalLayerFilters;
//scoped pointer to restore all original layer filters (subsetStrings) when pointer goes out of scope
//there's LOTS of potential exit paths here, so we avoid having to restore the filters manually
QScopedPointer< QgsOWSServerFilterRestorer > filterRestorer( new QgsOWSServerFilterRestorer() );

if ( doc.setContent( mParameters.value( "REQUEST_BODY" ), true, &errorMsg ) ) if ( doc.setContent( mParameters.value( "REQUEST_BODY" ), true, &errorMsg ) )
{ {
QDomElement docElem = doc.documentElement(); QDomElement docElem = doc.documentElement();
Expand Down Expand Up @@ -466,10 +470,9 @@ int QgsWFSServer::getFeature( QgsRequestHandler& request, const QString& format
#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
if ( !mAccessControl->layerReadPermission( currentLayer ) ) if ( !mAccessControl->layerReadPermission( currentLayer ) )
{ {
restoreLayerFilters( originalLayerFilters );
throw QgsMapServiceException( "Security", "Feature access permission denied" ); throw QgsMapServiceException( "Security", "Feature access permission denied" );
} }
applyAccessControlLayerFilters( currentLayer, originalLayerFilters ); applyAccessControlLayerFilters( currentLayer, filterRestorer->originalFilters() );
#endif #endif


expressionContext << QgsExpressionContextUtils::layerScope( layer ); expressionContext << QgsExpressionContextUtils::layerScope( layer );
Expand Down Expand Up @@ -646,9 +649,6 @@ int QgsWFSServer::getFeature( QgsRequestHandler& request, const QString& format
{ {
if ( filter->hasParserError() ) if ( filter->hasParserError() )
{ {
#ifdef HAVE_SERVER_PYTHON_PLUGINS
restoreLayerFilters( originalLayerFilters );
#endif
throw QgsMapServiceException( "RequestNotWellFormed", filter->parserErrorString() ); throw QgsMapServiceException( "RequestNotWellFormed", filter->parserErrorString() );
} }
while ( fit.nextFeature( feature ) && ( maxFeatures == -1 || featureCounter < maxFeat + startIndex ) ) while ( fit.nextFeature( feature ) && ( maxFeatures == -1 || featureCounter < maxFeat + startIndex ) )
Expand All @@ -658,10 +658,6 @@ int QgsWFSServer::getFeature( QgsRequestHandler& request, const QString& format
QVariant res = filter->evaluate( &expressionContext ); QVariant res = filter->evaluate( &expressionContext );
if ( filter->hasEvalError() ) if ( filter->hasEvalError() )
{ {

#ifdef HAVE_SERVER_PYTHON_PLUGINS
restoreLayerFilters( originalLayerFilters );
#endif
throw QgsMapServiceException( "RequestNotWellFormed", filter->evalErrorString() ); throw QgsMapServiceException( "RequestNotWellFormed", filter->evalErrorString() );
} }
if ( res.toInt() != 0 ) if ( res.toInt() != 0 )
Expand Down Expand Up @@ -703,9 +699,8 @@ int QgsWFSServer::getFeature( QgsRequestHandler& request, const QString& format


} }


#ifdef HAVE_SERVER_PYTHON_PLUGINS //force restoration of original layer filters
restoreLayerFilters( originalLayerFilters ); filterRestorer.reset();
#endif


QgsMessageLog::logMessage( mErrors.join( "\n" ) ); QgsMessageLog::logMessage( mErrors.join( "\n" ) );


Expand Down
52 changes: 28 additions & 24 deletions src/server/qgswmsserver.cpp
View file
Expand Up @@ -1239,7 +1239,6 @@ QDomDocument QgsWMSServer::describeLayer()
QByteArray* QgsWMSServer::getPrint( const QString& formatString ) QByteArray* QgsWMSServer::getPrint( const QString& formatString )
{ {
QStringList layersList, stylesList, layerIdList; QStringList layersList, stylesList, layerIdList;
QString dummyFormat;
QImage* theImage = initializeRendering( layersList, stylesList, layerIdList ); QImage* theImage = initializeRendering( layersList, stylesList, layerIdList );
if ( !theImage ) if ( !theImage )
{ {
Expand All @@ -1257,18 +1256,21 @@ QByteArray* QgsWMSServer::getPrint( const QString& formatString )
} }
#endif #endif


QHash<QgsMapLayer*, QString> originalLayerFilters = applyRequestedLayerFilters( layersList ); //scoped pointer to restore all original layer filters (subsetStrings) when pointer goes out of scope
//there's LOTS of potential exit paths here, so we avoid having to restore the filters manually
QScopedPointer< QgsOWSServerFilterRestorer > filterRestorer( new QgsOWSServerFilterRestorer() );

applyRequestedLayerFilters( layersList, filterRestorer->originalFilters() );


#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
applyAccessControlLayersFilters( layersList, originalLayerFilters ); applyAccessControlLayersFilters( layersList, filterRestorer->originalFilters() );
#endif #endif


QStringList selectedLayerIdList = applyFeatureSelections( layersList ); QStringList selectedLayerIdList = applyFeatureSelections( layersList );


//GetPrint request needs a template parameter //GetPrint request needs a template parameter
if ( !mParameters.contains( "TEMPLATE" ) ) if ( !mParameters.contains( "TEMPLATE" ) )
{ {
restoreLayerFilters( originalLayerFilters );
clearFeatureSelections( selectedLayerIdList ); clearFeatureSelections( selectedLayerIdList );
throw QgsMapServiceException( "ParameterMissing", "The TEMPLATE parameter is required for the GetPrint request" ); throw QgsMapServiceException( "ParameterMissing", "The TEMPLATE parameter is required for the GetPrint request" );
} }
Expand All @@ -1284,7 +1286,6 @@ QByteArray* QgsWMSServer::getPrint( const QString& formatString )
QgsComposition* c = mConfigParser->createPrintComposition( mParameters[ "TEMPLATE" ], mMapRenderer, QMap<QString, QString>( mParameters ) ); QgsComposition* c = mConfigParser->createPrintComposition( mParameters[ "TEMPLATE" ], mMapRenderer, QMap<QString, QString>( mParameters ) );
if ( !c ) if ( !c )
{ {
restoreLayerFilters( originalLayerFilters );
clearFeatureSelections( selectedLayerIdList ); clearFeatureSelections( selectedLayerIdList );
return nullptr; return nullptr;
} }
Expand Down Expand Up @@ -1333,7 +1334,6 @@ QByteArray* QgsWMSServer::getPrint( const QString& formatString )
if ( !tempFile.open() ) if ( !tempFile.open() )
{ {
delete c; delete c;
restoreLayerFilters( originalLayerFilters );
clearFeatureSelections( selectedLayerIdList ); clearFeatureSelections( selectedLayerIdList );
return nullptr; return nullptr;
} }
Expand All @@ -1344,13 +1344,11 @@ QByteArray* QgsWMSServer::getPrint( const QString& formatString )
} }
else //unknown format else //unknown format
{ {
restoreLayerFilters( originalLayerFilters );
clearFeatureSelections( selectedLayerIdList ); clearFeatureSelections( selectedLayerIdList );
throw QgsMapServiceException( "InvalidFormat", "Output format '" + formatString + "' is not supported in the GetPrint request" ); throw QgsMapServiceException( "InvalidFormat", "Output format '" + formatString + "' is not supported in the GetPrint request" );
} }


restoreOpacities( bkVectorRenderers, bkRasterRenderers, labelTransparencies, labelBufferTransparencies ); restoreOpacities( bkVectorRenderers, bkRasterRenderers, labelTransparencies, labelBufferTransparencies );
restoreLayerFilters( originalLayerFilters );
clearFeatureSelections( selectedLayerIdList ); clearFeatureSelections( selectedLayerIdList );


delete c; delete c;
Expand Down Expand Up @@ -1397,10 +1395,14 @@ QImage* QgsWMSServer::getMap( HitTest* hitTest )
} }
#endif #endif


QHash<QgsMapLayer*, QString> originalLayerFilters = applyRequestedLayerFilters( layersList ); //scoped pointer to restore all original layer filters (subsetStrings) when pointer goes out of scope
//there's LOTS of potential exit paths here, so we avoid having to restore the filters manually
QScopedPointer< QgsOWSServerFilterRestorer > filterRestorer( new QgsOWSServerFilterRestorer() );

applyRequestedLayerFilters( layersList, filterRestorer->originalFilters() );


#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
applyAccessControlLayersFilters( layersList, originalLayerFilters ); applyAccessControlLayersFilters( layersList, filterRestorer->originalFilters() );
#endif #endif


QStringList selectedLayerIdList = applyFeatureSelections( layersList ); QStringList selectedLayerIdList = applyFeatureSelections( layersList );
Expand All @@ -1426,7 +1428,6 @@ QImage* QgsWMSServer::getMap( HitTest* hitTest )
} }


restoreOpacities( bkVectorRenderers, bkRasterRenderers, labelTransparencies, labelBufferTransparencies ); restoreOpacities( bkVectorRenderers, bkRasterRenderers, labelTransparencies, labelBufferTransparencies );
restoreLayerFilters( originalLayerFilters );
clearFeatureSelections( selectedLayerIdList ); clearFeatureSelections( selectedLayerIdList );


// QgsMessageLog::logMessage( "clearing filters" ); // QgsMessageLog::logMessage( "clearing filters" );
Expand Down Expand Up @@ -1625,10 +1626,15 @@ int QgsWMSServer::getFeatureInfo( QDomDocument& result, const QString& version )
} }


//get the layer registered in QgsMapLayerRegistry and apply possible filters //get the layer registered in QgsMapLayerRegistry and apply possible filters
QStringList layerIds = layerSet( layersList, stylesList, mMapRenderer->destinationCrs() ); ( void )layerSet( layersList, stylesList, mMapRenderer->destinationCrs() );
QHash<QgsMapLayer*, QString> originalLayerFilters = applyRequestedLayerFilters( layersList );
//scoped pointer to restore all original layer filters (subsetStrings) when pointer goes out of scope
//there's LOTS of potential exit paths here, so we avoid having to restore the filters manually
QScopedPointer< QgsOWSServerFilterRestorer > filterRestorer( new QgsOWSServerFilterRestorer() );
applyRequestedLayerFilters( layersList, filterRestorer->originalFilters() );

#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
applyAccessControlLayersFilters( layersList, originalLayerFilters ); applyAccessControlLayersFilters( layersList, filterRestorer->originalFilters() );
#endif #endif


QDomElement getFeatureInfoElement; QDomElement getFeatureInfoElement;
Expand Down Expand Up @@ -1710,7 +1716,6 @@ int QgsWMSServer::getFeatureInfo( QDomDocument& result, const QString& version )
#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
if ( !mAccessControl->layerReadPermission( currentLayer ) ) if ( !mAccessControl->layerReadPermission( currentLayer ) )
{ {
restoreLayerFilters( originalLayerFilters );
throw QgsMapServiceException( "Security", "You are not allowed to access to the layer: " + currentLayer->name() ); throw QgsMapServiceException( "Security", "You are not allowed to access to the layer: " + currentLayer->name() );
} }
#endif #endif
Expand Down Expand Up @@ -1831,7 +1836,9 @@ int QgsWMSServer::getFeatureInfo( QDomDocument& result, const QString& version )
convertFeatureInfoToSIA2045( result ); convertFeatureInfoToSIA2045( result );
} }


restoreLayerFilters( originalLayerFilters ); //force restoration of original filters
filterRestorer.reset();

QgsMapLayerRegistry::instance()->removeAllMapLayers(); QgsMapLayerRegistry::instance()->removeAllMapLayers();
delete featuresRect; delete featuresRect;
return 0; return 0;
Expand Down Expand Up @@ -2495,13 +2502,11 @@ QStringList QgsWMSServer::layerSet( const QStringList &layersList,
} }




QHash<QgsMapLayer*, QString> QgsWMSServer::applyRequestedLayerFilters( const QStringList& layerList ) const void QgsWMSServer::applyRequestedLayerFilters( const QStringList& layerList , QHash<QgsMapLayer*, QString>& originalFilters ) const
{ {
QHash<QgsMapLayer*, QString> filterMap;

if ( layerList.isEmpty() ) if ( layerList.isEmpty() )
{ {
return filterMap; return;
} }


QString filterParameter = mParameters.value( "FILTER" ); QString filterParameter = mParameters.value( "FILTER" );
Expand Down Expand Up @@ -2548,7 +2553,7 @@ QHash<QgsMapLayer*, QString> QgsWMSServer::applyRequestedLayerFilters( const QSt
QgsVectorLayer* filteredLayer = dynamic_cast<QgsVectorLayer*>( filter ); QgsVectorLayer* filteredLayer = dynamic_cast<QgsVectorLayer*>( filter );
if ( filteredLayer ) if ( filteredLayer )
{ {
filterMap.insert( filteredLayer, filteredLayer->subsetString() ); originalFilters.insert( filteredLayer, filteredLayer->subsetString() );
QString newSubsetString = eqSplit.at( 1 ); QString newSubsetString = eqSplit.at( 1 );
if ( !filteredLayer->subsetString().isEmpty() ) if ( !filteredLayer->subsetString().isEmpty() )
{ {
Expand All @@ -2565,8 +2570,8 @@ QHash<QgsMapLayer*, QString> QgsWMSServer::applyRequestedLayerFilters( const QSt
if ( mMapRenderer && mMapRenderer->extent().isEmpty() ) if ( mMapRenderer && mMapRenderer->extent().isEmpty() )
{ {
QgsRectangle filterExtent; QgsRectangle filterExtent;
QHash<QgsMapLayer*, QString>::const_iterator filterIt = filterMap.constBegin(); QHash<QgsMapLayer*, QString>::const_iterator filterIt = originalFilters.constBegin();
for ( ; filterIt != filterMap.constEnd(); ++filterIt ) for ( ; filterIt != originalFilters.constEnd(); ++filterIt )
{ {
QgsMapLayer* mapLayer = filterIt.key(); QgsMapLayer* mapLayer = filterIt.key();
if ( !mapLayer ) if ( !mapLayer )
Expand All @@ -2587,7 +2592,6 @@ QHash<QgsMapLayer*, QString> QgsWMSServer::applyRequestedLayerFilters( const QSt
mMapRenderer->setExtent( filterExtent ); mMapRenderer->setExtent( filterExtent );
} }
} }
return filterMap;
} }


#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
Expand Down
11 changes: 9 additions & 2 deletions src/server/qgswmsserver.h
View file
Expand Up @@ -187,15 +187,22 @@ class QgsWMSServer: public QgsOWSServer
#endif #endif


/** Apply filter (subset) strings from the request to the layers. Example: '&FILTER=<layer1>:"AND property > 100",<layer2>:"AND bla = 'hallo!'" ' /** Apply filter (subset) strings from the request to the layers. Example: '&FILTER=<layer1>:"AND property > 100",<layer2>:"AND bla = 'hallo!'" '
@return a map with the original filters ( layer id / filter string )*/ * @param layerList list of layer IDs to filter
QHash<QgsMapLayer*, QString> applyRequestedLayerFilters( const QStringList& layerList ) const; * @param originalFilters hash of layer ID to original filter string
* @note It is strongly recommended that this method be called alongside use of QgsOWSServerFilterRestorer
* to ensure that the original filters are always correctly restored, regardless of whether exceptions
* are thrown or functions are terminated early.
*/
void applyRequestedLayerFilters( const QStringList& layerList, QHash<QgsMapLayer*, QString>& originalFilters ) const;

#ifdef HAVE_SERVER_PYTHON_PLUGINS #ifdef HAVE_SERVER_PYTHON_PLUGINS
/** Apply filter strings from the access control to the layers. /** Apply filter strings from the access control to the layers.
* @param layerList layers to filter * @param layerList layers to filter
* @param originalLayerFilters the original layers filter dictionary * @param originalLayerFilters the original layers filter dictionary
*/ */
void applyAccessControlLayersFilters( const QStringList& layerList, QHash<QgsMapLayer*, QString>& originalLayerFilters ) const; void applyAccessControlLayersFilters( const QStringList& layerList, QHash<QgsMapLayer*, QString>& originalLayerFilters ) const;
#endif #endif

/** Tests if a filter sql string is allowed (safe) /** Tests if a filter sql string is allowed (safe)
@return true in case of success, false if string seems unsafe*/ @return true in case of success, false if string seems unsafe*/
bool testFilterStringSafety( const QString& filter ) const; bool testFilterStringSafety( const QString& filter ) const;
Expand Down

0 comments on commit f264799

Please sign in to comment.