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

Fail to decode interval with value bigger than int32 #8887

Closed
DifferentialOrange opened this issue Jul 17, 2023 · 0 comments · Fixed by #8888
Closed

Fail to decode interval with value bigger than int32 #8887

DifferentialOrange opened this issue Jul 17, 2023 · 0 comments · Fixed by #8888
Assignees
Labels
1sp bug Something isn't working teamE

Comments

@DifferentialOrange
Copy link
Contributor

It is allowed to create intervals with values bigger than int32 for minutes and seconds.

local MAX_MIN_RANGE = MAX_HOUR_RANGE * 60
local MAX_SEC_RANGE = MAX_DAY_RANGE * SECS_PER_DAY

Since doubles are used to store values, interval itself can process such big values.

struct interval {
/** Duration in seconds. */
double sec;
/** Number of minutes, if specified. */
double min;
/** Number of hours, if specified. */
double hour;
/** Number of days, if specified. */
double day;
/** Number of weeks, if specified. */
int32_t week;
/** Number of months, if specified. */
int32_t month;
/** Number of years, if specified. */
int32_t year;
/** Fraction part of duration in seconds. */
int32_t nsec;
/** Adjustment mode for day in month operations, @sa dt_adjust_t */
dt_adjust_t adjust;
};

Msgpack encoding also succeeds:

tarantool> msgpack.encode(datetime.interval.new{min=6184879877159}):hex()
---
- c70d060205cf000005a007916c270801
...

Yet msgpack decoding fails:

tarantool> msgpack.decode(msgpack.encode(datetime.interval.new{min=6184879877159}))
---
- error: 'msgpack.decode: invalid MsgPack'
...

The reason is that int32 is used to extract the value

if (mp_read_int32(data, &value) != 0)

while allowed values are not really restricted to int32. (For example, msgpack above has uint64 value for minutes.)

@DifferentialOrange DifferentialOrange added the bug Something isn't working label Jul 17, 2023
DifferentialOrange added a commit to DifferentialOrange/tarantool that referenced this issue Jul 17, 2023
It is possible for interval to have days, hours, minutes and seconds
bigger 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 tarantool#8887

NO_DOC=small bug fix
DifferentialOrange added a commit to DifferentialOrange/tarantool that referenced this issue Jul 17, 2023
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 tarantool#8887

NO_DOC=small bug fix
DifferentialOrange added a commit to tarantool/tarantool-python that referenced this issue Jul 17, 2023
Before this patch, any value was allowed for interval attributes. Now
we use the same rules as in Tarantool. A couple of issues were met while
developing this patch, follow [1, 2] for core updates.

1. tarantool/tarantool#8878
2. tarantool/tarantool#8887
DifferentialOrange added a commit to tarantool/tarantool-python that referenced this issue Jul 18, 2023
Before this patch, any value was allowed for interval attributes. Now
we use the same rules as in Tarantool. A couple of issues were met while
developing this patch, follow [1, 2] for core updates.

1. tarantool/tarantool#8878
2. tarantool/tarantool#8887
DifferentialOrange added a commit to tarantool/tarantool-python that referenced this issue Jul 18, 2023
Before this patch, any value was allowed for interval attributes. Now
we use the same rules as in Tarantool. A couple of issues were met while
developing this patch, follow [1, 2] for core updates.

1. tarantool/tarantool#8878
2. tarantool/tarantool#8887
DifferentialOrange added a commit to tarantool/tarantool-python that referenced this issue Jul 18, 2023
Before this patch, any value was allowed for interval attributes. Now
we use the same rules as in Tarantool. A couple of issues were met while
developing this patch, follow [1, 2] for core updates.

1. tarantool/tarantool#8878
2. tarantool/tarantool#8887
@DifferentialOrange DifferentialOrange self-assigned this Jul 21, 2023
DifferentialOrange added a commit to DifferentialOrange/tarantool that referenced this issue Jul 21, 2023
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 tarantool#8887

NO_DOC=small bug fix
igormunkin pushed a commit that referenced this issue Jul 24, 2023
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
igormunkin pushed a commit that referenced this issue Jul 24, 2023
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)
igormunkin pushed a commit that referenced this issue Jul 24, 2023
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1sp bug Something isn't working teamE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants