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

validate max_allowed_packet on the client side #84

Merged
merged 4 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion contrib/ruby/ext/trilogy-ruby/cext.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ static ID id_socket, id_host, id_port, id_username, id_password, id_found_rows,
id_ivar_affected_rows, id_ivar_fields, id_ivar_last_insert_id, id_ivar_rows, id_ivar_query_time, id_password,
id_database, id_ssl_ca, id_ssl_capath, id_ssl_cert, id_ssl_cipher, id_ssl_crl, id_ssl_crlpath, id_ssl_key,
id_ssl_mode, id_tls_ciphersuites, id_tls_min_version, id_tls_max_version, id_multi_statement, id_multi_result,
id_from_code, id_from_errno, id_connection_options;
id_from_code, id_from_errno, id_connection_options, id_max_allowed_packet;

struct trilogy_ctx {
trilogy_conn_t conn;
Expand Down Expand Up @@ -415,6 +415,11 @@ static VALUE rb_trilogy_initialize(VALUE self, VALUE encoding, VALUE charset, VA
connopt.keepalive_interval = NUM2USHORT(val);
}

if ((val = rb_hash_lookup(opts, ID2SYM(id_max_allowed_packet))) != Qnil) {
Check_Type(val, T_FIXNUM);
connopt.max_allowed_packet = NUM2SIZET(val);
}

if ((val = rb_hash_lookup(opts, ID2SYM(id_host))) != Qnil) {
Check_Type(val, T_STRING);

Expand Down Expand Up @@ -1114,6 +1119,7 @@ RUBY_FUNC_EXPORTED void Init_cext()
id_connect_timeout = rb_intern("connect_timeout");
id_read_timeout = rb_intern("read_timeout");
id_write_timeout = rb_intern("write_timeout");
id_max_allowed_packet = rb_intern("max_allowed_packet");
id_keepalive_enabled = rb_intern("keepalive_enabled");
id_keepalive_idle = rb_intern("keepalive_idle");
id_keepalive_count = rb_intern("keepalive_count");
Expand Down
108 changes: 98 additions & 10 deletions contrib/ruby/test/client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -745,28 +745,116 @@ def test_connect_by_multiple_names
Trilogy.new(host: "localhost")
end

PADDED_QUERY_TEMPLATE = "SELECT LENGTH('%s')"
PROTOCOL_OVERHEAD = 2 # One byte for the 0x03 (COM_QUERY); one because the packet is actually required to be shorter than the "max"
PADDED_QUERY_OVERHEAD =
PADDED_QUERY_TEMPLATE.size - "%s".size + PROTOCOL_OVERHEAD

def query_for_target_packet_size(size)
PADDED_QUERY_TEMPLATE % ("x" * (size - PADDED_QUERY_OVERHEAD))
end

def set_max_allowed_packet(size)
client = new_tcp_client
client.query "SET GLOBAL max_allowed_packet = #{size}"
ensure
ensure_closed client
end

def test_packet_size
def test_packet_size_lower_than_trilogy_max_packet_len
set_max_allowed_packet(4 * 1024 * 1024) # TRILOGY_MAX_PACKET_LEN is 16MB

client = new_tcp_client(max_allowed_packet: 4 * 1024 * 1024)

assert client.query query_for_target_packet_size(1 * 1024 * 1024)

assert client.query query_for_target_packet_size(2 * 1024 * 1024)

assert client.query query_for_target_packet_size(4 * 1024 * 1024)

exception = assert_raises Trilogy::QueryError do
client.query query_for_target_packet_size(4 * 1024 * 1024 + 1)
end

assert_equal "trilogy_query_send: TRILOGY_MAX_PACKET_EXCEEDED", exception.message
ensure
ensure_closed client
end

def test_packet_size_greater_than_trilogy_max_packet_len
set_max_allowed_packet(32 * 1024 * 1024) # TRILOGY_MAX_PACKET_LEN is 16MB

client = new_tcp_client(max_allowed_packet: 32 * 1024 * 1024)

assert client.query query_for_target_packet_size(16 * 1024 * 1024)

assert client.query query_for_target_packet_size(32 * 1024 * 1024)

exception = assert_raises Trilogy::QueryError do
client.query query_for_target_packet_size(32 * 1024 * 1024 + 1)
end

assert_equal "trilogy_query_send: TRILOGY_MAX_PACKET_EXCEEDED", exception.message
ensure
ensure_closed client
end

def test_configured_max_packet_below_server
set_max_allowed_packet(32 * 1024 * 1024)

client = new_tcp_client
client = new_tcp_client(max_allowed_packet: 24 * 1024 * 1024)

create_test_table(client)
client.query "TRUNCATE trilogy_test"
assert client.query query_for_target_packet_size(16 * 1024 * 1024)

result = client.query "INSERT INTO trilogy_test (blob_test) VALUES ('#{"x" * (15 * 1024 * 1024)}')"
assert result
assert_equal 1, client.last_insert_id
assert client.query query_for_target_packet_size(24 * 1024 * 1024)

result = client.query "INSERT INTO trilogy_test (blob_test) VALUES ('#{"x" * (31 * 1024 * 1024)}')"
assert result
assert_equal 2, client.last_insert_id
exception = assert_raises Trilogy::QueryError do
client.query query_for_target_packet_size(24 * 1024 * 1024 + 1)
end

assert_equal "trilogy_query_send: TRILOGY_MAX_PACKET_EXCEEDED", exception.message
ensure
ensure_closed client
end

def test_configured_max_packet_above_server
set_max_allowed_packet(24 * 1024 * 1024)

client = new_tcp_client(max_allowed_packet: 32 * 1024 * 1024)

assert client.query query_for_target_packet_size(16 * 1024 * 1024)

assert client.query query_for_target_packet_size(24 * 1024 * 1024)

exception = assert_raises Trilogy::QueryError do
client.query query_for_target_packet_size(32 * 1024 * 1024 + 1)
end

assert_equal "trilogy_query_send: TRILOGY_MAX_PACKET_EXCEEDED", exception.message

exception = assert_raises Trilogy::QueryError do
client.query query_for_target_packet_size(24 * 1024 * 1024 + 1)
end

refute_match(/TRILOGY_MAX_PACKET_EXCEEDED/, exception.message)
ensure
ensure_closed client
end

def test_absolute_maximum_packet_size
skip unless ENV["CI"]

set_max_allowed_packet(1024 * 1024 * 1024) # 1GB is the highest maximum allowed

client = new_tcp_client(max_allowed_packet: 1024 * 1024 * 1024)

assert client.query query_for_target_packet_size(1024 * 1024 * 1024)

exception = assert_raises Trilogy::QueryError do
client.query query_for_target_packet_size(1024 * 1024 * 1024 + 1)
end

assert_equal "trilogy_query_send: TRILOGY_MAX_PACKET_EXCEEDED", exception.message
ensure
ensure_closed client
end
Expand Down
34 changes: 34 additions & 0 deletions inc/trilogy/builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
typedef struct {
trilogy_buffer_t *buffer;
size_t header_offset;
size_t packet_length;
size_t packet_max_length;
uint32_t fragment_length;
uint8_t seq;
} trilogy_builder_t;
Expand Down Expand Up @@ -54,6 +56,8 @@ void trilogy_builder_finalize(trilogy_builder_t *builder);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_uint8(trilogy_builder_t *builder, uint8_t val);

Expand All @@ -66,6 +70,8 @@ int trilogy_builder_write_uint8(trilogy_builder_t *builder, uint8_t val);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_uint16(trilogy_builder_t *builder, uint16_t val);

Expand All @@ -78,6 +84,8 @@ int trilogy_builder_write_uint16(trilogy_builder_t *builder, uint16_t val);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_uint24(trilogy_builder_t *builder, uint32_t val);

Expand All @@ -90,6 +98,8 @@ int trilogy_builder_write_uint24(trilogy_builder_t *builder, uint32_t val);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_uint32(trilogy_builder_t *builder, uint32_t val);

Expand All @@ -102,6 +112,8 @@ int trilogy_builder_write_uint32(trilogy_builder_t *builder, uint32_t val);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_uint64(trilogy_builder_t *builder, uint64_t val);

Expand All @@ -117,6 +129,8 @@ int trilogy_builder_write_uint64(trilogy_builder_t *builder, uint64_t val);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_lenenc(trilogy_builder_t *builder, uint64_t val);

Expand All @@ -130,6 +144,8 @@ int trilogy_builder_write_lenenc(trilogy_builder_t *builder, uint64_t val);
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_buffer(trilogy_builder_t *builder, const void *data, size_t len);

Expand All @@ -144,6 +160,8 @@ int trilogy_builder_write_buffer(trilogy_builder_t *builder, const void *data, s
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_lenenc_buffer(trilogy_builder_t *builder, const void *data, size_t len);

Expand All @@ -155,7 +173,23 @@ int trilogy_builder_write_lenenc_buffer(trilogy_builder_t *builder, const void *
* Return values:
* TRILOGY_OK - The value was appended to the packet buffer.
* TRILOGY_SYSERR - A system error occurred, check errno.
* TRILOGY_MAX_PACKET_EXCEEDED - Appending this value would exceed the maximum
* packet size.
*/
int trilogy_builder_write_string(trilogy_builder_t *builder, const char *data);

/* trilogy_builder_set_max_packet_length - Set the maximum packet length for
* the builder. Writing data to the builder that would cause the packet length
* to exceed this value will cause the builder to error.
*
* builder - A pre-initialized trilogy_builder_t pointer
* max_length - The new maximum packet length to set
*
* Return values:
* TRILOGY_OK - The maximum packet length was set.
* TRILOGY_MAX_PACKET_EXCEEDED - The current packet length is already
* larger than the requested maximum.
*/
int trilogy_builder_set_max_packet_length(trilogy_builder_t *builder, size_t max_length);

#endif
3 changes: 2 additions & 1 deletion inc/trilogy/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
XX(TRILOGY_OPENSSL_ERR, -16) /* check ERR_get_error() */ \
XX(TRILOGY_UNSUPPORTED, -17) \
XX(TRILOGY_DNS_ERR, -18) \
XX(TRILOGY_AUTH_SWITCH, -19)
XX(TRILOGY_AUTH_SWITCH, -19) \
XX(TRILOGY_MAX_PACKET_EXCEEDED, -20)

enum {
#define XX(name, code) name = code,
Expand Down
2 changes: 2 additions & 0 deletions inc/trilogy/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ typedef struct {
uint16_t keepalive_interval;

TRILOGY_CAPABILITIES_t flags;

size_t max_allowed_packet;
} trilogy_sockopt_t;

typedef struct trilogy_sock_t {
Expand Down
25 changes: 25 additions & 0 deletions src/builder.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ int trilogy_builder_init(trilogy_builder_t *builder, trilogy_buffer_t *buff, uin
builder->buffer->len = 0;

builder->seq = seq;
builder->packet_length = 0;
builder->packet_max_length = SIZE_MAX;

return write_header(builder);
}
Expand All @@ -60,10 +62,15 @@ void trilogy_builder_finalize(trilogy_builder_t *builder)

int trilogy_builder_write_uint8(trilogy_builder_t *builder, uint8_t val)
{
if (builder->packet_length >= builder->packet_max_length - 1) {
return TRILOGY_MAX_PACKET_EXCEEDED;
}

CHECKED(trilogy_buffer_expand(builder->buffer, 1));

builder->buffer->buff[builder->buffer->len++] = val;
builder->fragment_length++;
builder->packet_length++;

if (builder->fragment_length == TRILOGY_MAX_PACKET_LEN) {
CHECKED(write_continuation_header(builder));
Expand Down Expand Up @@ -137,6 +144,10 @@ int trilogy_builder_write_buffer(trilogy_builder_t *builder, const void *data, s

size_t fragment_remaining = TRILOGY_MAX_PACKET_LEN - builder->fragment_length;

if (builder->packet_length >= builder->packet_max_length - len) {
return TRILOGY_MAX_PACKET_EXCEEDED;
}

// if this buffer write is not going to straddle a fragment boundary:
if (len < fragment_remaining) {
CHECKED(trilogy_buffer_expand(builder->buffer, len));
Expand All @@ -145,6 +156,7 @@ int trilogy_builder_write_buffer(trilogy_builder_t *builder, const void *data, s

builder->buffer->len += len;
builder->fragment_length += len;
builder->packet_length += len;

return TRILOGY_OK;
}
Expand All @@ -157,6 +169,7 @@ int trilogy_builder_write_buffer(trilogy_builder_t *builder, const void *data, s

builder->buffer->len += fragment_remaining;
builder->fragment_length += fragment_remaining;
builder->packet_length += fragment_remaining;

ptr += fragment_remaining;
len -= fragment_remaining;
Expand All @@ -172,6 +185,7 @@ int trilogy_builder_write_buffer(trilogy_builder_t *builder, const void *data, s

builder->buffer->len += len;
builder->fragment_length += len;
builder->packet_length += len;
}

return TRILOGY_OK;
Expand All @@ -195,4 +209,15 @@ int trilogy_builder_write_string(trilogy_builder_t *builder, const char *data)
return TRILOGY_OK;
}

int trilogy_builder_set_max_packet_length(trilogy_builder_t *builder, size_t max_length)
{
if (builder->packet_length > max_length) {
return TRILOGY_MAX_PACKET_EXCEEDED;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch here 👍


builder->packet_max_length = max_length;

return TRILOGY_OK;
}

#undef CHECKED
4 changes: 4 additions & 0 deletions src/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ static int begin_command_phase(trilogy_builder_t *builder, trilogy_conn_t *conn,
return rc;
}

if (conn->socket->opts.max_allowed_packet > 0) {
trilogy_builder_set_max_packet_length(builder, conn->socket->opts.max_allowed_packet);
}

conn->packet_parser.sequence_number = seq + 1;

return 0;
Expand Down
Loading