Skip to content

Commit

Permalink
Don't confuse length encoded int with EOF
Browse files Browse the repository at this point in the history
Prior to this commit a row starting with a field with length 2 ** 24 or
higher would get interpreted as an EOF and we'd fail with:

```
Trilogy::QueryError: trilogy_read_row: TRILOGY_EXTRA_DATA_IN_PACKET
```

rows are sent as a series of length encoded strings. These are strings
prefixed by a length encoded integer, where 0xFE is the prefix byte for
an 8 byte integer.

After all the rows are sent, we get a OK/EOF packet beginning with 0xFE.

The way to tell the OK/EOF apart is by checking that the length of the
whole payload is < 9 (i.e. there isn't enough room for an 8-byte int).

We were doing the `< 9` check for the deprecated EOF packets, but not
for the newer OK packets.
  • Loading branch information
composerinteralia committed Apr 9, 2024
1 parent f199c58 commit e322daa
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 22 deletions.
17 changes: 17 additions & 0 deletions contrib/ruby/test/client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1122,4 +1122,21 @@ def test_error_classes_exclusively_match_subclasses
assert_operator SystemCallError, :===, klass.new
assert_operator Trilogy::ConnectionError, :===, klass.new
end

def test_reads_row_not_eof
# We're going to write a big packet, so ensure we are allowed to send it
set_max_allowed_packet(32 * 1024 * 1024)
client = new_tcp_client
create_test_table(client)

# Write a value whose length will require an 8-byte length-encoded int
value = "x" * 2 ** 24
client.query("INSERT INTO `trilogy_test` (`long_text_test`) VALUES (\"#{value}\")")

# Ensure we read the row and don't confuse it with an EOF packet
result = client.query("SELECT `long_text_test` FROM `trilogy_test` LIMIT 1")
assert_equal value.length, result.rows[0][0].length
ensure
ensure_closed client
end
end
47 changes: 25 additions & 22 deletions src/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ static int read_err_packet(trilogy_conn_t *conn)
return TRILOGY_ERR;
}

static int read_eof_packet(trilogy_conn_t *conn)
static int read_deprecated_eof_packet(trilogy_conn_t *conn)
{
trilogy_eof_packet_t eof_packet;

Expand All @@ -236,6 +236,21 @@ static int read_eof_packet(trilogy_conn_t *conn)
return TRILOGY_EOF;
}

static int read_eof_packet(trilogy_conn_t *conn)
{
int rc;

if (conn->capabilities & TRILOGY_CAPABILITIES_DEPRECATE_EOF) {
return read_ok_packet(conn);
} else {
if ((rc = read_deprecated_eof_packet(conn)) != TRILOGY_EOF) {
return rc;
}

return TRILOGY_OK;
}
}

static int read_auth_switch_packet(trilogy_conn_t *conn, trilogy_handshake_t *handshake)
{
trilogy_auth_switch_request_packet_t auth_switch_packet;
Expand Down Expand Up @@ -647,15 +662,7 @@ static int read_eof(trilogy_conn_t *conn)
return rc;
}

if (conn->capabilities & TRILOGY_CAPABILITIES_DEPRECATE_EOF) {
return read_ok_packet(conn);
} else {
if ((rc = read_eof_packet(conn)) != TRILOGY_EOF) {
return rc;
}

return TRILOGY_OK;
}
return read_eof_packet(conn);
}

int trilogy_read_row(trilogy_conn_t *conn, trilogy_value_t *values_out)
Expand All @@ -680,14 +687,12 @@ int trilogy_read_row(trilogy_conn_t *conn, trilogy_value_t *values_out)
return rc;
}

if (conn->capabilities & TRILOGY_CAPABILITIES_DEPRECATE_EOF && current_packet_type(conn) == TRILOGY_PACKET_EOF) {
if ((rc = read_ok_packet(conn)) != TRILOGY_OK) {
if (current_packet_type(conn) == TRILOGY_PACKET_EOF && conn->packet_buffer.len < 9) {
if ((rc = read_eof_packet(conn)) != TRILOGY_OK) {
return rc;
}

return TRILOGY_EOF;
} else if (current_packet_type(conn) == TRILOGY_PACKET_EOF && conn->packet_buffer.len < 9) {
return read_eof_packet(conn);
} else if (current_packet_type(conn) == TRILOGY_PACKET_ERR) {
return read_err_packet(conn);
} else {
Expand Down Expand Up @@ -941,20 +946,18 @@ int trilogy_stmt_bind_data_send(trilogy_conn_t *conn, trilogy_stmt_t *stmt, uint
int trilogy_stmt_read_row(trilogy_conn_t *conn, trilogy_stmt_t *stmt, trilogy_column_packet_t *columns,
trilogy_binary_value_t *values_out)
{
int err = read_packet(conn);
int rc = read_packet(conn);

if (err < 0) {
return err;
if (rc < 0) {
return rc;
}

if (conn->capabilities & TRILOGY_CAPABILITIES_DEPRECATE_EOF && current_packet_type(conn) == TRILOGY_PACKET_EOF) {
if ((err = read_ok_packet(conn)) != TRILOGY_OK) {
return err;
if (current_packet_type(conn) == TRILOGY_PACKET_EOF && conn->packet_buffer.len < 9) {
if ((rc = read_eof_packet(conn)) != TRILOGY_OK) {
return rc;
}

return TRILOGY_EOF;
} else if (current_packet_type(conn) == TRILOGY_PACKET_EOF && conn->packet_buffer.len < 9) {
return read_eof_packet(conn);
} else if (current_packet_type(conn) == TRILOGY_PACKET_ERR) {
return read_err_packet(conn);
} else {
Expand Down

0 comments on commit e322daa

Please sign in to comment.