StreamPeerGzip::finish() fails when compressing with Condition "err != (p_close ? 1 : 0)" is true. Returning: FAILED

[Zoranan]

Tested versions

Reproduced in the following versions

• v4.3.stable.official [77dcf97]
• v4.3.stable.mono.official [77dcf97]

System information

Windows 11 Pro. i5-12600k

Issue description

When attempting to compress some "large" data (around ~6kb json string), calling finish() fails with the following error and only the 10 byte gzip header is returned.

E 0:00:28:0506   global.gd:51 @ Gzip(): Condition "err != (p_close ? 1 : 0)" is true. Returning: FAILED
  <C++ Source>   core/io/stream_peer_gzip.cpp:115 @ _process()

Link to Source

My Code

static func Gzip(data: PackedByteArray, unzip := false) -> PackedByteArray:
	var gzip = StreamPeerGZIP.new()
	var err

	if unzip:
		err = gzip.start_decompression()
	else:
		err = gzip.start_compression()
		
	print("GZIP Start: " + str(err))
	err = gzip.put_data(data)
	print("GZIP Put Data: " + str(err))
	
	if !unzip:
		err = gzip.finish() #TODO: This throws an error when output is too large
		print("GZIP Finish: " + str(err))
	
	# Get all zipped content into one array
	var bytes: PackedByteArray = []
	while gzip.get_available_bytes() > 0:
		var res = gzip.get_partial_data(65535)
		if res[0] != 0:
			push_error("Error processing gzip data")
			return []
		bytes.append_array(res[1])
		
	return bytes

Output When Error Occurs

GZIP Start: 0
GZIP Put Data: 0
GZIP Finish: 1

Steps to reproduce

In my tests, this happens when the compressed output data is larger than 1034 bytes. In my case, this happened when trying to compress anything larger than ~6kb of json string data (with no whitespace)

Simply call the Gzip() func above, passing in data that will compress to anything larger than 1034 bytes

lorem_bad.txt
lorem_bad.txt.gz
lorem_ok.txt
lorem_ok.txt.gz

Minimal reproduction project (MRP)

gzip_mrp.zip

[AThousandShips]

Can confirm, but unsure exactly what the approach here should be and where the limitation lies, if this is an error in implementing things, or if this is a limitation that needs to be documented

As long as this isn't a bug in our implementation the error message should be improved, and this issue should then be tracked in (not adding yet as it isn't clear what exactly is wrong here):

• [TRACKER] Error macros with cryptic messages due to not having proper custom messages defined #42719

[Zoranan]

I noticed that the finish() method calls _process with a hard coded p_dst_size of 1024. The gzip header is 10 bytes, hence the apparent 1034 limit?

I'm not sure if I'm missing something, but shouldn't this pass in buffer.size() instead?

stream_peer_gzip.cpp:203

[Zoranan]

I was able to test this locally, and passing in buffer.size() to the _process method instead of 1024 did fix the issue for me:

Error StreamPeerGZIP::finish() {
	ERR_FAIL_COND_V(!ctx || !compressing, ERR_UNAVAILABLE);
	// Ensure we have enough space in temporary buffer.
	if (buffer.size() < 1024) {
		buffer.resize(1024); // 1024 should be more than enough.
	}
	int consumed = 0;
	int to_write = 0;
	Error err = _process(buffer.ptrw(), buffer.size(), nullptr, 0, consumed, to_write, true); // compress
	if (err != OK) {
		return err;
	}
	int wrote = rb.write(buffer.ptr(), to_write);
	ERR_FAIL_COND_V(wrote != to_write, ERR_OUT_OF_MEMORY);
	return OK;
}