Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resize compression buffer as needed in decompression [DNET944] #298

Closed
firebird-automations opened this issue Jul 30, 2020 · 8 comments
Closed

Comments

@firebird-automations
Copy link

Submitted by: tonim (tonim)

Jira_subtask_outward DNET948

Any environment, when selecting Compression=true in Connection string, more common selecting packetsize=32000

A big constant decompression buffer size is defined in FirebirdNetwordStream.
const int CompressionBufferSize = 1 * 1024 * 1024;

Any decompression bigger than this size throws an exception in HandleDecompression function.

I provide a tested fix, the buffer will grow dynamically, depending on the uncompressed size.

            // There is no need to define a big buffer size, it will grow as needed
	const int CompressionBufferSize = 32000;



	int HandleDecompression\(byte\[\] buffer, int count\)
	\{
		\_decompressor\.OutputBuffer = \_compressionBuffer;
		\_decompressor\.InputBuffer = buffer;
		\_decompressor\.NextOut = 0;
		\_decompressor\.NextIn = 0;
		\_decompressor\.AvailableBytesIn = count;
		do
		\{
			// Double the buffer size until the decompression fits in the output buffer
			\_decompressor\.OutputBuffer = \_compressionBuffer;
			\_decompressor\.AvailableBytesOut = \_compressionBuffer\.Length \- \_decompressor\.NextOut;
			var rc = \_decompressor\.Inflate\(Ionic\.Zlib\.FlushType\.None\);
			if \(rc \!= Ionic\.Zlib\.ZlibConstants\.Z\_OK\)
				throw new IOException\($"Error '\{rc\}' while decompressing the data\."\);
			if \(\_decompressor\.AvailableBytesIn \!= 0\)
			\{
				byte\[\] newCompressionBuffer = new byte\[\_compressionBuffer\.Length \* 2\];
				Array\.Copy\(\_compressionBuffer, newCompressionBuffer, \_decompressor\.NextOut\);
				\_compressionBuffer = newCompressionBuffer;
			\}
		\} while \(\_decompressor\.AvailableBytesIn \!= 0\);
		return \_decompressor\.NextOut;
	\}

Commits: c1d674c

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Do I understand correctly that you overrun the default buffer size when simply selecting from SELECT * FROM RDB$PROCEDURES?

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: Open [ 1 ] => In Progress [ 3 ]

@firebird-automations
Copy link
Author

Commented by: tonim (tonim)

Not exactly, I also altered some connection string parameters like packet size set to 32000.

Anyway you can't expect variable decompressed data will allways fit in a fixed buffer, so in my opinion the current implementation of HandleDecompresion is wrong. I think there is not a maximum compression ratio (only typical compression ratios for some types of contents).

You can reproduce the bug easily in any database by setting a default decompressed buffer size of 8000 for example (and of course Compression flag in connection string)

If you need a sample project including a database reproducing the bug with the 1 megabyte buffer size I will provide you.

Thanks.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

> You can reproduce the bug easily in any database by setting a default decompressed buffer size of 8000 for example (and of course Compression flag in connection string)

Sure. One can make the buffer 1 byte and it will fail.

> If you need a sample project including a database reproducing the bug with the 1 megabyte buffer size I will provide you.

Yeah, that would be interesting to see.

@firebird-automations
Copy link
Author

Commented by: tonim (tonim)

Here is a link with a very simple proyect doing a connection and a reading data with DataReader, reproducing the bug "decompression buffer too small".
It also contains the database (metadata only), copy the database to a path and change the connection string updating the path.
It uses the latest nuget package of Firebird Client.

https://www.dropbox.com/s/e8k3i0tymjq2bo7/repos.zip?dl=0

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Thanks. I changed the code to resize the buffer as needed (just using Array.Resize for easier/better code). Now I think I have to handle the compression as well, because with smaller buffer it's easier to not have enough to compress complete `buffer`.

@firebird-automations
Copy link
Author

Modified by: @cincuranet

Version: 7.5.0.0 [ 10915 ]

Fix Version: vNext [ 10920 ]

summary: Compression buffer too small error thrown => Resize compression buffer as needed in decompression

Version: vNext [ 10920 ] =>

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: In Progress [ 3 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants