Skip to content

Commit

Permalink
Fix StreamPeerGZIP::finish() internal buffer size usage
Browse files Browse the repository at this point in the history
Fixes godotengine#97201

Instead of using and arbitrary fixed size for the internal buffer,
the remaining available bytes of the internal `RingBuffer` is used.

Also add unit tests for `StreamPeerGZIP`
  • Loading branch information
pafuent committed Dec 15, 2024
1 parent b9437c3 commit ce4a6fe
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 4 deletions.
6 changes: 3 additions & 3 deletions core/io/stream_peer_gzip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,12 @@ int StreamPeerGZIP::get_available_bytes() const {
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.
if (buffer.size() < get_available_bytes()) {
buffer.resize(get_available_bytes()); // get_available_bytes() is what we can store in RingBuffer.
}
int consumed = 0;
int to_write = 0;
Error err = _process(buffer.ptrw(), 1024, nullptr, 0, consumed, to_write, true); // compress
Error err = _process(buffer.ptrw(), buffer.size(), nullptr, 0, consumed, to_write, true); // compress
if (err != OK) {
return err;
}
Expand Down
3 changes: 2 additions & 1 deletion doc/classes/StreamPeerGZIP.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
<method name="finish">
<return type="int" enum="Error" />
<description>
Finalizes the stream, compressing or decompressing any buffered chunk left.
Finalizes the stream, compressing any buffered chunk left.
You must call it only when you are compressing.
</description>
</method>
<method name="start_compression">
Expand Down
198 changes: 198 additions & 0 deletions tests/core/io/test_stream_peer_gzip.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/**************************************************************************/
/* test_stream_peer_gzip.h */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#ifndef TEST_STREAM_PEER_GZIP_H
#define TEST_STREAM_PEER_GZIP_H

#include "core/io/stream_peer_gzip.h"
#include "tests/test_macros.h"

namespace TestStreamPeerGZIP {

const String hello = "Hello World!!!";

TEST_CASE("[StreamPeerGZIP] Initialization") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();
CHECK_EQ(spgz->get_available_bytes(), 0);
}

TEST_CASE("[StreamPeerGZIP] Compress/Decompress") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();

bool is_deflate = false;

SUBCASE("GZIP") {
is_deflate = false;
}

SUBCASE("DEFLATE") {
is_deflate = true;
}

CHECK_EQ(spgz->start_compression(is_deflate), Error::OK);
CHECK_EQ(spgz->put_data(hello.to_ascii_buffer().ptr(), hello.to_ascii_buffer().size()), Error::OK);
CHECK_EQ(spgz->finish(), Error::OK);

Vector<uint8_t> hello_compressed;
int hello_compressed_size = spgz->get_available_bytes();
hello_compressed.resize(hello_compressed_size);
CHECK_EQ(spgz->get_data(hello_compressed.ptrw(), hello_compressed_size), Error::OK);

spgz->clear();

CHECK_EQ(spgz->start_decompression(is_deflate), Error::OK);
CHECK_EQ(spgz->put_data(hello_compressed.ptr(), hello_compressed.size()), Error::OK);

Vector<uint8_t> hello_decompressed;
int hello_decompressed_size = spgz->get_available_bytes();
hello_decompressed.resize(hello_decompressed_size);
CHECK_EQ(spgz->get_data(hello_decompressed.ptrw(), hello_decompressed_size), Error::OK);
CHECK_EQ(hello_decompressed, hello.to_ascii_buffer());
}

TEST_CASE("[StreamPeerGZIP] Compress/Decompress big chunks of data") { // GH-97201
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();
CHECK_EQ(spgz->start_compression(false), Error::OK);

Vector<uint8_t> big_data;
big_data.resize(2500);
// Filling it with random data because the issue is related to the size of the data when it's compress.
// Random data results in bigger compressed data size.
for (int i = 0; i < big_data.size(); i++) {
big_data.write[i] = Math::random(48, 122);
}
CHECK_EQ(spgz->put_data(big_data.ptr(), big_data.size()), Error::OK);
CHECK_EQ(spgz->finish(), Error::OK);

Vector<uint8_t> big_data_compressed;
int big_data_compressed_size = spgz->get_available_bytes();
big_data_compressed.resize(big_data_compressed_size);
CHECK_EQ(spgz->get_data(big_data_compressed.ptrw(), big_data_compressed_size), Error::OK);

spgz->clear();

CHECK_EQ(spgz->start_decompression(false), Error::OK);
CHECK_EQ(spgz->put_data(big_data_compressed.ptr(), big_data_compressed.size()), Error::OK);

Vector<uint8_t> big_data_decompressed;
int big_data_decompressed_size = spgz->get_available_bytes();
big_data_decompressed.resize(big_data_decompressed_size);
CHECK_EQ(spgz->get_data(big_data_decompressed.ptrw(), big_data_decompressed_size), Error::OK);
CHECK_EQ(big_data_decompressed, big_data);
}

TEST_CASE("[StreamPeerGZIP] Can't start twice") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();
CHECK_EQ(spgz->start_compression(false), Error::OK);

ERR_PRINT_OFF;
CHECK_EQ(spgz->start_compression(false), Error::ERR_ALREADY_IN_USE);
CHECK_EQ(spgz->start_decompression(false), Error::ERR_ALREADY_IN_USE);
ERR_PRINT_ON;
}

TEST_CASE("[StreamPeerGZIP] Can't start with a buffer size equal or less than zero") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();

ERR_PRINT_OFF;
CHECK_EQ(spgz->start_compression(false, 0), Error::ERR_INVALID_PARAMETER);
CHECK_EQ(spgz->start_compression(false, -1), Error::ERR_INVALID_PARAMETER);
CHECK_EQ(spgz->start_decompression(false, 0), Error::ERR_INVALID_PARAMETER);
CHECK_EQ(spgz->start_decompression(false, -1), Error::ERR_INVALID_PARAMETER);
ERR_PRINT_ON;
}

TEST_CASE("[StreamPeerGZIP] Can't put/get data with a buffer size less than zero") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();
CHECK_EQ(spgz->start_compression(false), Error::OK);

ERR_PRINT_OFF;
CHECK_EQ(spgz->put_data(hello.to_ascii_buffer().ptr(), -1), Error::ERR_INVALID_PARAMETER);

Vector<uint8_t> hello_compressed;
hello_compressed.resize(5);
CHECK_EQ(spgz->get_data(hello_compressed.ptrw(), -1), Error::ERR_INVALID_PARAMETER);
ERR_PRINT_ON;
}

TEST_CASE("[StreamPeerGZIP] Needs to be started before use") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();

ERR_PRINT_OFF;
CHECK_EQ(spgz->put_data(hello.to_ascii_buffer().ptr(), hello.to_ascii_buffer().size()), Error::ERR_UNCONFIGURED);
ERR_PRINT_ON;
}

TEST_CASE("[StreamPeerGZIP] Can't be finished after clear or if it's decompressing") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();

CHECK_EQ(spgz->start_compression(false), Error::OK);
spgz->clear();
ERR_PRINT_OFF;
CHECK_EQ(spgz->finish(), Error::ERR_UNAVAILABLE);
ERR_PRINT_ON;

spgz->clear();
CHECK_EQ(spgz->start_decompression(false), Error::OK);
ERR_PRINT_OFF;
CHECK_EQ(spgz->finish(), Error::ERR_UNAVAILABLE);
ERR_PRINT_ON;
}

TEST_CASE("[StreamPeerGZIP] Fails to get if nothing was compress/decompress") {
Ref<StreamPeerGZIP> spgz;
spgz.instantiate();

SUBCASE("Compression") {
CHECK_EQ(spgz->start_compression(false), Error::OK);
}

SUBCASE("Decompression") {
CHECK_EQ(spgz->start_decompression(false), Error::OK);
}

ERR_PRINT_OFF;
Vector<uint8_t> hello_compressed;
hello_compressed.resize(5);
CHECK_EQ(spgz->get_data(hello_compressed.ptrw(), hello_compressed.size()), Error::ERR_UNAVAILABLE);
ERR_PRINT_ON;
}

} // namespace TestStreamPeerGZIP

#endif // TEST_STREAM_PEER_GZIP_H
1 change: 1 addition & 0 deletions tests/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "tests/core/io/test_resource.h"
#include "tests/core/io/test_stream_peer.h"
#include "tests/core/io/test_stream_peer_buffer.h"
#include "tests/core/io/test_stream_peer_gzip.h"
#include "tests/core/io/test_tcp_server.h"
#include "tests/core/io/test_udp_server.h"
#include "tests/core/io/test_xml_parser.h"
Expand Down

0 comments on commit ce4a6fe

Please sign in to comment.