Fix #10365 (incorrectly escaped strings from QgsExpression)

wonder-sk · wonder-sk · commit 882912f748fa · 2014-06-09T14:31:42.000+07:00
diff --git a/src/core/qgsexpression.cpp b/src/core/qgsexpression.cpp
@@ -1739,6 +1739,20 @@ QList<QgsExpression::Function*> QgsExpression::specialColumns()
   return defs;
 }
 
+QString QgsExpression::quotedColumnRef( QString name )
+{
+  return QString( "\"%1\"" ).arg( name.replace( "\"", "\"\"" ) );
+}
+
+QString QgsExpression::quotedString( QString text )
+{
+  text.replace( "'", "''" );
+  text.replace( '\\', "\\\\" );
+  text.replace( '\n', "\\n" );
+  text.replace( '\t', "\\t" );
+  return QString( "'%1'" ).arg( text );
+}
+
 bool QgsExpression::isFunctionName( QString name )
 {
   return functionIndex( name ) != -1;
@@ -2429,7 +2443,7 @@ QString QgsExpression::NodeLiteral::dump() const
   {
     case QVariant::Int: return QString::number( mValue.toInt() );
     case QVariant::Double: return QString::number( mValue.toDouble() );
-    case QVariant::String: return QString( "'%1'" ).arg( mValue.toString() );
+    case QVariant::String: return quotedString( mValue.toString() );
     default: return QObject::tr( "[unsupported type;%1; value:%2]" ).arg( mValue.typeName() ).arg( mValue.toString() );
   }
 }
diff --git a/src/core/qgsexpression.h b/src/core/qgsexpression.h
@@ -314,9 +314,9 @@ class CORE_EXPORT QgsExpression
     static QList<Function*> specialColumns();
 
     //! return quoted column reference (in double quotes)
-    static QString quotedColumnRef( QString name ) { return QString( "\"%1\"" ).arg( name.replace( "\"", "\"\"" ) ); }
+    static QString quotedColumnRef( QString name );
     //! return quoted string (in single quotes)
-    static QString quotedString( QString text ) { return QString( "'%1'" ).arg( text.replace( "'", "''" ) ); }
+    static QString quotedString( QString text );
 
     //////
 
diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -882,18 +882,31 @@ class TestQgsExpression: public QObject
       QgsExpression::unsetSpecialColumn( "$var1" );
     }
 
+    void expression_from_expression_data()
+    {
+      QTest::addColumn<QString>( "string" );
+      QTest::newRow( "column ref" ) << "my_column";
+      QTest::newRow( "column ref with space" ) << "\"my column\"";
+      QTest::newRow( "string literal" ) << "'hello'";
+      QTest::newRow( "string with quote" ) << "'hel''lo'";
+    }
+
     void expression_from_expression()
     {
-      {
-        QgsExpression e( "my_column" );
-        QCOMPARE( e.expression() , QgsExpression( e.expression() ).expression() );
-      }
-      {
-        QgsExpression e( "\"my column\"" );
-        QCOMPARE( e.expression() , QgsExpression( e.expression() ).expression() );
-      }
+      QFETCH( QString, string );
+
+      QgsExpression e( string );
+      QVERIFY( !e.hasParserError() );
+      qDebug() << e.expression();
+      QCOMPARE( e.expression() , QgsExpression( e.expression() ).expression() );
     }
 
+    void quote_string()
+    {
+      QCOMPARE( QgsExpression::quotedString( "hello\nworld" ), QString( "'hello\\nworld'" ) );
+      QCOMPARE( QgsExpression::quotedString( "hello\tworld" ), QString( "'hello\\tworld'" ) );
+      QCOMPARE( QgsExpression::quotedString( "hello\\world" ), QString( "'hello\\\\world'" ) );
+    }
 
 };
 

Original file line number	Diff line number	Diff line change
`@@ -1739,6 +1739,20 @@ QList<QgsExpression::Function*> QgsExpression::specialColumns()`
`1739`	`1739`	`return defs;`
`1740`	`1740`	`}`
`1741`	`1741`
	`1742`	`+QString QgsExpression::quotedColumnRef( QString name )`
	`1743`	`+{`
	`1744`	`+ return QString( "\"%1\"" ).arg( name.replace( "\"", "\"\"" ) );`
	`1745`	`+}`
	`1746`	`+`
	`1747`	`+QString QgsExpression::quotedString( QString text )`
	`1748`	`+{`
	`1749`	`+ text.replace( "'", "''" );`
	`1750`	`+ text.replace( '\\', "\\\\" );`
	`1751`	`+ text.replace( '\n', "\\n" );`
	`1752`	`+ text.replace( '\t', "\\t" );`
	`1753`	`+ return QString( "'%1'" ).arg( text );`
	`1754`	`+}`
	`1755`	`+`
`1742`	`1756`	`bool QgsExpression::isFunctionName( QString name )`
`1743`	`1757`	`{`
`1744`	`1758`	`return functionIndex( name ) != -1;`
`@@ -2429,7 +2443,7 @@ QString QgsExpression::NodeLiteral::dump() const`
`2429`	`2443`	`{`
`2430`	`2444`	`case QVariant::Int: return QString::number( mValue.toInt() );`
`2431`	`2445`	`case QVariant::Double: return QString::number( mValue.toDouble() );`
`2432`		`- case QVariant::String: return QString( "'%1'" ).arg( mValue.toString() );`
	`2446`	`+ case QVariant::String: return quotedString( mValue.toString() );`
`2433`	`2447`	`default: return QObject::tr( "[unsupported type;%1; value:%2]" ).arg( mValue.typeName() ).arg( mValue.toString() );`
`2434`	`2448`	`}`
`2435`	`2449`	`}`