Skip to content

Commit

Permalink
Fix #907 file leaking by making a white-list for zipping pclx
Browse files Browse the repository at this point in the history 
- plus many debug details during the saving process
  • Loading branch information
@chchwy
chchwy committed Jun 5, 2018
1 parent 3451444 commit 0248361
Show file tree
Hide file tree
Showing 12 changed files with 134 additions and 134 deletions.
47 changes: 20 additions & 27 deletions core_lib/src/qminiz.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,76 +38,69 @@ bool MiniZ::isZip(const QString& sZipFilePath)
return (num > 0);
}

Status MiniZ::compressFolder(QString sZipFilePath, QString sSrcPath)
// ReSharper disable once CppInconsistentNaming
Status MiniZ::compressFolder(QString zipFilePath, QString srcFolderPath, const QStringList& fileList)
{
DebugDetails dd;
dd << QString("Zip the folder %1 to %2").arg(sZipFilePath).arg(sSrcPath);
dd << QString("Zip the folder %1 to %2").arg(zipFilePath).arg(srcFolderPath);

if (!sSrcPath.endsWith("/"))
if (!srcFolderPath.endsWith("/"))
{
sSrcPath.append("/");
srcFolderPath.append("/");
}

sSrcPath = QDir::toNativeSeparators(sSrcPath);
sZipFilePath = QDir::toNativeSeparators(sZipFilePath);

mz_zip_archive* mz = new mz_zip_archive;
OnScopeExit(delete mz);
mz_zip_zero_struct(mz);

mz_bool ok = mz_zip_writer_init_file(mz, sZipFilePath.toUtf8().data(), 0);
mz_bool ok = mz_zip_writer_init_file(mz, zipFilePath.toUtf8().data(), 0);

if (!ok)
{
dd << "Miniz writer init failed.";
}

QDirIterator it(sSrcPath, QDirIterator::Subdirectories);
while (it.hasNext())
qDebug() << "SrcFolder=" << srcFolderPath;
for (QString filePath : fileList)
{
QString sFullPath = it.next();

if (it.fileInfo().isDir())
{
continue;
}

QString sRelativePath = sFullPath;
sRelativePath.replace(sSrcPath, "");
QString sRelativePath = filePath;
sRelativePath.replace(srcFolderPath, "");

dd << QString("Add file to zip: ").append(sRelativePath);

ok = mz_zip_writer_add_file(mz,
sRelativePath.toUtf8().data(),
sFullPath.toUtf8().data(),
filePath.toUtf8().data(),
"", 0, MZ_DEFAULT_COMPRESSION);
qDebug() << "Zip: " << filePath;
if (!ok)
{
dd << QString(" Cannot add %1 to zip").arg(sRelativePath);
}
}
ok &= mz_zip_writer_finalize_archive(mz);
mz_zip_writer_end(mz);

if (!ok)
{
dd << "Miniz finalize archive failed";
return Status(Status::FAIL, dd);
}

mz_zip_writer_end(mz);

return Status::OK;
}

Status MiniZ::uncompressFolder(QString sZipFilePath, QString sDestPath)
Status MiniZ::uncompressFolder(QString zipFilePath, QString destPath)
{
DebugDetails dd;
dd << QString("Unzip file %1 to folder %2").arg(sZipFilePath).arg(sDestPath);
dd << QString("Unzip file %1 to folder %2").arg(zipFilePath).arg(destPath);

if (!QFile::exists(sZipFilePath))
if (!QFile::exists(zipFilePath))
{
return Status::FILE_NOT_FOUND;
}

QString sBaseDir = QFileInfo(sDestPath).absolutePath();
QString sBaseDir = QFileInfo(destPath).absolutePath();
QDir baseDir(sBaseDir);
if (!baseDir.exists())
{
Expand All @@ -121,7 +114,7 @@ Status MiniZ::uncompressFolder(QString sZipFilePath, QString sDestPath)
OnScopeExit(delete mz);
mz_zip_zero_struct(mz);

mz_bool ok = mz_zip_reader_init_file(mz, sZipFilePath.toUtf8().data(), 0);
mz_bool ok = mz_zip_reader_init_file(mz, zipFilePath.toUtf8().data(), 0);
if (!ok)
return Status(Status::FAIL, dd);

Expand Down
4 changes: 2 additions & 2 deletions core_lib/src/qminiz.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ GNU General Public License for more details.
namespace MiniZ
{
bool isZip(const QString& sZipFilePath);
Status compressFolder(QString sZipFilePath, QString sSrcPath);
Status uncompressFolder(QString sZipFilePath, QString sDestPath);
Status compressFolder(QString zipFilePath, QString srcFolderPath, const QStringList& fileList);
Status uncompressFolder(QString zipFilePath, QString destPath);
}
#endif
89 changes: 46 additions & 43 deletions core_lib/src/structure/filemanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ bool FileManager::loadObjectOldWay(Object* object, const QDomElement& root)
return object->loadXML(root, [this] { progressForward(); });
}

bool FileManager::isOldForamt(const QString& fileName)
bool FileManager::isOldForamt(const QString& fileName) const
{
return (MiniZ::isZip(fileName) == false);
return !MiniZ::isZip(fileName);
}

Status FileManager::save(Object* object, QString strFileName)
Expand Down Expand Up @@ -284,43 +284,42 @@ Status FileManager::save(Object* object, QString strFileName)
tr("The path (\"%1\") is not writable.").arg(fileInfo.absoluteFilePath()));
}

QString strTempWorkingFolder;
QString strMainXMLFile;
QString strDataFolder;
QString sTempWorkingFolder;
QString sMainXMLFile;
QString sDataFolder;

bool isOldFile = strFileName.endsWith(PFF_OLD_EXTENSION);
if (isOldFile)
bool isOldType = strFileName.endsWith(PFF_OLD_EXTENSION);
if (isOldType)
{
qCDebug(mLog) << "Old Pencil File Format (*.pcl) !";
dd << "Old Pencil File Format (*.pcl) !";

strMainXMLFile = strFileName;
strDataFolder = strMainXMLFile + "." + PFF_OLD_DATA_DIR;
sMainXMLFile = strFileName;
sDataFolder = sMainXMLFile + "." + PFF_OLD_DATA_DIR;
}
else
{
qCDebug(mLog) << "New zipped Pencil File Format (*.pclx) !";
dd << "New zipped Pencil File Format (*.pclx) !";

strTempWorkingFolder = object->workingDir();
Q_ASSERT(QDir(strTempWorkingFolder).exists());
dd << QString("strTempWorkingFolder = ").append(strTempWorkingFolder);
sTempWorkingFolder = object->workingDir();
Q_ASSERT(QDir(sTempWorkingFolder).exists());
dd << QString("TempWorkingFolder = ").append(sTempWorkingFolder);

qCDebug(mLog) << "Temp Folder=" << strTempWorkingFolder;
strMainXMLFile = QDir(strTempWorkingFolder).filePath(PFF_XML_FILE_NAME);
strDataFolder = QDir(strTempWorkingFolder).filePath(PFF_OLD_DATA_DIR);
sMainXMLFile = QDir(sTempWorkingFolder).filePath(PFF_XML_FILE_NAME);
sDataFolder = QDir(sTempWorkingFolder).filePath(PFF_OLD_DATA_DIR);
}

QFileInfo dataInfo(strDataFolder);
QFileInfo dataInfo(sDataFolder);
if (!dataInfo.exists())
{
QDir dir(strDataFolder); // the directory where all key frames will be saved
QDir dir(sDataFolder); // the directory where all key frames will be saved

if (!dir.mkpath(strDataFolder))
if (!dir.mkpath(sDataFolder))
{
dd << QString("dir.absolutePath() = %1").arg(dir.absolutePath());

return Status(Status::FAIL, dd,
tr("Cannot Create Data Directory"),
tr("Failed to create directory \"%1\". Please make sure you have sufficient permissions.").arg(strDataFolder));
tr("Failed to create directory \"%1\". Please make sure you have sufficient permissions.").arg(sDataFolder));
}
}
if (!dataInfo.isDir())
Expand All @@ -333,39 +332,41 @@ Status FileManager::save(Object* object, QString strFileName)
}

// save data
int layerCount = object->getLayerCount();
dd << QString("layerCount = %1").arg(layerCount);
int numLayers = object->getLayerCount();
dd << QString("Total %1 layers").arg(numLayers);

QStringList attachedFiles;

bool saveLayerOK = true;
for (int i = 0; i < layerCount; ++i)
bool saveLayersOK = true;
for (int i = 0; i < numLayers; ++i)
{
Layer* layer = object->getLayer(i);
dd << QString("layer[%1] = Layer[id=%2, name=%3, type=%4]").arg(i).arg(layer->id()).arg(layer->name()).arg(layer->type());
dd << QString("Layer[%1] = [id=%2, name=%3, type=%4]").arg(i).arg(layer->id()).arg(layer->name()).arg(layer->type());

Status st = layer->save(strDataFolder, [this] { progressForward(); });
Status st = layer->save(sDataFolder, attachedFiles, [this] { progressForward(); });
if (!st.ok())
{
saveLayerOK = false;
saveLayersOK = false;
dd.collect(st.details());
dd << QString(" !! Failed to save Layer[%1] %2 ").arg(i).arg(layer->name());
dd << QString(" !! Failed to save Layer[%1] %2").arg(i).arg(layer->name());
}
}
dd << "All Layers saved";

// save palette
bool bPaletteOK = object->savePalette(strDataFolder);
if (!bPaletteOK)
{
QString sPaletteFile = object->savePalette(sDataFolder);
if (!sPaletteFile.isEmpty())
attachedFiles.append(sPaletteFile);
else
dd << "Failed to save palette";
}


progressForward();

// -------- save main XML file -----------
QFile file(strMainXMLFile);
QFile file(sMainXMLFile);
if (!file.open(QFile::WriteOnly | QFile::Text))
{
return Status::ERROR_FILE_CANNOT_OPEN;
return Status(Status::ERROR_FILE_CANNOT_OPEN, dd);
}

QDomDocument xmlDoc("PencilDocument");
Expand All @@ -377,30 +378,32 @@ Status FileManager::save(Object* object, QString strFileName)
progressForward();

// save editor information
QDomElement projectDataElement = saveProjectData(object->data(), xmlDoc);
root.appendChild(projectDataElement);
QDomElement projDataXml = saveProjectData(object->data(), xmlDoc);
root.appendChild(projDataXml);

// save object
QDomElement objectElement = object->saveXML(xmlDoc);
root.appendChild(objectElement);

dd << "Writing main xml file...";

const int IndentSize = 2;
const int indentSize = 2;

QTextStream out(&file);
xmlDoc.save(out, IndentSize);
xmlDoc.save(out, indentSize);
out.flush();
file.close();

dd << "Done writing main xml file";

attachedFiles.append(sMainXMLFile);

progressForward();

if (!isOldFile)
if (!isOldType)
{
dd << "Miniz";
Status s = MiniZ::compressFolder(strFileName, strTempWorkingFolder);
Status s = MiniZ::compressFolder(strFileName, sTempWorkingFolder, attachedFiles);
if (!s.ok())
{
dd.collect(s.details());
Expand All @@ -416,7 +419,7 @@ Status FileManager::save(Object* object, QString strFileName)

progressForward();

if (!saveLayerOK)
if (!saveLayersOK)
{
return Status(Status::FAIL, dd,
tr("Internal Error"),
Expand Down
4 changes: 2 additions & 2 deletions core_lib/src/structure/filemanager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class FileManager : public QObject
Status save(Object*, QString strFileName);

QList<ColourRef> loadPaletteFile(QString strFilename);
Status error() { return mError; }
Status error() const { return mError; }
Status verifyObject(Object* obj);

Q_SIGNALS:
Expand All @@ -54,7 +54,7 @@ class FileManager : public QObject

bool loadObject(Object*, const QDomElement& root);
bool loadObjectOldWay(Object*, const QDomElement& root);
bool isOldForamt(const QString& fileName);
bool isOldForamt(const QString& fileName) const;
bool loadPalette(Object*);

ObjectData* loadProjectData(const QDomElement& element);
Expand Down
49 changes: 20 additions & 29 deletions core_lib/src/structure/layer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,30 +277,32 @@ bool Layer::loadKey(KeyFrame* pKey)
return true;
}

Status Layer::save(QString strDataFolder, ProgressCallback progressStep)
Status Layer::save(const QString& sDataFolder, QStringList& attachedFiles, ProgressCallback progressStep)
{
DebugDetails debugInfo;
debugInfo << "Layer::save";
debugInfo << ("strDataFolder = " + strDataFolder);
DebugDetails dd;
dd << __FUNCTION__;

bool isOkay = true;
bool ok = true;

for (auto pair : mKeyFrames)
{
KeyFrame* pKeyFrame = pair.second;
Status st = saveKeyFrameFile(pKeyFrame, strDataFolder);
if (!st.ok())
KeyFrame* keyFrame = pair.second;
Status st = saveKeyFrameFile(keyFrame, sDataFolder);
if (st.ok())
{
isOkay = false;

debugInfo.collect(st.details());
debugInfo << QString("- Keyframe[%1] failed to save").arg(pKeyFrame->pos()); // << keyFrameDetails;
attachedFiles.append(keyFrame->fileName());
}
else
{
ok = false;
dd.collect(st.details());
dd << QString("- Keyframe[%1] failed to save").arg(keyFrame->pos()); // << keyFrameDetails;
}
progressStep();
}
if (!isOkay)
if (!ok)
{
return Status(Status::FAIL, debugInfo);
return Status(Status::FAIL, dd);
}
return Status::OK;
}
Expand Down Expand Up @@ -441,7 +443,7 @@ void Layer::setModified(int position, bool modified)
}
}

bool Layer::isFrameSelected(int position)
bool Layer::isFrameSelected(int position) const
{
KeyFrame* keyFrame = getKeyFrameWhichCovers(position);
if (keyFrame)
Expand Down Expand Up @@ -636,28 +638,17 @@ bool Layer::moveSelectedFrames(int offset)
return false;
}

bool Layer::isPaintable()
bool Layer::isPaintable() const
{
switch (type())
{
case BITMAP:
case VECTOR:
return true;
case CAMERA:
case SOUND:
return false;
default:
break;
}
return false;
return (type() == BITMAP || type() == VECTOR);
}

bool Layer::keyExistsWhichCovers(int frameNumber)
{
return getKeyFrameWhichCovers(frameNumber) != nullptr;
}

KeyFrame* Layer::getKeyFrameWhichCovers(int frameNumber)
KeyFrame* Layer::getKeyFrameWhichCovers(int frameNumber) const
{
auto keyFrame = getLastKeyFrameAtPosition(frameNumber);
if (keyFrame != nullptr)
Expand Down
Loading

0 comments on commit 0248361

Please sign in to comment.