Skip to content

Commit 740daed

Browse files
Gumixlocker
Gumix
authored and
locker
committed
box: support MP_ARROW by mp_check()
The missed `mp_validate_arrow()` is added to `msgpack_check_ext_data()`. Also the xrow body decoding error now contains its cause. Follow-up tarantool#10508 NO_DOC=bugfix NO_CHANGELOG=No sense in mentioning in CE
1 parent caca1e1 commit 740daed

File tree

7 files changed

+107
-15
lines changed

7 files changed

+107
-15
lines changed

‎src/box/msgpack.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "msgpuck/msgpuck.h"
3333

3434
#include "mp_extension_types.h"
35+
#include "mp_arrow.h"
3536
#include "mp_decimal.h"
3637
#include "mp_error.h"
3738
#include "mp_uuid.h"
@@ -143,6 +144,14 @@ msgpack_check_ext_data(int8_t type, const char *data, uint32_t len)
143144
return -1;
144145
}
145146
return 0;
147+
case MP_ARROW:
148+
if (mp_validate_arrow(data, len) != 0) {
149+
/* The error is set by arrow_ipc_decode(). */
150+
diag_add(ClientError, ER_INVALID_MSGPACK,
151+
"cannot unpack arrow data");
152+
return -1;
153+
}
154+
return 0;
146155
case MP_COMPRESSION:
147156
default:
148157
return mp_check_ext_data_default(type, data, len);

‎src/box/xrow.c‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -258,23 +258,32 @@ xrow_decode(struct xrow_header *header, const char **pos,
258258
/* Restore transaction id from lsn and transaction serial number. */
259259
header->tsn = header->lsn - header->tsn;
260260

261-
/* Nop requests aren't supposed to have a body. */
262-
if (*pos < end && header->type != IPROTO_NOP) {
263-
const char *body = *pos;
264-
if (mp_check(pos, end))
265-
goto bad_body;
266-
header->bodycnt = 1;
267-
header->body[0].iov_base = (void *) body;
268-
header->body[0].iov_len = *pos - body;
269-
}
270-
if (end_is_exact && *pos < end)
271-
goto bad_body;
261+
if (*pos == end) {
262+
/* No body, nothing to validate. */
263+
return 0;
264+
}
265+
if (header->type == IPROTO_NOP) {
266+
if (end_is_exact) {
267+
diag_set(ClientError, ER_INVALID_MSGPACK,
268+
"junk after packet body");
269+
goto dump;
270+
}
271+
/* Nop requests aren't supposed to have a body. */
272+
return 0;
273+
}
274+
275+
const char *body = *pos;
276+
int rc = end_is_exact ? mp_check_exact(pos, end) : mp_check(pos, end);
277+
if (rc != 0) {
278+
diag_add(ClientError, ER_INVALID_MSGPACK, "packet body");
279+
goto dump;
280+
}
281+
header->bodycnt = 1;
282+
header->body[0].iov_base = (void *)body;
283+
header->body[0].iov_len = *pos - body;
272284
return 0;
273285
bad_header:
274286
diag_set(ClientError, ER_INVALID_MSGPACK, "packet header");
275-
goto dump;
276-
bad_body:
277-
diag_set(ClientError, ER_INVALID_MSGPACK, "packet body");
278287
dump:
279288
dump_row_hex(start, end);
280289
return -1;

‎src/lib/core/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ set(core_sources
2929
trigger.cc
3030
port.c
3131
arrow_ipc.c
32+
mp_arrow.c
3233
decimal.c
3334
mp_decimal.c
3435
cord_buf.c

‎src/lib/core/mp_arrow.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* SPDX-License-Identifier: BSD-2-Clause
3+
*
4+
* Copyright 2010-2024, Tarantool AUTHORS, please see AUTHORS file.
5+
*/
6+
7+
#include "mp_arrow.h"
8+
9+
#include <assert.h>
10+
#include <string.h>
11+
#include "core/arrow_ipc.h"
12+
13+
int
14+
mp_validate_arrow(const char *data, uint32_t len)
15+
{
16+
struct ArrowArray array;
17+
struct ArrowSchema schema;
18+
memset(&array, 0, sizeof(array));
19+
memset(&schema, 0, sizeof(schema));
20+
if (arrow_ipc_decode(&array, &schema, data, data + len) != 0)
21+
return 1;
22+
assert(array.release != NULL);
23+
assert(schema.release != NULL);
24+
array.release(&array);
25+
schema.release(&schema);
26+
return 0;
27+
}

‎src/lib/core/mp_arrow.h‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* SPDX-License-Identifier: BSD-2-Clause
3+
*
4+
* Copyright 2010-2024, Tarantool AUTHORS, please see AUTHORS file.
5+
*/
6+
7+
#pragma once
8+
9+
#include <stdint.h>
10+
11+
#if defined(__cplusplus)
12+
extern "C" {
13+
#endif /* defined(__cplusplus) */
14+
15+
/**
16+
* Check that the given buffer contains a valid Arrow record batch.
17+
* @param data The buffer containing a record batch in Arrow IPC format, without
18+
* MP_EXT header.
19+
* @param len Length of @a data.
20+
* @retval 1 Couldn't decode Arrow data.
21+
* @retval 0 Ok.
22+
*/
23+
int
24+
mp_validate_arrow(const char *data, uint32_t len);
25+
26+
#if defined(__cplusplus)
27+
} /* extern "C" */
28+
#endif /* defined(__cplusplus) */

‎test/box-luatest/gh_10508_iproto_insert_arrow_test.lua‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ g.test_iproto_insert_arrow_invalid = function(cg)
8888
-- size: 0}}
8989
r = _G.iproto_insert_arrow('8210cd020036c70008')
9090
t.assert_equals(r[box.iproto.key.ERROR_24],
91+
'Invalid MsgPack - packet body')
92+
t.assert_equals(r[box.iproto.key.ERROR][0][2][3],
93+
'Invalid MsgPack - invalid extension')
94+
t.assert_equals(r[box.iproto.key.ERROR][0][3][3],
95+
'Invalid MsgPack - cannot unpack arrow data')
96+
t.assert_equals(r[box.iproto.key.ERROR][0][4][3],
9197
'Arrow decode error: unexpected data size')
9298
-- <MP_MAP> {
9399
-- IPROTO_SPACE_ID: 512,
@@ -97,6 +103,12 @@ g.test_iproto_insert_arrow_invalid = function(cg)
97103
-- data: [0xde, 0xad, 0xbe, 0xef]}}
98104
r = _G.iproto_insert_arrow('8210cd020036c70408deadbeef')
99105
t.assert_equals(r[box.iproto.key.ERROR_24],
106+
'Invalid MsgPack - packet body')
107+
t.assert_equals(r[box.iproto.key.ERROR][0][2][3],
108+
'Invalid MsgPack - invalid extension')
109+
t.assert_equals(r[box.iproto.key.ERROR][0][3][3],
110+
'Invalid MsgPack - cannot unpack arrow data')
111+
t.assert_equals(r[box.iproto.key.ERROR][0][4][3],
100112
'Arrow decode error: ' ..
101113
'Expected at least 8 bytes in remainder of stream')
102114

@@ -107,6 +119,12 @@ g.test_iproto_insert_arrow_invalid = function(cg)
107119
000004000000f0ffffff4000000001000000610000000600080004000c0010000400
108120
080009000c000c000c0000000400000008000a000c00040006000800ffffffff]])
109121
t.assert_equals(r[box.iproto.key.ERROR_24],
122+
'Invalid MsgPack - packet body')
123+
t.assert_equals(r[box.iproto.key.ERROR][0][2][3],
124+
'Invalid MsgPack - invalid extension')
125+
t.assert_equals(r[box.iproto.key.ERROR][0][3][3],
126+
'Invalid MsgPack - cannot unpack arrow data')
127+
t.assert_equals(r[box.iproto.key.ERROR][0][4][3],
110128
'Arrow decode error: ' ..
111129
'Expected at least 8 bytes in remainder of stream')
112130

‎test/box-luatest/iproto_request_handlers_overriding_test.lua‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ g.test_box_iproto_override_nop_rq_type = function(cg)
310310
[box.iproto.key.SQL_TEXT] = 'text'
311311
}, {__serialize = 'map'})
312312
_G.test_cb_err(header, body, box.error.INVALID_MSGPACK,
313-
"Invalid MsgPack %- packet body")
313+
"Invalid MsgPack %- junk after packet body")
314314
box.iproto.override(box.iproto.type.NOP, nil)
315315
end)
316316
end

0 commit comments

Comments
 (0)