From fbda356a749b3ad4eacd04a74809a1e257928e46 Mon Sep 17 00:00:00 2001 From: Stefanos Natsis Date: Tue, 27 Apr 2021 07:05:23 +0300 Subject: [PATCH] Order map keys by length before using in replace function Fix #42940 --- resources/function_help/json/replace | 5 +++-- src/core/expression/qgsexpressionfunction.cpp | 17 ++++++++++++++++- tests/src/core/testqgsexpression.cpp | 1 + 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/resources/function_help/json/replace b/resources/function_help/json/replace index 2b824fe0974..655ee3f973d 100644 --- a/resources/function_help/json/replace +++ b/resources/function_help/json/replace @@ -13,9 +13,10 @@ { "expression":"replace('QGIS ABC',array('A','B','C'),array('X','Y','Z'))", "returns":"'QGIS XYZ'"}, { "expression":"replace('QGIS',array('Q','S'),'')", "returns":"'GI'"} ] }, { "variant": "Map variant", - "variant_description": "Returns a string with the supplied map keys replaced by paired values.", + "variant_description": "Returns a string with the supplied map keys replaced by paired values. Longer map keys are evaluated first.", "arguments": [ {"arg":"string","description":"the input string"}, {"arg":"map","description":"the map containing keys and values"} ], - "examples": [ { "expression":"replace('APP SHOULD ROCK',map('APP','QGIS','SHOULD','DOES'))", "returns":"'QGIS DOES ROCK'"} ] + "examples": [ { "expression":"replace('APP SHOULD ROCK',map('APP','QGIS','SHOULD','DOES'))", "returns":"'QGIS DOES ROCK'"}, + { "expression":"replace('forty two',map('for','4','two','2','forty two','42'))", "returns":"'42'"} ] }] } diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp index c6b580dbc0e..ed7ba5a163b 100644 --- a/src/core/expression/qgsexpressionfunction.cpp +++ b/src/core/expression/qgsexpressionfunction.cpp @@ -1367,10 +1367,25 @@ static QVariant fcnReplace( const QVariantList &values, const QgsExpressionConte { QString str = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); QVariantMap map = QgsExpressionUtils::getMapValue( values.at( 1 ), parent ); + QVector< QPair< QString, QString > > mapItems; for ( QVariantMap::const_iterator it = map.constBegin(); it != map.constEnd(); ++it ) { - str = str.replace( it.key(), it.value().toString() ); + mapItems.append( qMakePair( it.key(), it.value().toString() ) ); + } + + // larger keys should be replaced first since they may contain whole smaller keys + std::sort( mapItems.begin(), + mapItems.end(), + []( const QPair< QString, QString > &pair1, + const QPair< QString, QString > &pair2 ) + { + return ( pair1.first.length() > pair2.first.length() ); + } ); + + for ( auto it = mapItems.constBegin(); it != mapItems.constEnd(); ++it ) + { + str = str.replace( it->first, it->second ); } return QVariant( str ); diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp index 1020759f138..266c91f2fdd 100644 --- a/tests/src/core/testqgsexpression.cpp +++ b/tests/src/core/testqgsexpression.cpp @@ -1312,6 +1312,7 @@ class TestQgsExpression: public QObject QTest::newRow( "replace (unbalanced array, before > after)" ) << "replace('12345', array('1','2','3'), array('6','7'))" << true << QVariant(); QTest::newRow( "replace (unbalanced array, before < after)" ) << "replace('12345', array('1','2'), array('6','7','8'))" << true << QVariant(); QTest::newRow( "replace (map)" ) << "replace('APP SHOULD ROCK',map('APP','QGIS','SHOULD','DOES'))" << false << QVariant( "QGIS DOES ROCK" ); + QTest::newRow( "replace (map with overlapping keys)" ) << "replace('11111',map('1','small','11','large'))" << false << QVariant( "largelargesmall" ); QTest::newRow( "regexp_replace" ) << "regexp_replace('HeLLo','[eL]+', '-')" << false << QVariant( "H-o" ); QTest::newRow( "regexp_replace greedy" ) << "regexp_replace('HeLLo','(?<=H).*L', '-')" << false << QVariant( "H-o" ); QTest::newRow( "regexp_replace non greedy" ) << "regexp_replace('HeLLo','(?<=H).*?L', '-')" << false << QVariant( "H-Lo" );