[PATCH 0/3] MR5845: opcservices: Provide memory allocator functions to zlib

[Danyil Blyschak (＠blyss)]

Provide memory allocator functions to zlib so that it is able to allocate memory and then deflate later.
Also, some minor error-handling things.

[PATCH 1/3] opcservices: Provide memory allocator functions to zlib.

[Danyil Blyschak]

From: Danyil Blyschak

---
 dlls/opcservices/compress.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/dlls/opcservices/compress.c b/dlls/opcservices/compress.c
index 37a90716a74..186469a782c 100644
--- a/dlls/opcservices/compress.c
+++ b/dlls/opcservices/compress.c
@@ -184,6 +184,16 @@ void compress_finalize_archive(struct zip_archive *archive)
     free(archive);
 }

+static void *zalloc(void *opaque, unsigned int items, unsigned int size)
+{
+    return malloc(items * size);
+}
+
+static void zfree(void *opaque, void *ptr)
+{
+    free(ptr);
+}
+
 static void compress_write_content(struct zip_archive *archive, IStream *content,
         OPC_COMPRESSION_OPTIONS options, struct data_descriptor *data_desc)
 {
@@ -220,6 +230,8 @@ static void compress_write_content(struct zip_archive *archive, IStream *content
     }

memset(&z_str, 0, sizeof(z_str));
+    z_str.zalloc = zalloc;
+    z_str.zfree = zfree;
     deflateInit2(&z_str, level, Z_DEFLATED, -MAX_WBITS, MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY);

do

[PATCH 2/3] opcservices: Suppress unnecessary zlib deflate warnings.

[Danyil Blyschak]

From: Danyil Blyschak

Zlib documents that negative return values for this function are errors
and positive ones are special but normal events.
---
 dlls/opcservices/compress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/opcservices/compress.c b/dlls/opcservices/compress.c
index 186469a782c..841c674c26c 100644
--- a/dlls/opcservices/compress.c
+++ b/dlls/opcservices/compress.c
@@ -257,7 +257,7 @@ static void compress_write_content(struct zip_archive *archive, IStream *content
             z_str.avail_out = sizeof(archive->output_buffer);
             z_str.next_out = archive->output_buffer;

-            if ((ret = deflate(&z_str, flush)))
+            if ((ret = deflate(&z_str, flush)) < 0)
                 WARN("Failed to deflate, ret %d.\n", ret);
             have = sizeof(archive->output_buffer) - z_str.avail_out;
             compress_write(archive, archive->output_buffer, have);

[PATCH 3/3] opcservices: Check for memory allocation failure before deflating.

[Danyil Blyschak]

From: Danyil Blyschak

---
 dlls/opcservices/compress.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dlls/opcservices/compress.c b/dlls/opcservices/compress.c
index 841c674c26c..f7abbfd21e2 100644
--- a/dlls/opcservices/compress.c
+++ b/dlls/opcservices/compress.c
@@ -202,6 +202,7 @@ static void compress_write_content(struct zip_archive *archive, IStream *content
     LARGE_INTEGER move;
     ULONG num_read;
     HRESULT hr;
+    int init_ret;

data_desc->crc32 = RtlComputeCrc32(0, NULL, 0);
     move.QuadPart = 0;
@@ -232,7 +233,8 @@ static void compress_write_content(struct zip_archive *archive, IStream *content
     memset(&z_str, 0, sizeof(z_str));
     z_str.zalloc = zalloc;
     z_str.zfree = zfree;
-    deflateInit2(&z_str, level, Z_DEFLATED, -MAX_WBITS, MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY);
+    if((init_ret = deflateInit2(&z_str, level, Z_DEFLATED, -MAX_WBITS, MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY)) != Z_OK)
+        WARN("Failed to allocate memory in deflateInit2, ret %d.\n", init_ret);

do
     {

[Nikolay Sivov (＠nsivov)]

What's happening when we don't provide those functions?

[Danyil Blyschak (＠blyss)]

On Thu Jun 13 18:33:48 2024 +0000, Nikolay Sivov wrote:
> What's happening when we don't provide those functions?

If these functions aren't provided, the later call to zlib's deflate() returns an error, and so it's unable to compress the input data and process it later. I discovered this working on a bug where Word was unable to open a doc from a pdf file; in that context, the deflate() failure  meant that wine wasn't able to compress document data and eventually write it to a .docx file, which led to a corrupted .docx file.

[Nikolay Sivov (＠nsivov)]

On Thu Jun 13 18:33:48 2024 +0000, Danyil Blyschak wrote:
> If these functions aren't provided, the later call to zlib's deflate()
> returns an error, and so it's unable to compress the input data and
> process it later. I discovered this working on a bug where Word was
> unable to open a doc from a pdf file; in that context, the deflate()
> failure  meant that wine wasn't able to compress document data and
> eventually write it to a .docx file, which led to a corrupted .docx file.

That's exactly the use case this library was added for initially, and it was able to create archives I think. But I can see now that default allocation functions are disabled with Z_SOLO, and old code we used before that didn't even have defaults.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/opcservices/compress.c:
>  free(archive);
>
> }
>
> +static void *zalloc(void *opaque, unsigned int items, unsigned int size)
> +{
> - return malloc(items * size);
>
> +}
> -
>
> +static void zfree(void *opaque, void *ptr)
> +{
> - free(ptr);
>
> +}

These could be using zlib types I think, uInt and voidpf.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/opcservices/compress.c:
>  memset(&z_str, 0, sizeof(z_str));
>  z_str.zalloc = zalloc;
>  z_str.zfree = zfree;
> - deflateInit2(&z_str, level, Z_DEFLATED, -MAX_WBITS, MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY);
> - if((init_ret = deflateInit2(&z_str, level, Z_DEFLATED, -MAX_WBITS, MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY)) != Z_OK)
> -
>    WARN("Failed to allocate memory in deflateInit2, ret %d.\n", init_ret);

Please add a space after "if". I see no issues otherwise.

[Nikolay Sivov (＠nsivov)]

I wonder how it worked before, I can see it really can't as is. What I recall was happening is that opening PDF was creating in-memory or temporary file for .docx representation so you can edit it in Word. Thanks for looking into this.

[Danyil Blyschak (＠blyss)]

On Thu Jun 13 18:57:21 2024 +0000, Nikolay Sivov wrote:
> These could be using zlib types I think, uInt and voidpf.

Should I change them? I did it this way since that's what `dlls/cabinet/fci.c` does (it similarly provides these functions to zlib).