Reset QTemporaryFile's state after failed open() on Windows

This fixes a regression introduced in 4.5.2 where QTemporaryFile would
no longer attempt to generate a random name after a failed open. Under
certain situations, this led to a non-random file being left behind in
QDir::tempPath when using the fallback implementation of QFile::copy.

Avoid calling QFSFileEngine::setFileName() on a template, so as not to
process it as file name.

By consistently not calling setFileTemplate in the constructor, we also
delay allocation of the fileEngine. Changes made to that function also
keep it from unnecessarily allocating the fileEngine.

Task-number: 260165
Reviewed-by: Thiago Macieira

2 files changed, 112 insertions, 8 deletions

diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp
index bd039c8559..47fa257ae5 100644
--- a/src/corelib/io/qtemporaryfile.cpp
+++ b/src/corelib/io/qtemporaryfile.cpp
@@ -292,14 +292,20 @@ class QTemporaryFileEngine : public QFSFileEngine
     Q_DECLARE_PRIVATE(QFSFileEngine)
 public:
     QTemporaryFileEngine(const QString &file, bool fileIsTemplate = true)
-        : QFSFileEngine(file), filePathIsTemplate(fileIsTemplate)
+        : QFSFileEngine(), filePathIsTemplate(fileIsTemplate)
     {
+        Q_D(QFSFileEngine);
+        d->filePath = file;
+
+        if (!filePathIsTemplate)
+            QFSFileEngine::setFileName(file);
     }
 
     ~QTemporaryFileEngine();
 
     bool isReallyOpen();
     void setFileName(const QString &file);
+    void setFileTemplate(const QString &fileTemplate);
 
     bool open(QIODevice::OpenMode flags);
     bool remove();
@@ -336,6 +342,13 @@ void QTemporaryFileEngine::setFileName(const QString &file)
     QFSFileEngine::setFileName(file);
 }
 
+void QTemporaryFileEngine::setFileTemplate(const QString &fileTemplate)
+{
+    Q_D(QFSFileEngine);
+    if (filePathIsTemplate)
+        d->filePath = fileTemplate;
+}
+
 bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
 {
     Q_D(QFSFileEngine);
@@ -382,12 +395,19 @@ bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
         return false;
     }
 
+    QString template_ = d->filePath;
     d->filePath = QString::fromLocal8Bit(filename);
-    filePathIsTemplate = false;
     d->nativeInitFileName();
-    d->closeFileHandle = true;
     delete [] filename;
-    return QFSFileEngine::open(openMode);
+
+    if (QFSFileEngine::open(openMode)) {
+        filePathIsTemplate = false;
+        return true;
+    }
+
+    d->filePath = template_;
+    d->nativeFilePath.clear();
+    return false;
 #endif
 }
 
@@ -533,7 +553,8 @@ QTemporaryFile::QTemporaryFile()
 QTemporaryFile::QTemporaryFile(const QString &templateName)
     : QFile(*new QTemporaryFilePrivate, 0)
 {
-    setFileTemplate(templateName);
+    Q_D(QTemporaryFile);
+    d->templateName = templateName;
 }
 
 /*!
@@ -567,7 +588,8 @@ QTemporaryFile::QTemporaryFile(QObject *parent)
 QTemporaryFile::QTemporaryFile(const QString &templateName, QObject *parent)
     : QFile(*new QTemporaryFilePrivate, parent)
 {
-    setFileTemplate(templateName);
+    Q_D(QTemporaryFile);
+    d->templateName = templateName;
 }
 #endif
 
@@ -671,10 +693,10 @@ QString QTemporaryFile::fileTemplate() const
 */
 void QTemporaryFile::setFileTemplate(const QString &name)
 {
-    Q_ASSERT(!isOpen());
     Q_D(QTemporaryFile);
-    fileEngine()->setFileName(name);
     d->templateName = name;
+    if (d->fileEngine)
+        static_cast<QTemporaryFileEngine*>(d->fileEngine)->setFileTemplate(name);
 }
 
 /*!
diff --git a/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp b/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp
index 2541b48d88..73486e709b 100644
--- a/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp
+++ b/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp
@@ -89,6 +89,8 @@ private slots:
     void renameFdLeak();
     void reOpenThroughQFile();
     void keepOpenMode();
+    void resetTemplateAfterError();
+    void setTemplateAfterOpen();
 
 public:
 };
@@ -472,5 +474,85 @@ void tst_QTemporaryFile::keepOpenMode()
     }
 }
 
+void tst_QTemporaryFile::resetTemplateAfterError()
+{
+    // calling setFileTemplate on a failed open
+
+    QString tempPath = QDir::tempPath();
+
+    QString const fileTemplate("destination/qt_temp_file_test.XXXXXX");
+    QString const fileTemplate2(tempPath + "/qt_temp_file_test.XXXXXX");
+
+    QVERIFY2( QDir(tempPath).exists() || QDir().mkpath(tempPath), "Test precondition" );
+    QVERIFY2( !QFile::exists("destination"), "Test precondition" );
+    QVERIFY2( !QFile::exists(fileTemplate2) || QFile::remove(fileTemplate2), "Test precondition" );
+
+    QFile file(fileTemplate2);
+    QByteArray fileContent("This file is intentionally NOT left empty.");
+
+    QVERIFY( file.open(QIODevice::ReadWrite | QIODevice::Truncate) );
+    QCOMPARE( file.write(fileContent), (qint64)fileContent.size() );
+    QVERIFY( file.flush() );
+
+    QString fileName;
+    {
+        QTemporaryFile temp;
+
+        QVERIFY( temp.fileName().isEmpty() );
+        QVERIFY( !temp.fileTemplate().isEmpty() );
+
+        temp.setFileTemplate( fileTemplate );
+
+        QVERIFY( temp.fileName().isEmpty() );
+        QCOMPARE( temp.fileTemplate(), fileTemplate );
+
+        QVERIFY( !temp.open() );
+
+        QVERIFY( temp.fileName().isEmpty() );
+        QCOMPARE( temp.fileTemplate(), fileTemplate );
+
+        temp.setFileTemplate( fileTemplate2 );
+        QVERIFY( temp.open() );
+
+        fileName = temp.fileName();
+        QVERIFY( QFile::exists(fileName) );
+        QVERIFY( !fileName.isEmpty() );
+        QVERIFY2( fileName != fileTemplate2,
+            ("Generated name shouldn't be same as template: " + fileTemplate2).toLocal8Bit().constData() );
+    }
+
+    QVERIFY( !QFile::exists(fileName) );
+
+    file.seek(0);
+    QCOMPARE( QString(file.readAll()), QString(fileContent) );
+    QVERIFY( file.remove() );
+}
+
+void tst_QTemporaryFile::setTemplateAfterOpen()
+{
+    QTemporaryFile temp;
+
+    QVERIFY( temp.fileName().isEmpty() );
+    QVERIFY( !temp.fileTemplate().isEmpty() );
+
+    QVERIFY( temp.open() );
+
+    QString const fileName = temp.fileName();
+    QString const newTemplate("funny-path/funny-name-XXXXXX.tmp");
+
+    QVERIFY( !fileName.isEmpty() );
+    QVERIFY( QFile::exists(fileName) );
+    QVERIFY( !temp.fileTemplate().isEmpty() );
+    QVERIFY( temp.fileTemplate() != newTemplate );
+
+    temp.close(); // QTemporaryFile::setFileTemplate will assert on isOpen() up to 4.5.2
+    temp.setFileTemplate(newTemplate);
+    QCOMPARE( temp.fileTemplate(), newTemplate );
+
+    QVERIFY( temp.open() );
+    QCOMPARE( temp.fileName(), fileName );
+    QCOMPARE( temp.fileTemplate(), newTemplate );
+}
+
 QTEST_MAIN(tst_QTemporaryFile)
 #include "tst_qtemporaryfile.moc"