Skip to content

Commit

Permalink
msgpack: fix decoding intervals with int64
Browse files Browse the repository at this point in the history
It is possible for interval to have days, hours, minutes and seconds
larger than INT_MAX (or less than INT_MIN). Before this patch, msgpack
decoding had failed to parse intervals with msgpack int64 and uint64.
int64_t should be enough to store any value allowed for datetime
intervals.

Closes #8887

NO_DOC=small bug fix

(cherry picked from commit 01c7ae1)
  • Loading branch information
DifferentialOrange authored and igormunkin committed Jul 24, 2023
1 parent 678a2f0 commit 26acd38
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 8 deletions.
4 changes: 4 additions & 0 deletions changelogs/unreleased/gh-8887-fix-interval-int64.md
@@ -0,0 +1,4 @@
## bugfix/msgpack

* Fixed decoding datetime intervals with fields larger than possible int32
values (gh-8887).
6 changes: 3 additions & 3 deletions src/lib/core/mp_interval.c
Expand Up @@ -116,7 +116,7 @@ interval_unpack(const char **data, uint32_t len, struct interval *itv)
memset(itv, 0, sizeof(*itv));
for (uint32_t i = 0; i < count; ++i) {
uint32_t field = mp_load_u8(data);
int32_t value;
int64_t value;
enum mp_type type = mp_typeof(**data);
if (type == MP_UINT) {
if (mp_check_uint(*data, end) > 0)
Expand All @@ -127,7 +127,7 @@ interval_unpack(const char **data, uint32_t len, struct interval *itv)
} else {
return NULL;
}
if (mp_read_int32(data, &value) != 0)
if (mp_read_int64(data, &value) != 0)
return NULL;
switch (field) {
case FIELD_YEAR:
Expand Down Expand Up @@ -155,7 +155,7 @@ interval_unpack(const char **data, uint32_t len, struct interval *itv)
itv->nsec = value;
break;
case FIELD_ADJUST:
if (value > (int32_t)DT_SNAP)
if (value > (int64_t)DT_SNAP)
return NULL;
itv->adjust = (dt_adjust_t)value;
break;
Expand Down
13 changes: 10 additions & 3 deletions test/app-tap/datetime.test.lua
Expand Up @@ -5,6 +5,7 @@ local test = tap.test('errno')
local date = require('datetime')
local ffi = require('ffi')
local json = require('json')
local msgpack = require('msgpack')
local TZ = date.TZ

--[[
Expand Down Expand Up @@ -1321,7 +1322,7 @@ test:test("Time interval operations - different timezones", function(test)
end)

test:test("Time intervals creation - range checks", function(test)
test:plan(63)
test:plan(83)

local inew = date.interval.new

Expand Down Expand Up @@ -1386,12 +1387,18 @@ test:test("Time intervals creation - range checks", function(test)
local val_max = math.floor(range_max)

local attrib_min = {[name] = -val_max}
test:is(tostring(inew(attrib_min)), iv_str_repr(attrib_min),
local iv_min = inew(attrib_min)
test:is(tostring(iv_min), iv_str_repr(attrib_min),
('interval %s is allowed'):format(json.encode(attrib_min)))
test:is(msgpack.decode(msgpack.encode(iv_min)), iv_min,
('msgpack can process interval %s'):format(json.encode(attrib_min)))

local attrib_max = {[name] = val_max}
test:is(tostring(inew(attrib_max)), iv_str_repr(attrib_max),
local iv_max = inew(attrib_max)
test:is(tostring(iv_max), iv_str_repr(attrib_max),
('interval %s is allowed'):format(json.encode(attrib_max)))
test:is(msgpack.decode(msgpack.encode(iv_max)), iv_max,
('msgpack can process interval %s'):format(json.encode(attrib_max)))

local attrib_over_min = {[name] = -val_max - 1}
assert_raises(
Expand Down
46 changes: 45 additions & 1 deletion test/unit/interval.c
@@ -1,5 +1,6 @@
#include <stdio.h>
#include <assert.h>
#include <limits.h>

#include "unit.h"
#include "string.h"
Expand Down Expand Up @@ -94,11 +95,54 @@ test_interval_encode_decode(void)
is(result.adjust, DT_EXCESS, "Adjust value is right");
}

static void
test_interval_encode_decode_values_outside_int32_limits(void)
{
struct interval itv;
memset(&itv, 0, sizeof(itv));
struct interval result;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.day = (double)INT32_MIN - 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.day = (double)INT32_MAX + 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.hour = (double)INT32_MIN - 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.hour = (double)INT32_MAX + 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.min = (double)INT32_MIN - 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.min = (double)INT32_MAX + 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.sec = (double)INT32_MIN - 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");

itv.sec = (double)INT32_MAX + 1;
interval_mp_recode(&itv, &result);
ok(is_interval_equal(&itv, &result), "Intervals are equal.");
}

int
main(void)
{
plan(21);
plan(30);
test_interval_sizeof();
test_interval_encode_decode();
test_interval_encode_decode_values_outside_int32_limits();
return check_plan();
}
11 changes: 10 additions & 1 deletion test/unit/interval.result
@@ -1,4 +1,4 @@
1..21
1..30
ok 1 - Size of interval is 3
ok 2 - Size of interval is 6
ok 3 - Size of interval is 9
Expand All @@ -20,3 +20,12 @@ ok 18 - Minute value is right
ok 19 - Second value is right
ok 20 - Nanosecond value is right
ok 21 - Adjust value is right
ok 22 - Intervals are equal.
ok 23 - Intervals are equal.
ok 24 - Intervals are equal.
ok 25 - Intervals are equal.
ok 26 - Intervals are equal.
ok 27 - Intervals are equal.
ok 28 - Intervals are equal.
ok 29 - Intervals are equal.
ok 30 - Intervals are equal.

0 comments on commit 26acd38

Please sign in to comment.