Skip to content

Commit fcb8d64

Browse files
SlySvenSlySven
committed
BugFix: scan for and create needed directories when installing packages
Some file archiving production systems do not seem to be inserting the (zero length) entries with names ending in a '/' for sub-directories in archive files. This commit now makes two passes through any package or module and identifies all directories that will be needed, it then makes them before it extracts the files to populate those directories. This should fix https://bugs.launchpad.net/mudlet/+bug/1413069 as it also ensures that the proper cleaning process is done should package installation fail, it removes the "Unpacking" dialog on failure which was not happening before. NOTE: There is still an extreme lack of error reporting but as Host::installPackage(...) is used in several places by different parts of the application that is probably addressed separately. Also: * wrapped strings in Host::installPackage(...) in QStringLiteral(...) or tr(...) wrappers and made it more likely to be Utf8 clean. * Increased the size of the buffer used during extraction of possibly compressed archived package files from 100 to 4096 bytes which means less loops will be needed to process such archived files. * Ensured path / file name manipulations/test are not Case Sensitive so that they will not misbehave on MacOS platforms. * Tweaked displaying of the "Unpacking package/module..." dialog so that it shows more consistently (it often hid under other windows on my GNU/Linux setup!) Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
1 parent a8fc5c2 commit fcb8d64

1 file changed

Lines changed: 214 additions & 78 deletions

File tree

‎src/Host.cpp‎

Lines changed: 214 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -725,21 +725,24 @@ bool Host::installPackage(const QString& fileName, int module )
725725
// a script. This separation is necessary to be able to reuse code
726726
// while avoiding infinite loops from script installations.
727727

728-
if( fileName.isEmpty() ) return false;
728+
if( fileName.isEmpty() )
729+
{
730+
return false;
731+
}
729732

730733
QFile file(fileName);
731734
if( ! file.open(QFile::ReadOnly | QFile::Text) )
732735
{
733736
return false;
734737
}
735-
QString packageName = fileName.section("/", -1);
736-
packageName.replace( ".zip" , "" );
737-
packageName.replace( "trigger", "" );
738-
packageName.replace( "xml", "" );
739-
packageName.replace( ".mpackage" , "" );
740-
packageName.replace( '/' , "" );
741-
packageName.replace( '\\' , "" );
742-
packageName.replace( '.' , "" );
738+
739+
QString packageName = fileName.section( QStringLiteral( "/" ), -1 );
740+
packageName.remove( QStringLiteral( ".trigger" ), Qt::CaseInsensitive );
741+
packageName.remove( QStringLiteral( ".xml" ), Qt::CaseInsensitive );
742+
packageName.remove( QStringLiteral( ".zip" ), Qt::CaseInsensitive );
743+
packageName.remove( QStringLiteral( ".mpackage" ), Qt::CaseInsensitive );
744+
packageName.remove( QLatin1Char( '\\' ) );
745+
packageName.remove( QLatin1Char( '.' ) );
743746
if ( module )
744747
{
745748
if( (module == 2) && (mActiveModules.contains( packageName ) ))
@@ -764,33 +767,57 @@ bool Host::installPackage(const QString& fileName, int module )
764767
mpEditorDialog->doCleanReset();
765768
}
766769
QFile file2;
767-
if( fileName.endsWith(".zip") || fileName.endsWith(".mpackage") )
770+
if( fileName.endsWith( QStringLiteral( ".zip" ), Qt::CaseInsensitive )
771+
|| fileName.endsWith( QStringLiteral( ".mpackage"), Qt::CaseInsensitive ) )
768772
{
769-
QString _home = QDir::homePath();
770-
_home.append( "/.config/mudlet/profiles/" );
771-
_home.append( getName() );
772-
QString _dest = QString( "%1/%2/").arg( _home ).arg( packageName );
773-
QDir _tmpDir;
774-
_tmpDir.mkpath(_dest);
775-
776-
QUiLoader loader;
777-
QFile file(":/ui/package_manager_unpack.ui");
778-
file.open(QFile::ReadOnly);
779-
pUnzipDialog = dynamic_cast<QDialog *>(loader.load( &file, 0 ) );
780-
file.close();
781-
if( ! pUnzipDialog ) return false;
782-
783-
pUnzipDialog->setWindowTitle( tr( "Unpacking package: %1" ).arg( fileName ) );
773+
QString _home = QStringLiteral( "%1/.config/mudlet/profiles/%2" )
774+
.arg( QDir::homePath() )
775+
.arg( getName() );
776+
QString _dest = QStringLiteral( "%1/%2/" )
777+
.arg( _home )
778+
.arg( packageName );
779+
QDir _tmpDir( _home ); // home directory for the PROFILE
780+
_tmpDir.mkpath( _dest );
781+
// TODO: report failure to create destination folder for package/module in profile
782+
783+
QUiLoader loader( this );
784+
QFile uiFile( QStringLiteral( ":/ui/package_manager_unpack.ui" ) );
785+
uiFile.open(QFile::ReadOnly);
786+
pUnzipDialog = dynamic_cast<QDialog *>(loader.load( &uiFile, 0 ) );
787+
uiFile.close();
788+
if( ! pUnzipDialog )
789+
{
790+
return false;
791+
}
792+
793+
QLabel * pLabel = pUnzipDialog->findChild<QLabel*>( QStringLiteral( "label" ) );
794+
if( pLabel )
795+
{
796+
if( module )
797+
{
798+
pLabel->setText( tr( "Unpacking module:\n\"%1\"\nplease wait..." ).arg( packageName ) );
799+
}
800+
else
801+
{
802+
pLabel->setText( tr( "Unpacking package:\n\"%1\"\nplease wait..." ).arg( packageName ) );
803+
}
804+
}
805+
pUnzipDialog->setWindowTitle( tr( "Unpacking" ) );
806+
pUnzipDialog->hide();
807+
pUnzipDialog->setWindowModality( Qt::ApplicationModal );
784808
pUnzipDialog->show();
785-
pUnzipDialog->raise();
786809
qApp->processEvents();
810+
pUnzipDialog->raise();
811+
pUnzipDialog->show(); // Must do this to ensure modality is applied
812+
pUnzipDialog->update();
813+
qApp->processEvents(); // Try to ensure we are on top of any other dialogs
787814

788815
int err = 0;
789816
//from: https://gist.github.com/mobius/1759816
790817
struct zip_stat zs;
791818
struct zip_file *zf;
792-
long long sum;
793-
char buf[100];
819+
zip_uint64_t bytesRead = 0;
820+
char buf[4096]; // Was 100 but that seems unduely stingy...!
794821
zip* archive = zip_open( fileName.toStdString().c_str(), 0, &err);
795822
if ( err != 0 )
796823
{
@@ -803,72 +830,177 @@ bool Host::installPackage(const QString& fileName, int module )
803830
}
804831
return false;
805832
}
806-
for (int i=0;i<zip_get_num_entries( archive, 0 );i++ )
833+
834+
// We now scan for directories first, and gather needed ones first, not
835+
// just relying on (zero length) archive entries ending in '/' as some
836+
// (possibly broken) archive building libraries seem to forget to
837+
// include them.
838+
QMap<QString, QString> directoriesNeededMap;
839+
// Key is: relative path stored in archive
840+
// Value is: absolute path needed when extracting files
841+
for ( zip_int64_t i = 0, total = zip_get_num_entries( archive, 0 ); i < total; ++i )
807842
{
808-
int zsi = zip_stat_index( archive, i, 0, &zs );
809-
if( zsi == 0 )
843+
if ( ! zip_stat_index( archive, static_cast<zip_uint64_t>( i ), 0, &zs ) )
810844
{
811-
if ( zs.name[strlen( zs.name )-1] == '/' )
845+
QString entryInArchive( QString::fromUtf8( zs.name ) );
846+
QString pathInArchive( entryInArchive.section( QLatin1Literal( "/" ), 0, -2 ) );
847+
// TODO: We are supposed to validate the fields (except the
848+
// "valid" one itself) in zs before using them:
849+
// i.e. check that zs.name is valid ( zs.valid & ZIP_STAT_NAME )
850+
if ( entryInArchive.endsWith( QLatin1Char( '/' ) ) )
812851
{
813-
QDir dir = QDir( _dest );
814-
if ( !dir.exists( zs.name ) )
815-
{
816-
if ( dir.mkdir( zs.name ) == false )
817-
{
818-
//FIXME: report error to user
819-
//qDebug()<<"error creating subdirectory: "<<QString(zs.name);
820-
}
852+
// qDebug() << "Host::installPackage() Scanning archive (for directories) found item:" << i << "called:" << entryInArchive << "this is a DIRECTORY...!";
853+
if ( ! directoriesNeededMap.contains( pathInArchive ) ) {
854+
QString pathInProfile( QStringLiteral( "%1/%2" )
855+
.arg( packageName )
856+
.arg( pathInArchive ) );
857+
directoriesNeededMap.insert( pathInArchive, pathInProfile );
858+
// qDebug() << "Added:" << pathInArchive << "to list of sub-directories to be made.";
821859
}
860+
// else
861+
// {
862+
// qDebug() << "No need to add:" << pathInArchive << "we have already spotted the need for it!";
863+
// }
822864
}
823865
else
824866
{
825-
zf = zip_fopen_index( archive, i, 0 );
826-
if ( !zf )
867+
// qDebug() << "Host::installPackage() Scanning archive (for directories) found item:" << i << "called:" << entryInArchive << "this is a FILE...!";
868+
// Extract needed path from name for archives that do NOT
869+
// explicitly list directories
870+
if( ! pathInArchive.isEmpty() && ! directoriesNeededMap.contains( pathInArchive ) ) {
871+
QString pathInProfile( QStringLiteral( "%1/%2" )
872+
.arg( packageName )
873+
.arg( pathInArchive ) );
874+
directoriesNeededMap.insert( pathInArchive, pathInProfile );
875+
// qDebug() << "Added:" << pathInArchive << "to list of sub-directories to be made.";
876+
}
877+
// else
878+
// {
879+
// qDebug() << "No need to add:" << pathInArchive << "we have already spotted the need for it!";
880+
// }
881+
}
882+
}
883+
else
884+
{
885+
// TODO: Report failure to obtain an archive entry to parse
886+
}
887+
}
888+
889+
// Now create the needed directories:
890+
QMapIterator<QString, QString> itPath( directoriesNeededMap );
891+
while( itPath.hasNext() )
892+
{
893+
itPath.next();
894+
// qDebug() << "Host::installPackage(...) INFO testing for presence of:"
895+
// << itPath.value()
896+
// << "relative to:"
897+
// << _home;
898+
if( ! _tmpDir.exists( itPath.value() ) )
899+
{
900+
if( ! _tmpDir.mkpath( itPath.value() ) )
901+
{
902+
// TODO: report failure to create needed sub-directory
903+
// within package destination directory in profile directory
904+
905+
zip_close( archive );
906+
if( pUnzipDialog ) {
907+
pUnzipDialog->deleteLater();
908+
pUnzipDialog = Q_NULLPTR;
909+
// Previously we forgot to close the dialog if we aborted
910+
}
911+
return false; // Abort reading rest of archive
912+
}
913+
_tmpDir.refresh();
914+
}
915+
}
916+
917+
// Now extract the files
918+
for ( zip_int64_t i = 0, total = zip_get_num_entries( archive, 0 ); i < total; ++i )
919+
{
920+
// No need to check return value as we've already done it first time
921+
zip_stat_index( archive, static_cast<zip_uint64_t>( i ), 0, &zs );
922+
QString entryInArchive( QString::fromUtf8( zs.name ) );
923+
if ( ! entryInArchive.endsWith( QLatin1Char( '/' ) ) )
924+
{
925+
// TODO: check that zs.size is valid ( zs.valid & ZIP_STAT_SIZE )
926+
zf = zip_fopen_index( archive, static_cast<zip_uint64_t>( i ), 0 );
927+
if ( !zf )
928+
{
929+
int sep = 0;
930+
zip_error_get( archive, &err, &sep );
931+
zip_error_to_str(buf, sizeof(buf), err, errno);
932+
// FIXME: report error to user, zip_error_to_str(...) is
933+
// already deprecated, if not obsoleted...! - Slysven
934+
zip_close( archive );
935+
if ( pUnzipDialog )
936+
{
937+
pUnzipDialog->deleteLater();
938+
pUnzipDialog = Q_NULLPTR;
939+
}
940+
return false;
941+
}
942+
943+
QFile fd( QStringLiteral( "%1%2" )
944+
.arg( _dest )
945+
.arg( entryInArchive ) );
946+
947+
if ( !fd.open( QIODevice::ReadWrite|QIODevice::Truncate ) )
948+
{
949+
//FIXME: report error to user
950+
qDebug() << "Host::installPackage("
951+
<< fileName
952+
<< ","
953+
<< module
954+
<< ")\n ERROR opening:"
955+
<< QStringLiteral( "%1%2" ).arg( _dest ).arg( entryInArchive )
956+
<< "!\n Reported error was:"
957+
<< fd.errorString();
958+
zip_fclose( zf );
959+
zip_close( archive );
960+
if ( pUnzipDialog )
827961
{
828-
int sep = 0;
829-
zip_error_get( archive, &err, &sep);
830-
zip_error_to_str(buf, sizeof(buf), err, errno);
831-
//FIXME: report error to user
962+
pUnzipDialog->deleteLater();
963+
pUnzipDialog = Q_NULLPTR;
964+
}
965+
return false;
966+
}
967+
968+
bytesRead = 0;
969+
zip_uint64_t bytesExpected = zs.size;
970+
while( bytesRead < bytesExpected && fd.error() == QFileDevice::NoError )
971+
{
972+
zip_int64_t len = zip_fread( zf, buf, sizeof( buf ) );
973+
if ( len < 0 )
974+
{
975+
//FIXME: report error to user qDebug()<<"zip_fread error"<<len;
976+
fd.close();
977+
zip_fclose( zf );
978+
zip_close( archive );
832979
if ( pUnzipDialog )
833980
{
834981
pUnzipDialog->deleteLater();
835982
pUnzipDialog = Q_NULLPTR;
836983
}
837984
return false;
838985
}
839-
QFile fd(_dest+QString(zs.name));
840-
fd.open(QIODevice::ReadWrite|QIODevice::Truncate);
841-
if ( !fd.isOpen() )
986+
987+
if( fd.write( buf, len ) == -1 )
842988
{
843-
//FIXME: report error to user qDebug()<<"error opening"<<_dest+QString(zs.name);
989+
// TODO: Report failure to write data to actual file
990+
fd.close();
991+
zip_fclose( zf );
992+
zip_close( archive );
844993
if ( pUnzipDialog )
845994
{
846995
pUnzipDialog->deleteLater();
847996
pUnzipDialog = Q_NULLPTR;
848997
}
849998
return false;
850999
}
851-
sum = 0;
852-
//HEIKO: comparison between signed and unsigned
853-
while( static_cast<zip_uint64_t>(sum) != zs.size )
854-
{
855-
int len = zip_fread( zf, buf, 100 );
856-
if ( len < 0 )
857-
{
858-
//FIXME: report error to user qDebug()<<"zip_fread error"<<len;
859-
if ( pUnzipDialog )
860-
{
861-
pUnzipDialog->deleteLater();
862-
pUnzipDialog = Q_NULLPTR;
863-
}
864-
return false;
865-
}
866-
fd.write( buf, len );
867-
sum += len;
868-
}
869-
fd.close();
870-
zip_fclose( zf );
1000+
bytesRead += static_cast<zip_uint64_t>( len );
8711001
}
1002+
fd.close();
1003+
zip_fclose( zf );
8721004
}
8731005
}
8741006

@@ -899,10 +1031,10 @@ bool Host::installPackage(const QString& fileName, int module )
8991031
QDir _dir( _dest );
9001032
// before we start importing xmls in, see if the config.lua manifest file exists
9011033
// - if it does, update the packageName from it
902-
if (_dir.exists("config.lua"))
1034+
if ( _dir.exists( QStringLiteral( "config.lua" ) ) )
9031035
{
9041036
// read in the new packageName from Lua. Should be expanded in future to whatever else config.lua will have
905-
readPackageConfig(_dir.absoluteFilePath("config.lua"), packageName);
1037+
readPackageConfig( _dir.absoluteFilePath( QStringLiteral( "config.lua" ) ), packageName );
9061038
// now that the packageName changed, redo relevant checks to make sure it's still valid
9071039
if (module)
9081040
{
@@ -921,12 +1053,12 @@ bool Host::installPackage(const QString& fileName, int module )
9211053
}
9221054
}
9231055
// continuing, so update the folder name on disk
924-
QString newpath(QString( "%1/%2/").arg( _home ).arg( packageName ));
1056+
QString newpath( QStringLiteral( "%1/%2/" ).arg( _home ).arg( packageName ));
9251057
_dir.rename(_dir.absolutePath(), newpath);
9261058
_dir = QDir( newpath );
9271059
}
9281060
QStringList _filterList;
929-
_filterList << "*.xml" << "*.trigger";
1061+
_filterList << QStringLiteral( "*.xml" ) << QStringLiteral( "*.trigger" );
9301062
QFileInfoList entries = _dir.entryInfoList( _filterList, QDir::Files );
9311063
for( int i=0; i<entries.size(); i++ )
9321064
{
@@ -940,12 +1072,14 @@ bool Host::installPackage(const QString& fileName, int module )
9401072
{
9411073
QStringList moduleEntry;
9421074
moduleEntry << fileName;
943-
moduleEntry << "0";
944-
mInstalledModules[packageName] = moduleEntry;//.append( packageName );
1075+
moduleEntry << QStringLiteral( "0" );
1076+
mInstalledModules[packageName] = moduleEntry;
9451077
mActiveModules.append(packageName);
9461078
}
9471079
else
1080+
{
9481081
mInstalledPackages.append( packageName );
1082+
}
9491083
reader.importPackage( & file2, packageName, module);
9501084
setName( profileName );
9511085
setLogin( login );
@@ -966,12 +1100,14 @@ bool Host::installPackage(const QString& fileName, int module )
9661100
{
9671101
QStringList moduleEntry;
9681102
moduleEntry << fileName;
969-
moduleEntry << "0";
970-
mInstalledModules[packageName] = moduleEntry;//.append( packageName );
1103+
moduleEntry << moduleEntry << QStringLiteral( "0" );
1104+
mInstalledModules[packageName] = moduleEntry;
9711105
mActiveModules.append(packageName);
9721106
}
9731107
else
1108+
{
9741109
mInstalledPackages.append( packageName );
1110+
}
9751111
reader.importPackage( & file2, packageName, module);
9761112
setName( profileName );
9771113
setLogin( login );

0 commit comments

Comments
 (0)