Skip to content

Commit

Permalink
Fix #9290 (crashes in symbology on fields with special characters)
Browse files Browse the repository at this point in the history 
Fixes several crashes and problems with backwards compatibility.
Before expressions were allowed, field names were loaded and saved
unquoted, e.g. x.y  - with introduction of expressions it was
necessary to use properly quoted "x.y" name so that expression
is correctly parsed. With these changes both options are possible
and when possible, the unquoted field name is used for compatibility
  • Loading branch information
@wonder-sk
wonder-sk committed Feb 11, 2014
1 parent c13b6a4 commit e6c2ecd
Show file tree
Hide file tree
Showing 12 changed files with 159 additions and 53 deletions.
4 changes: 2 additions & 2 deletions python/core/symbology-ng/qgscategorizedsymbolrendererv2.sip
View file
Expand Up @@ -110,12 +110,12 @@ class QgsCategorizedSymbolRendererV2 : QgsFeatureRendererV2
void setInvertedColorRamp( bool inverted );

//! @note added in 1.6
void setRotationField( QString expression );
void setRotationField( QString fieldOrExpression );
//! @note added in 1.6
QString rotationField() const;

//! @note added in 1.6
void setSizeScaleField( QString expression );
void setSizeScaleField( QString fieldOrExpression );
//! @note added in 1.6
QString sizeScaleField() const;

Expand Down
4 changes: 2 additions & 2 deletions python/core/symbology-ng/qgsgraduatedsymbolrendererv2.sip
View file
Expand Up @@ -136,12 +136,12 @@ class QgsGraduatedSymbolRendererV2 : QgsFeatureRendererV2
void updateSymbols( QgsSymbolV2* sym /Transfer/ );

//! @note added in 1.6
void setRotationField( QString expression );
void setRotationField( QString fieldOrExpression );
//! @note added in 1.6
QString rotationField() const;

//! @note added in 1.6
void setSizeScaleField( QString expression );
void setSizeScaleField( QString fieldOrExpression );
//! @note added in 1.6
QString sizeScaleField() const;

Expand Down
4 changes: 2 additions & 2 deletions python/core/symbology-ng/qgssinglesymbolrendererv2.sip
View file
Expand Up @@ -21,12 +21,12 @@ class QgsSingleSymbolRendererV2 : QgsFeatureRendererV2
void setSymbol( QgsSymbolV2* s /Transfer/ );

//! @note added in 1.5
void setRotationField( QString expression );
void setRotationField( QString fieldOrExpression );
//! @note added in 1.5
QString rotationField() const;

//! @note added in 1.5
void setSizeScaleField( QString expression );
void setSizeScaleField( QString fieldOrExpression );
//! @note added in 1.5
QString sizeScaleField() const;

Expand Down
16 changes: 16 additions & 0 deletions python/core/symbology-ng/qgssymbollayerv2utils.sip
View file
Expand Up @@ -226,4 +226,20 @@ class QgsSymbolLayerV2Utils

//! Calculate the centroid point of a QPolygonF
static QPointF polygonCentroid( const QPolygonF& points );

/** Return a new valid expression instance for given field or expression string.
* If the input is not a valid expression, it is assumed that it is a field name and gets properly quoted.
* If the string is empty, returns null pointer.
* This is useful when accepting input which could be either a non-quoted field name or expression.
* @note added in 2.2
*/
static QgsExpression* fieldOrExpressionToExpression( const QString& fieldOrExpression ) /Factory/;

/** Return a field name if the whole expression is just a name of the field .
* Returns full expression string if the expression is more complex than just one field.
* Using just expression->expression() method may return quoted field name, but that is not
* wanted for saving (due to backward compatibility) or display in GUI.
* @note added in 2.2
*/
static QString fieldOrExpressionFromExpression( QgsExpression* expression );
};
36 changes: 31 additions & 5 deletions src/core/symbology-ng/qgscategorizedsymbolrendererv2.cpp
View file
Expand Up @@ -211,7 +211,7 @@ QgsSymbolV2* QgsCategorizedSymbolRendererV2::symbolForFeature( QgsFeature& featu
const double sizeScale = mSizeScale.data() ? mSizeScale->evaluate( feature ).toDouble() : 1.;

// take a temporary symbol (or create it if doesn't exist)
QgsSymbolV2* tempSymbol = mTempSymbols[attrs[mAttrNum].toString()];
QgsSymbolV2* tempSymbol = mTempSymbols[value.toString()];

// modify the temporary symbol and return it
if ( tempSymbol->type() == QgsSymbolV2::Marker )
Expand Down Expand Up @@ -388,8 +388,14 @@ void QgsCategorizedSymbolRendererV2::stopRender( QgsRenderContext& context )

QList<QString> QgsCategorizedSymbolRendererV2::usedAttributes()
{
QgsExpression exp( mAttrName );
QSet<QString> attributes( exp.referencedColumns().toSet() );
QSet<QString> attributes;

if ( QgsExpression* exp = QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( mAttrName ) )
{
attributes.unite( exp->referencedColumns().toSet() );
delete exp;
}

if ( mRotation.data() ) attributes.unite( mRotation->referencedColumns().toSet() );
if ( mSizeScale.data() ) attributes.unite( mSizeScale->referencedColumns().toSet() );

Expand Down Expand Up @@ -582,12 +588,12 @@ QDomElement QgsCategorizedSymbolRendererV2::save( QDomDocument& doc )

QDomElement rotationElem = doc.createElement( "rotation" );
if ( mRotation.data() )
rotationElem.setAttribute( "field", mRotation->expression() );
rotationElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) );
rendererElem.appendChild( rotationElem );

QDomElement sizeScaleElem = doc.createElement( "sizescale" );
if ( mSizeScale.data() )
sizeScaleElem.setAttribute( "field", mSizeScale->expression() );
sizeScaleElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) );
sizeScaleElem.setAttribute( "scalemethod", QgsSymbolLayerV2Utils::encodeScaleMethod( mScaleMethod ) );
rendererElem.appendChild( sizeScaleElem );

Expand Down Expand Up @@ -656,6 +662,26 @@ void QgsCategorizedSymbolRendererV2::setSourceColorRamp( QgsVectorColorRampV2* r
mSourceColorRamp.reset( ramp );
}

void QgsCategorizedSymbolRendererV2::setRotationField( QString fieldOrExpression )
{
mRotation.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
}

QString QgsCategorizedSymbolRendererV2::rotationField() const
{
return mRotation.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) : QString();
}

void QgsCategorizedSymbolRendererV2::setSizeScaleField( QString fieldOrExpression )
{
mSizeScale.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
}

QString QgsCategorizedSymbolRendererV2::sizeScaleField() const
{
return mSizeScale.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) : QString();
}

void QgsCategorizedSymbolRendererV2::updateSymbols( QgsSymbolV2 * sym )
{
int i = 0;
Expand Down
16 changes: 4 additions & 12 deletions src/core/symbology-ng/qgscategorizedsymbolrendererv2.h
View file
Expand Up @@ -139,22 +139,14 @@ class CORE_EXPORT QgsCategorizedSymbolRendererV2 : public QgsFeatureRendererV2
void setInvertedColorRamp( bool inverted ) { mInvertedColorRamp = inverted; }

//! @note added in 1.6
void setRotationField( QString expression )
{
mRotation.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
Q_ASSERT( !mRotation.data() || !mRotation->hasParserError() );
}
void setRotationField( QString fieldOrExpression );
//! @note added in 1.6
QString rotationField() const { return mRotation.data() ? mRotation->expression() : QString();}
QString rotationField() const;

//! @note added in 1.6
void setSizeScaleField( QString expression )
{
mSizeScale.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
Q_ASSERT( !mSizeScale.data() || !mSizeScale->hasParserError() );
}
void setSizeScaleField( QString fieldOrExpression );
//! @note added in 1.6
QString sizeScaleField() const { return mSizeScale.data() ? mSizeScale->expression() : QString(); }
QString sizeScaleField() const;

//! @note added in 2.0
void setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod );
Expand Down
34 changes: 30 additions & 4 deletions src/core/symbology-ng/qgsgraduatedsymbolrendererv2.cpp
View file
Expand Up @@ -263,8 +263,14 @@ void QgsGraduatedSymbolRendererV2::stopRender( QgsRenderContext& context )

QList<QString> QgsGraduatedSymbolRendererV2::usedAttributes()
{
QgsExpression exp( mAttrName );
QSet<QString> attributes( exp.referencedColumns().toSet() );
QSet<QString> attributes;

if ( QgsExpression* exp = QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( mAttrName ) )
{
attributes.unite( exp->referencedColumns().toSet() );
delete exp;
}

if ( mRotation.data() ) attributes.unite( mRotation->referencedColumns().toSet() );
if ( mSizeScale.data() ) attributes.unite( mSizeScale->referencedColumns().toSet() );

Expand Down Expand Up @@ -1041,12 +1047,12 @@ QDomElement QgsGraduatedSymbolRendererV2::save( QDomDocument& doc )

QDomElement rotationElem = doc.createElement( "rotation" );
if ( mRotation.data() )
rotationElem.setAttribute( "field", mRotation->expression() );
rotationElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) );
rendererElem.appendChild( rotationElem );

QDomElement sizeScaleElem = doc.createElement( "sizescale" );
if ( mSizeScale.data() )
sizeScaleElem.setAttribute( "field", mSizeScale->expression() );
sizeScaleElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) );
sizeScaleElem.setAttribute( "scalemethod", QgsSymbolLayerV2Utils::encodeScaleMethod( mScaleMethod ) );
rendererElem.appendChild( sizeScaleElem );

Expand Down Expand Up @@ -1154,6 +1160,26 @@ void QgsGraduatedSymbolRendererV2::updateSymbols( QgsSymbolV2 *sym )
this->setSourceSymbol( sym->clone() );
}

void QgsGraduatedSymbolRendererV2::setRotationField( QString fieldOrExpression )
{
mRotation.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
}

QString QgsGraduatedSymbolRendererV2::rotationField() const
{
return mRotation.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) : QString();
}

void QgsGraduatedSymbolRendererV2::setSizeScaleField( QString fieldOrExpression )
{
mSizeScale.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
}

QString QgsGraduatedSymbolRendererV2::sizeScaleField() const
{
return mSizeScale.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) : QString();
}

void QgsGraduatedSymbolRendererV2::setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod )
{
mScaleMethod = scaleMethod;
Expand Down
16 changes: 4 additions & 12 deletions src/core/symbology-ng/qgsgraduatedsymbolrendererv2.h
View file
Expand Up @@ -165,22 +165,14 @@ class CORE_EXPORT QgsGraduatedSymbolRendererV2 : public QgsFeatureRendererV2
void updateSymbols( QgsSymbolV2* sym );

//! @note added in 1.6
void setRotationField( QString expression )
{
mRotation.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
Q_ASSERT( !mRotation.data() || !mRotation->hasParserError() );
}
void setRotationField( QString fieldOrExpression );
//! @note added in 1.6
QString rotationField() const { return mRotation.data() ? mRotation->expression() : QString();}
QString rotationField() const;

//! @note added in 1.6
void setSizeScaleField( QString expression )
{
mSizeScale.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
Q_ASSERT( !mSizeScale.data() || !mSizeScale->hasParserError() );
}
void setSizeScaleField( QString fieldOrExpression );
//! @note added in 1.6
QString sizeScaleField() const { return mSizeScale.data() ? mSizeScale->expression() : QString(); }
QString sizeScaleField() const;

//! @note added in 2.0
void setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod );
Expand Down
24 changes: 22 additions & 2 deletions src/core/symbology-ng/qgssinglesymbolrendererv2.cpp
View file
Expand Up @@ -136,6 +136,26 @@ void QgsSingleSymbolRendererV2::setSymbol( QgsSymbolV2* s )
mSymbol.reset( s );
}

void QgsSingleSymbolRendererV2::setRotationField( QString fieldOrExpression )
{
mRotation.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
}

QString QgsSingleSymbolRendererV2::rotationField() const
{
return mRotation.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) : QString();
}

void QgsSingleSymbolRendererV2::setSizeScaleField( QString fieldOrExpression )
{
mSizeScale.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
}

QString QgsSingleSymbolRendererV2::sizeScaleField() const
{
return mSizeScale.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) : QString();
}

void QgsSingleSymbolRendererV2::setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod )
{
mScaleMethod = scaleMethod;
Expand Down Expand Up @@ -315,12 +335,12 @@ QDomElement QgsSingleSymbolRendererV2::save( QDomDocument& doc )

QDomElement rotationElem = doc.createElement( "rotation" );
if ( mRotation.data() )
rotationElem.setAttribute( "field", mRotation->expression() );
rotationElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) );
rendererElem.appendChild( rotationElem );

QDomElement sizeScaleElem = doc.createElement( "sizescale" );
if ( mSizeScale.data() )
sizeScaleElem.setAttribute( "field", mSizeScale->expression() );
sizeScaleElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) );
sizeScaleElem.setAttribute( "scalemethod", QgsSymbolLayerV2Utils::encodeScaleMethod( mScaleMethod ) );
rendererElem.appendChild( sizeScaleElem );

Expand Down
16 changes: 4 additions & 12 deletions src/core/symbology-ng/qgssinglesymbolrendererv2.h
View file
Expand Up @@ -41,22 +41,14 @@ class CORE_EXPORT QgsSingleSymbolRendererV2 : public QgsFeatureRendererV2
void setSymbol( QgsSymbolV2* s );

//! @note added in 1.5
void setRotationField( QString expression )
{
mRotation.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
Q_ASSERT( !mRotation.data() || !mRotation->hasParserError() );
}
void setRotationField( QString fieldOrExpression );
//! @note added in 1.5
QString rotationField() const { return mRotation.data() ? mRotation->expression() : QString(); }
QString rotationField() const;

//! @note added in 1.5
void setSizeScaleField( QString expression )
{
mSizeScale.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
Q_ASSERT( !mSizeScale.data() || !mSizeScale->hasParserError() );
}
void setSizeScaleField( QString fieldOrExpression );
//! @note added in 1.5
QString sizeScaleField() const { return mSizeScale.data() ? mSizeScale->expression() : QString(); }
QString sizeScaleField() const;

//! @note added in 2.0
void setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod );
Expand Down
25 changes: 25 additions & 0 deletions src/core/symbology-ng/qgssymbollayerv2utils.cpp
View file
Expand Up @@ -2972,3 +2972,28 @@ QPointF QgsSymbolLayerV2Utils::polygonCentroid( const QPolygonF& points )
}


QgsExpression* QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( const QString& fieldOrExpression )
{
if ( fieldOrExpression.isEmpty() )
return 0;

QgsExpression* expr = new QgsExpression( fieldOrExpression );
if ( !expr->hasParserError() )
return expr;

// now try with quoted field name
delete expr;
QgsExpression* expr2 = new QgsExpression( QgsExpression::quotedColumnRef( fieldOrExpression ) );
Q_ASSERT( !expr2->hasParserError() );
return expr2;
}

QString QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( QgsExpression* expression )
{
const QgsExpression::Node* n = expression->rootNode();

if ( n && n->nodeType() == QgsExpression::ntColumnRef )
return static_cast<const QgsExpression::NodeColumnRef*>( n )->name();

return expression->expression();
}
17 changes: 17 additions & 0 deletions src/core/symbology-ng/qgssymbollayerv2utils.h
View file
Expand Up @@ -25,6 +25,7 @@
#include "qgssymbolv2.h"
#include "qgis.h"

class QgsExpression;
class QgsSymbolLayerV2;
class QgsVectorColorRampV2;

Expand Down Expand Up @@ -260,6 +261,22 @@ class CORE_EXPORT QgsSymbolLayerV2Utils

//! Calculate the centroid point of a QPolygonF
static QPointF polygonCentroid( const QPolygonF& points );

/** Return a new valid expression instance for given field or expression string.
* If the input is not a valid expression, it is assumed that it is a field name and gets properly quoted.
* If the string is empty, returns null pointer.
* This is useful when accepting input which could be either a non-quoted field name or expression.
* @note added in 2.2
*/
static QgsExpression* fieldOrExpressionToExpression( const QString& fieldOrExpression );

/** Return a field name if the whole expression is just a name of the field .
* Returns full expression string if the expression is more complex than just one field.
* Using just expression->expression() method may return quoted field name, but that is not
* wanted for saving (due to backward compatibility) or display in GUI.
* @note added in 2.2
*/
static QString fieldOrExpressionFromExpression( QgsExpression* expression );
};

class QPolygonF;
Expand Down

0 comments on commit e6c2ecd

Please sign in to comment.