Skip to content
Permalink
Browse files
Fix #9290 (crashes in symbology on fields with special characters)
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 committed Feb 11, 2014
1 parent c13b6a4 commit e6c2ecd9f36e6b4924b08e185443e7eebbfb2467
@@ -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;

@@ -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;

@@ -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;

@@ -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 );
};
@@ -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 )
@@ -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() );

@@ -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 );

@@ -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;
@@ -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 );
@@ -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() );

@@ -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 );

@@ -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;
@@ -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 );
@@ -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;
@@ -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 );

@@ -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 );
@@ -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();
}
@@ -25,6 +25,7 @@
#include "qgssymbolv2.h"
#include "qgis.h"

class QgsExpression;
class QgsSymbolLayerV2;
class QgsVectorColorRampV2;

@@ -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;

0 comments on commit e6c2ecd

Please sign in to comment.