Port against maintained minizip

[praiskup]

The contrib module from zlib named minizip is not maintained nowadays, and both original authors of minizip redirected me to the updated minizip fork https://github.com/nmoinvaz/minizip , it would be nice to allow compilation against that library.

I tried something similar:

--- a/cmake/modules/FindMINIZIP.cmake
+++ b/cmake/modules/FindMINIZIP.cmake
@@ -40,7 +40,7 @@ if ( UNIX AND NOT( APPLE OR CYGWIN ) )
 endif ( UNIX AND NOT( APPLE OR CYGWIN ) )
 
 set ( LIBINCS 
-       unzip.h
+       mz_compat.h
 )
 
 find_path(
diff --git a/src/libpsi/tools/zip/zip.cpp b/src/libpsi/tools/zip/zip.cpp
index 92f36f8..963653c 100644
--- a/src/libpsi/tools/zip/zip.cpp
+++ b/src/libpsi/tools/zip/zip.cpp
@@ -22,11 +22,7 @@
 #include <QStringList>
 #include <QFile>
 
-#ifdef PSIMINIZIP
-#include "minizip/unzip.h"
-#else
-#include <unzip.h>
-#endif
+#include <mz_compat.h>
 #include "zip.h"
 
 class UnZipPrivate

But the build failed with:

FAILED: src/libpsi/tools/zip/CMakeFiles/zip.dir/zip.cpp.o 
/usr/bin/c++  -DAPP_BIN_NAME=psi -DAPP_PREFIX=/usr -DFILETRANSFER -DGROUPCHAT -DHAVE_CONFIG -DHAVE_FREEDESKTOP -DHAVE_HUNSPELL -DHAVE_PGPUTIL -DHAVE_QT5 -DHAVE_X11 -DNEWCONTACTLIST -DPSI_PLUGINS -DQT_CONCURRENT_LIB -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_MULTIMEDIA_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_STATICPLUGIN -DQT_SVG_LIB -DQT_WEBKITWIDGETS_LIB -DQT_WEBKIT_LIB -DQT_WIDGETS_LIB -DQT_X11EXTRAS_LIB -DQT_XML_LIB -DUSE_DBUS -DUSE_PEP -DWEBKIT -DWHITEBOARDING -I/usr/include/hunspell -I../src -Isrc -I../ -I/usr/include/QtCrypto -I../iris/src -I../iris/include -I../iris/include/iris -I../src/. -I../src/plugins/include -I../src/libpsi/tools/zip -Isrc/libpsi/tools/zip -I../src/libpsi/tools/zip/minizip -I../src/libpsi/tools/zip/.. -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtWidgets -isystem /usr/include/qt5/QtGui -isystem /usr/include/qt5/QtCore -isystem /usr/lib64/qt5/mkspecs/linux-g++ -isystem /usr/include/qt5/QtNetwork -isystem /usr/include/qt5/QtXml -isystem /usr/include/qt5/QtWebKit -isystem /usr/include/qt5/QtWebKitWidgets -isystem /usr/include/qt5/QtConcurrent -isystem /usr/include/qt5/QtMultimedia -isystem /usr/include/qt5/QtSvg -isystem /usr/include/qt5/QtDBus -isystem /usr/include/qt5/QtX11Extras -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -std=c++11 -Wall -Wextra -DNDEBUG   -fPIC -std=gnu++11 -MD -MT src/libpsi/tools/zip/CMakeFiles/zip.dir/zip.cpp.o -MF src/libpsi/tools/zip/CMakeFiles/zip.dir/zip.cpp.o.d -o src/libpsi/tools/zip/CMakeFiles/zip.dir/zip.cpp.o -c ../src/libpsi/tools/zip/zip.cpp
../src/libpsi/tools/zip/zip.cpp: In member function 'bool UnZip::readFile(const QString&, QByteArray*, int)':
../src/libpsi/tools/zip/zip.cpp:132:69: error: cannot convert 'UnZip::CaseSensitivity' to 'unzFileNameComparer' {aka 'int (*)(void*, const char*, const char*)'}
  int err = unzLocateFile(d->uf, QFile::encodeName(fname).data(), d->caseSensitive);
                                                                  ~~~^~~~~~~~~~~~~
In file included from ../src/libpsi/tools/zip/zip.cpp:25:
/usr/include/mz_compat.h:279:90: note:   initializing argument 3 of 'int unzLocateFile(unzFile, const char*, unzFileNameComparer)'
 extern int ZEXPORT unzLocateFile(unzFile file, const char *filename, unzFileNameComparer filename_compare_func);
                                                                      ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
../src/libpsi/tools/zip/zip.cpp: In member function 'bool UnZip::fileExists(const QString&)':
../src/libpsi/tools/zip/zip.cpp:178:69: error: cannot convert 'UnZip::CaseSensitivity' to 'unzFileNameComparer' {aka 'int (*)(void*, const char*, const char*)'}
  int err = unzLocateFile(d->uf, QFile::encodeName(fname).data(), d->caseSensitive);
                                                                  ~~~^~~~~~~~~~~~~
In file included from ../src/libpsi/tools/zip/zip.cpp:25:
/usr/include/mz_compat.h:279:90: note:   initializing argument 3 of 'int unzLocateFile(unzFile, const char*, unzFileNameComparer)'
 extern int ZEXPORT unzLocateFile(unzFile file, const char *filename, unzFileNameComparer filename_compare_func);
                                                                      ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~

[tehnick]

The contrib module from zlib named minizip is not maintained nowadays, and both original authors of minizip redirected me to the updated minizip fork

Nevertheless, Debian, Ubuntu and some other Linux distributives use [1][2] original library and provide fixes for it.
[1] https://tracker.debian.org/pkg/minizip
[2] https://launchpad.net/ubuntu/+source/minizip

[tehnick]

More over, I have just remembered, that development of official minizip library was moved to zlib project and our embedded copy of this library in Psi is in sync with it.

[tehnick]

Personally I do not see any reason for switching to this fork.

@Ri0n What do you think about this?

[praiskup]

The old minizip won't ever be updated, and we'll e.g. drop that from Fedora in near future.

[praiskup]

(plus, of course security reasons)

[tehnick]

and we'll e.g. drop that from Fedora in near future.

Could you give us a link to related bug report in Fedora BTS?

As far as I see at [1][2] in Fedora 28 maintainers use the same version of minizip as in Debian, but they use the version from zlib library. Just compare with zlib package in Debian [3]...
[1] https://rpmfind.net/linux/rpm2html/search.php?query=minizip
[2] https://pkgs.org/download/minizip
[3] https://tracker.debian.org/pkg/zlib

(plus, of course security reasons)

Which are well handled by Debian Security Team, Ubuntu Security Team, etc..

[praiskup]

Could you give us a link to related bug report in Fedora BTS?

https://bugzilla.redhat.com/show_bug.cgi?id=1609830

[tehnick]

https://github.com/nmoinvaz/minizip

After small search in Internet I have not found packages for main GNU/Linux distros with this fork of library at all.

More over the timestamps of git tags in this repo looks very suspicious:
https://github.com/nmoinvaz/minizip/releases
https://github.com/nmoinvaz/minizip/releases?after=2.3.4
https://github.com/nmoinvaz/minizip/releases?after=2.2.4

[praiskup]

After small search in Internet I have not found packages for main GNU/Linux distros with this fork of library at all.

That's the only maintained fork, and there's basically compatibility with the original code (mz_compat.h).

(plus, of course security reasons)

Which are well handled by Debian Security Team, Ubuntu Security Team, etc..

These are only distro-specific efforts; we have Fedora Security Team too, but at some point it is rather better to move on than depend on abandoned software.

[praiskup]

Btw., how do we maintain security in the bundled version of minzip in psi?

[tehnick]

https://bugzilla.redhat.com/show_bug.cgi?id=1609830

Thanks.

These are only distro-specific efforts; we have Fedora Security Team too, but at some point it is rather better to move on than depend on abandoned software.

Yes, I understand your arguments. But in any case Psi should be compatible with system versions of libraries in most popular GNU/Linux distributives, so proposed patch should be a bit more complicated.

[praiskup]

Of course, sorry ... this was not in any way attempt to propose a patch. It's just to inform you that (a) I did a small research, and that (b) most probably we'll drop the old minizip code from Fedora one day.

[tehnick]

Btw., how do we maintain security in the bundled version of minzip in psi?

Irregularly. IIRC last time it was updated by me and I just used sources from Debian package...

[tehnick]

Irregularly. IIRC last time it was updated by me and I just used sources from Debian package...

psi-im/libpsi@869ee6d
psi-im/libpsi@369310c

[tehnick]

@praiskup zlib-ng/minizip-ng#358 (comment)

[rapgro]

The patch is needed for libpsi, no? There are two packages in Fedora, both psi (psi-im) and psi-plus, it would be nice to have a common dependency to a libpsi (sub-) package.

[tehnick]

@rapgro
Please use URL of this issue in your bug report in Fedora BTS:
https://bugzilla.redhat.com/show_bug.cgi?id=1632194

[rapgro]

@tehnick Done, thanks.

[tehnick]

The patch is needed for libpsi, no?

Yes, but this is a primary bug tracker for Psi and Psi+ projects.

There are two packages in Fedora, both psi (psi-im) and psi-plus, it would be nice to have a common dependency to a libpsi (sub-) package.

Sorry, but this is not possible. Psi and Psi+ have very different releasing cycles. And libpsi as internal library may be changed significantly between rare releases of Psi, while master branch of Psi and frequent Psi+ releases are always in sync.

[tehnick]

@rapgro

Done, thanks.

I still see Comment 10 as a last comment in that thread. And it points to https://github.com/psi-im/libpsi/issues/13

Does your BTS have timeout before adding of new comments?

[rapgro]

Okay. I'd have to accept your development model. Downstream packaging could get done more easily, though.

[rapgro]

Raphael Groner 2019-04-13 22:24:34 CEST
No longer depends On: 1667638
Raphael Groner 2019-04-13 22:12:50 CEST
External Bug ID: Github /issues/388

[Neustradamus]

There is a change since several years?

Note: The current zlib version is 1.3, time to update the code too?

[Ri0n]

it's better to try https://bugreports.qt.io/browse/QTBUG-3897