Don't lost valid characters unsed in sheetName, such as space

dbzhang800 · Mar 22, 2015 · 47fda01 · 47fda01
1 parent 5507e67
commit 47fda01
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 22 deletions.
diff --git a/src/xlsx/xlsxchart.cpp b/src/xlsx/xlsxchart.cpp
@@ -104,8 +104,8 @@ void Chart::addSeries(const CellRange &range, AbstractSheet *sheet)
         return;
 
     QString sheetName = sheet ? sheet->sheetName() : d->sheet->sheetName();
-    if (sheetName.contains(QLatin1Char(' ')))
-        sheetName = QLatin1Char('\'') + sheetName + QLatin1Char('\'');
+    //In case sheetName contains space or '
+    sheetName = escapeSheetName(sheetName);
 
     if (range.columnCount() == 1 || range.rowCount() == 1) {
         QSharedPointer<XlsxSeries> series = QSharedPointer<XlsxSeries>(new XlsxSeries);

diff --git a/src/xlsx/xlsxutility.cpp b/src/xlsx/xlsxutility.cpp
@@ -127,16 +127,51 @@ QString createSafeSheetName(const QString &nameProposal)
         return QString();
 
     QString ret = nameProposal;
-    if (nameProposal.contains(QRegularExpression(QStringLiteral("[/\\\\?*\\][:]+"))))
-        ret.replace(QRegularExpression(QStringLiteral("[/\\\\?*\\][:]+")), QStringLiteral(" "));
-    while(ret.contains(QRegularExpression(QStringLiteral("^\\s*'\\s*|\\s*'\\s*$"))))
-        ret.remove(QRegularExpression(QStringLiteral("^\\s*'\\s*|\\s*'\\s*$")));
-    ret = ret.trimmed();
+    if (nameProposal.length() > 2 && nameProposal.startsWith(QLatin1Char('\'')) && nameProposal.endsWith(QLatin1Char('\'')))
+        ret = unescapeSheetName(ret);
+
+    //Replace invalid chars with space.
+    if (nameProposal.contains(QRegularExpression(QStringLiteral("[/\\\\?*\\][:]"))))
+        ret.replace(QRegularExpression(QStringLiteral("[/\\\\?*\\][:]")), QStringLiteral(" "));
+    if (ret.startsWith(QLatin1Char('\'')))
+        ret[0] = QLatin1Char(' ');
+    if (ret.endsWith(QLatin1Char('\'')))
+        ret[ret.size()-1] = QLatin1Char(' ');
+
     if (ret.size() > 31)
         ret = ret.left(31);
     return ret;
 }
 
+/*
+ * When sheetName contains space or apostrophe, escaped is needed by cellFormula/definedName/chartSerials.
+ */
+QString escapeSheetName(const QString &sheetName)
+{
+    //Already escaped.
+    Q_ASSERT(!sheetName.startsWith(QLatin1Char('\'')) && !sheetName.endsWith(QLatin1Char('\'')));
+
+    //These is no need to escape
+    if (!sheetName.contains(QRegularExpression(QStringLiteral("[ +\\-,%^=<>'&]"))))
+        return sheetName;
+
+    //OK, escape is needed.
+    QString name = sheetName;
+    name.replace(QLatin1Char('\''), QLatin1String("\'\'"));
+    return QLatin1Char('\'') + name + QLatin1Char('\'');
+}
+
+/*
+ */
+QString unescapeSheetName(const QString &sheetName)
+{
+    Q_ASSERT(sheetName.length() > 2 && sheetName.startsWith(QLatin1Char('\'')) && sheetName.endsWith(QLatin1Char('\'')));
+
+    QString name = sheetName.mid(1, sheetName.length()-2);
+    name.replace(QLatin1String("\'\'"), QLatin1String("\'"));
+    return name;
+}
+
 /*
  * whether the string s starts or ends with space
  */

diff --git a/src/xlsx/xlsxutility_p.h b/src/xlsx/xlsxutility_p.h
@@ -57,6 +57,9 @@ XLSX_AUTOTEST_EXPORT QDateTime datetimeFromNumber(double num, bool is1904=false)
 XLSX_AUTOTEST_EXPORT double timeToNumber(const QTime &t);
 
 XLSX_AUTOTEST_EXPORT QString createSafeSheetName(const QString &nameProposal);
+XLSX_AUTOTEST_EXPORT QString escapeSheetName(const QString &sheetName);
+XLSX_AUTOTEST_EXPORT QString unescapeSheetName(const QString &sheetName);
+
 XLSX_AUTOTEST_EXPORT bool isSpaceReserveNeeded(const QString &string);
 
 XLSX_AUTOTEST_EXPORT QString convertSharedFormula(const QString &rootFormula, const CellReference &rootCell, const CellReference &cell);

diff --git a/src/xlsx/xlsxworkbook.cpp b/src/xlsx/xlsxworkbook.cpp
@@ -285,12 +285,12 @@ bool Workbook::setActiveSheet(int index)
 }
 
 /*!
- * Rename the worksheet at the \a index to \a name.
+ * Rename the worksheet at the \a index to \a newName.
  */
-bool Workbook::renameSheet(int index, const QString &name)
+bool Workbook::renameSheet(int index, const QString &newName)
 {
     Q_D(Workbook);
-
+    QString name = createSafeSheetName(newName);
     if (index < 0 || index >= d->sheets.size())
         return false;
 
@@ -350,7 +350,7 @@ bool Workbook::copySheet(int index, const QString &newName)
     if (index < 0 || index >= d->sheets.size())
         return false;
 
-    QString worksheetName = newName;
+    QString worksheetName = createSafeSheetName(newName);
     if (!newName.isEmpty()) {
         //If user given an already in-used name, we should not continue any more!
         if (d->sheetNames.contains(newName))

diff --git a/tests/auto/utility/tst_utilitytest.cpp b/tests/auto/utility/tst_utilitytest.cpp
@@ -48,6 +48,9 @@ private Q_SLOTS:
     void test_createSafeSheetName_data();
     void test_createSafeSheetName();
 
+    void test_escapeSheetName_data();
+    void test_escapeSheetName();
+
     void test_convertSharedFormula_data();
     void test_convertSharedFormula();
 };
@@ -131,17 +134,19 @@ void UtilityTest::test_createSafeSheetName_data()
     QTest::addColumn<QString>("original");
     QTest::addColumn<QString>("result");
 
-    QTest::newRow("[Hello]") << QString("[Hello]")<<QString("Hello");
-    QTest::newRow("[Hello:Qt]") << QString("[Hello:Qt]")<<QString("Hello Qt");
-    QTest::newRow("[Hello\\Qt/Xlsx]") << QString("[Hello\\Qt/Xlsx]")<<QString("Hello Qt Xlsx");
-    QTest::newRow("[Hello\\Qt/Xlsx:Lib]") << QString("[Hello\\Qt/Xlsx:Lib]")<<QString("Hello Qt Xlsx Lib");
-    QTest::newRow("'He'llo'") << QString("'He'llo'")<<QString("He'llo");
-    QTest::newRow("'He'llo*Qt'") << QString("'He'llo*Qt'")<<QString("He'llo Qt");
-    QTest::newRow("'He'llo*Qt?Lib'") << QString("'He'llo*Qt?Lib'")<<QString("He'llo Qt Lib");
-
-    QTest::newRow(":'[Hello']") << QString(":'[Hello']")<<QString("Hello");
-    QTest::newRow("':'[Hello']") << QString("':'[Hello']")<<QString("Hello");
-    QTest::newRow("  ' : '[ Hello ' ]  ") << QString("  ' : '[ Hello ' ]  ")<<QString("Hello");
+    QTest::newRow("Hello:Qt") << QString("Hello:Qt")<<QString("Hello Qt");
+    QTest::newRow(" Hello Qt ") << QString(" Hello Qt ")<<QString(" Hello Qt ");
+    QTest::newRow("[Hello]") << QString("[Hello]")<<QString(" Hello ");
+    QTest::newRow("[Hello:Qt]") << QString("[Hello:Qt]")<<QString(" Hello Qt ");
+    QTest::newRow("[Hello\\Qt/Xlsx]") << QString("[Hello\\Qt/Xlsx]")<<QString(" Hello Qt Xlsx ");
+    QTest::newRow("[Hello\\Qt/Xlsx:Lib]") << QString("[Hello\\Qt/Xlsx:Lib]")<<QString(" Hello Qt Xlsx Lib ");
+    QTest::newRow("He'llo") << QString("He'llo")<<QString("He'llo");
+    QTest::newRow("'He''llo'") << QString("'He''llo'")<<QString("He'llo");
+
+    QTest::newRow("'Hello*Qt'") << QString("'Hello*Qt'")<<QString("Hello Qt");
+    QTest::newRow("'He''llo*Qt?Lib'") << QString("'He''llo*Qt?Lib'")<<QString("He'llo Qt Lib");
+
+    QTest::newRow(":'[Hello']") << QString(":'[Hello']")<<QString(" ' Hello' ");
 }
 
 void UtilityTest::test_createSafeSheetName()
@@ -152,6 +157,34 @@ void UtilityTest::test_createSafeSheetName()
     QCOMPARE(QXlsx::createSafeSheetName(original), result);
 }
 
+void UtilityTest::test_escapeSheetName_data()
+{
+    QTest::addColumn<QString>("original");
+    QTest::addColumn<QString>("result");
+
+    QTest::newRow("HelloQt") << QString("HelloQt")<<QString("HelloQt");
+    QTest::newRow("HelloQt ") << QString("HelloQt ")<<QString("'HelloQt '");
+    QTest::newRow("Hello Qt") << QString("Hello Qt")<<QString("'Hello Qt'");
+    QTest::newRow("Hello+Qt") << QString("Hello+Qt")<<QString("'Hello+Qt'");
+    QTest::newRow("Hello-Qt") << QString("Hello-Qt")<<QString("'Hello-Qt'");
+    QTest::newRow("Hello^Qt") << QString("Hello^Qt")<<QString("'Hello^Qt'");
+    QTest::newRow("Hello%Qt") << QString("Hello%Qt")<<QString("'Hello%Qt'");
+    QTest::newRow("Hello=Qt") << QString("Hello=Qt")<<QString("'Hello=Qt'");
+    QTest::newRow("Hello<>Qt") << QString("Hello<>Qt")<<QString("'Hello<>Qt'");
+    QTest::newRow("Hello,Qt") << QString("Hello,Qt")<<QString("'Hello,Qt'");
+    QTest::newRow("Hello&Qt") << QString("Hello&Qt")<<QString("'Hello&Qt'");
+    QTest::newRow("Hello'Qt") << QString("Hello'Qt")<<QString("'Hello''Qt'");
+
+}
+
+void UtilityTest::test_escapeSheetName()
+{
+    QFETCH(QString, original);
+    QFETCH(QString, result);
+
+    QCOMPARE(QXlsx::escapeSheetName(original), result);
+}
+
 void UtilityTest::test_convertSharedFormula_data()
 {
     QTest::addColumn<QString>("original");