Bug 163818 - Opening ODG file is reported to be corrupted when created by 3rd party software (minizip-ng)
Summary: Opening ODG file is reported to be corrupted when created by 3rd party softwa...
Status: RESOLVED NOTOURBUG
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: filters and storage (show other bugs)
Version:
(earliest affected)
24.8.2.1 release
Hardware: x86-64 (AMD64) Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:25.2.0 target:24.8.4
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-08 09:28 UTC by aptitude
Modified: 2024-11-20 10:39 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
The file that is reported to be bad (10.92 KB, application/vnd.oasis.opendocument.graphics)
2024-11-08 09:28 UTC, aptitude
Details
draft minizip-ng patch (1.50 KB, patch)
2024-11-20 10:39 UTC, Michael Stahl (allotropia)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description aptitude 2024-11-08 09:28:50 UTC
Created attachment 197492 [details]
The file that is reported to be bad

Note, This appears related to Bug 163364 - Problem with opening ODS file generated by third party software. 

Since the introduction of the ZipPackage::checkZipEntriesWithDD function, the ODG file that I create using minizip-ng fails to load with the message "corrupt and therefore cannot be opened. LibreOffice can try to repair the file." (See attached) . It loads when I accept to repair.

I use the minizip-ng library in my C++ application to recreate a ODG file from a template. Debugging minizip-ng, I cannot identify any faults with the 3rd party library.

Why was ZipPackage::checkZipEntriesWithDD introduced? If this function is removed, the ODG file loads loaded as expected, without warnings (as tested by compiling LibreOffice on Linux)

See github.com/zlib-ng/minizip-ng/issues/817
Comment 1 Julien Nabet 2024-11-08 12:15:53 UTC
tdf#163364 has been fixed from 24.8.3

Could you give a try to LO 24.8.3?
You can either download the prerelease here:
https://dev-builds.libreoffice.org/daily/libreoffice-24-8/Win-x86_64@tb73-TDF/2024-10-26_22.33.44/
or wait for some days (see https://wiki.documentfoundation.org/ReleasePlan/24.8)
Comment 2 Julien Nabet 2024-11-08 19:15:25 UTC
Noticing Michael's patch in gerrit here: https://gerrit.libreoffice.org/c/core/+/176289
I suppose we can confirm the bug.
Comment 3 aptitude 2024-11-08 19:42:38 UTC
The problem still exists with " development build v24.8 - 2024-11-06_09.10.55 " on Windows

Yesterday, I compiled LibreOffice from source, to identify the location of the problem. In package/source/zippackage/ZipPackage.cxx . See ZipPackage::checkZipEntriesWithDD

Although Bug 163364 is marked as fixed. It only seems to allow recovery. This breaks existing libraries.

Researching the issue. I believe the issue is with minizip-ng handling of attributes differs from LibreOffice. A file created by LibreOffice zip external file attribute is not present for minizip-ng to read . So when minizip repacks it from RAM, it is stored as a file. (However, I still couldn't get it to work attempting to force minizip to deduce it from the filename ending in a slash. This maybe a red herring)

Note, I had not tested the legacy "minizip" library.
Comment 4 aptitude 2024-11-08 19:47:26 UTC
(In reply to Julien Nabet from comment #2)
> Noticing Michael's patch in gerrit here:
> https://gerrit.libreoffice.org/c/core/+/176289
> I suppose we can confirm the bug.

Thanks. I will have a look when back at work on Monday.
Comment 5 aptitude 2024-11-11 08:40:26 UTC
(In reply to Julien Nabet from comment #2)
> Noticing Michael's patch in gerrit here:
> https://gerrit.libreoffice.org/c/core/+/176289
> I suppose we can confirm the bug.

This patch unfortunately does not fix the issue. 

ZipFile::checkSizeAndCRC is called after ZipPackage::checkZipEntriesWithDD
Comment 6 Julien Nabet 2024-11-11 12:05:37 UTC
On pc Debian x86-64 with master sources updated today (afdf42181d0e25afbce7e3a9e958fdde67c2ce7e), I confirm LO proposes to repair it.

Michael: thought you might be interested in this one since it concerns zip management.
Comment 7 Michael Stahl (allotropia) 2024-11-11 14:17:16 UTC
there are 2 problems with this file:
1. when opening normally, it trips the check that was added for STORED zip entries with a data descriptor
2. when opening in recovery mode, the compressed size of one zip entry being 0 caused an exception and that aborted the recovery

regarding problem 1, i've been suggesting to "minizip-ng" maintainer to avoid such zip entries; currently i'm not convinced if LO should open such files.

regarding problem 2, this is of course a real issue in LO, and was fixed by my commit today, which somehow the bot didn't add to this issue, so let me try to do it here:

It has been pushed to "master":

https://git.libreoffice.org/core/commit/80cda6954adc88eac3b99171acafea004976915b

tdf#163818 package: fix recovery of zip entry local header with ...

It will be available in 25.2.0.
Comment 8 aptitude 2024-11-12 12:27:11 UTC
I am still investigating the fault in minizip-ng

In the meantime, I have a workaround. Instead of using 

mz_zip_writer_copy_from_reader()

Use

mz_zip_reader_entry_open(), mz_zip_reader_entry_read(), 
mz_zip_writer_entry_open(with basic mz_zip_file struct entries, instead of using mz_zip_reader_entry_get_info)
mz_zip_writer_entry_write(), mz_zip_writer_entry_close()

This now opens up correctly in LibreOffice.

It looks like the issue is with minizip

Thanks for your assistance.
Comment 9 Commit Notification 2024-11-16 07:22:11 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/68688503dfb67506dc41f3537e156d12f856ecb8

tdf#163818 package: fix recovery of zip entry local header with ...

It will be available in 24.8.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 aptitude 2024-11-20 09:11:19 UTC
Fixed the issue on my side.

When recreating LibreOffice zips using minizip-ng

After opening the zip file (mz_zip_writer_open_file), set an internal minizip flag:

	void* writer_zip_handle = nullptr;
	err = mz_zip_writer_get_zip_handle(zip_writer, &writer_zip_handle);
	if (err == MZ_OK)
		mz_zip_set_data_descriptor(writer_zip_handle, 0);

This sets the use of data descriptor flag when writing zip entries

mz_zip_writer_copy_from_reader() now works as expected with LibreOffice files.

I don't know why this is required.
Comment 11 Michael Stahl (allotropia) 2024-11-20 10:39:10 UTC
Created attachment 197697 [details]
draft minizip-ng patch

just for the record, this is the patch i sent to minizip-ng maintainer about the data_descriptor flag; didn't receive positive feedback about it yet.