Skip to content

Commit ead6493

Browse files
author
lexeyo
committed
fix postgres, utils: fail on possible overflows during float to double conversions
Adds an explicit `numeric_cast` to `FloatingPointBinaryParser` that throws an exception on overflow when casting `double` to `float`. This change can potentially break client code that relies on implicit overflows. Adds `numeric_cast` implementations for casts between `float` and `double`. commit_hash:ed3ba3e886cfcaaf0e8bee7e8b302fde3fb4a91d
1 parent 6000934 commit ead6493

File tree

5 files changed

+147
-9
lines changed

5 files changed

+147
-9
lines changed

.mapping.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3534,6 +3534,7 @@
35343534
"postgresql/src/storages/postgres/tests/dsn_test.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/dsn_test.cpp",
35353535
"postgresql/src/storages/postgres/tests/enums_pgtest.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/enums_pgtest.cpp",
35363536
"postgresql/src/storages/postgres/tests/error_test.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/error_test.cpp",
3537+
"postgresql/src/storages/postgres/tests/floating_point_test.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/floating_point_test.cpp",
35373538
"postgresql/src/storages/postgres/tests/geometry_pgtest.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/geometry_pgtest.cpp",
35383539
"postgresql/src/storages/postgres/tests/integral_test.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/integral_test.cpp",
35393540
"postgresql/src/storages/postgres/tests/interval_pgtest.cpp":"taxi/uservices/userver/postgresql/src/storages/postgres/tests/interval_pgtest.cpp",

postgresql/include/userver/storages/postgres/io/floating_point_types.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,15 @@ template <typename T>
4949
struct FloatingPointBinaryParser : BufferParserBase<T> {
5050
using BaseType = BufferParserBase<T>;
5151
using BaseType::BaseType;
52+
using ValueType = typename BaseType::ValueType;
5253

5354
void operator()(const FieldBuffer& buf) {
5455
switch (buf.length) {
5556
case 4:
56-
this->value = FloatingPointBySizeParser<4>::ParseBuffer(buf);
57+
this->value = NumericCast<ValueType>(FloatingPointBySizeParser<4>::ParseBuffer(buf));
5758
break;
5859
case 8:
59-
this->value = FloatingPointBySizeParser<8>::ParseBuffer(buf);
60+
this->value = NumericCast<ValueType>(FloatingPointBySizeParser<8>::ParseBuffer(buf));
6061
break;
6162
default:
6263
throw InvalidInputBufferSize{
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
#include <gtest/gtest.h>
2+
3+
#include <limits>
4+
5+
#include <storages/postgres/detail/connection.hpp>
6+
#include <storages/postgres/tests/test_buffers.hpp>
7+
#include <storages/postgres/tests/util_pgtest.hpp>
8+
9+
USERVER_NAMESPACE_BEGIN
10+
11+
namespace pg = storages::postgres;
12+
namespace io = pg::io;
13+
14+
namespace {
15+
16+
namespace tt = io::traits;
17+
18+
const pg::UserTypes types;
19+
20+
template <typename Float>
21+
class PostgreIOFloating : public ::testing::Test {};
22+
23+
using FloatTypes = ::testing::Types<float, double>;
24+
25+
} // namespace
26+
27+
TEST(PostgreIOFloating, ParserRegistry) {
28+
EXPECT_TRUE(io::HasParser(io::PredefinedOids::kFloat4));
29+
EXPECT_TRUE(io::HasParser(io::PredefinedOids::kFloat8));
30+
}
31+
32+
TYPED_TEST_SUITE(PostgreIOFloating, FloatTypes);
33+
34+
TYPED_TEST(PostgreIOFloating, Float) {
35+
static_assert(tt::kIsMappedToPg<TypeParam>, "missing mapping");
36+
static_assert(tt::kHasFormatter<TypeParam>, "missing binary formatter");
37+
static_assert(tt::kHasParser<TypeParam>, "missing binary parser");
38+
using Limits = std::numeric_limits<TypeParam>;
39+
40+
for (const TypeParam src :
41+
{Limits::lowest(),
42+
TypeParam(-42.0),
43+
TypeParam(-1.1),
44+
TypeParam(0.),
45+
TypeParam(1.5),
46+
TypeParam(36.6),
47+
Limits::max()})
48+
{
49+
pg::test::Buffer buffer;
50+
UEXPECT_NO_THROW(io::WriteBuffer(types, buffer, src));
51+
auto fb = pg::test::MakeFieldBuffer(buffer, io::BufferCategory::kPlainBuffer);
52+
TypeParam tgt{0};
53+
UEXPECT_NO_THROW(io::ReadBuffer(fb, tgt));
54+
EXPECT_EQ(src, tgt);
55+
}
56+
}
57+
58+
TEST(PostgreIOFloating, NarrowingSuccess) {
59+
using Limits = std::numeric_limits<float>;
60+
61+
for (const float src : {Limits::lowest(), -42.0f, -1.1f, 0.f, 1.5f, 36.6f, Limits::max()}) {
62+
pg::test::Buffer buffer;
63+
const double output = src;
64+
UEXPECT_NO_THROW(io::WriteBuffer(types, buffer, output));
65+
auto fb = pg::test::MakeFieldBuffer(buffer, io::BufferCategory::kPlainBuffer);
66+
float tgt{0};
67+
UEXPECT_NO_THROW(io::ReadBuffer(fb, tgt));
68+
EXPECT_EQ(src, tgt);
69+
}
70+
}
71+
72+
TEST(PostgreIOFloating, NarrowingFail) {
73+
using Limits = std::numeric_limits<float>;
74+
75+
for (const double src : {2.0 * Limits::lowest(), 2.0 * Limits::max()}) {
76+
pg::test::Buffer buffer;
77+
UEXPECT_NO_THROW(io::WriteBuffer(types, buffer, src));
78+
auto fb = pg::test::MakeFieldBuffer(buffer, io::BufferCategory::kPlainBuffer);
79+
float tgt{0};
80+
UEXPECT_THROW(io::ReadBuffer(fb, tgt), storages::postgres::NarrowingOverflow);
81+
}
82+
}
83+
84+
USERVER_NAMESPACE_END

universal/include/userver/utils/numeric_cast.hpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,32 @@ using PrintableValue = std::conditional_t<(sizeof(T) > 1), T, int>;
2929
/// @snippet utils/numeric_cast_test.cpp Sample utils::numeric_cast usage
3030
template <typename To, typename Exception = std::runtime_error, typename From>
3131
constexpr To numeric_cast(From input) { // NOLINT(readability-identifier-naming)
32-
static_assert(std::is_integral_v<From>);
33-
static_assert(std::is_integral_v<To>);
32+
constexpr bool is_integral = std::is_integral_v<From> && std::is_integral_v<To>;
33+
constexpr bool is_floating_point = std::is_floating_point_v<From> && std::is_floating_point_v<To>;
34+
static_assert(is_integral || is_floating_point);
35+
3436
using FromLimits = std::numeric_limits<From>;
3537
using ToLimits = std::numeric_limits<To>;
3638

39+
constexpr bool check_positive_overflow =
40+
is_integral ? ToLimits::digits < FromLimits::digits : ToLimits::max() < FromLimits::max();
41+
constexpr bool check_negative_overflow =
42+
is_integral
43+
?
44+
// signed -> signed: possible loss if narrowing
45+
// signed -> unsigned: loss if negative
46+
FromLimits::is_signed && (!ToLimits::is_signed || ToLimits::digits < FromLimits::digits)
47+
: ToLimits::lowest() > FromLimits::lowest();
48+
3749
std::string_view overflow_type{};
3850

39-
if constexpr (ToLimits::digits < FromLimits::digits) {
51+
if constexpr (check_positive_overflow) {
4052
if (input > static_cast<From>(ToLimits::max())) {
4153
overflow_type = "positive";
4254
}
4355
}
4456

45-
// signed -> signed: loss if narrowing
46-
// signed -> unsigned: loss
47-
if constexpr (FromLimits::is_signed && (!ToLimits::is_signed || ToLimits::digits < FromLimits::digits)) {
57+
if constexpr (check_negative_overflow) {
4858
if (input < static_cast<From>(ToLimits::lowest())) {
4959
overflow_type = "negative";
5060
}

universal/src/utils/numeric_cast_test.cpp

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#include <limits>
2+
13
#include <gmock/gmock.h>
24
#include <userver/utest/assert_macros.hpp>
35

@@ -7,11 +9,28 @@ USERVER_NAMESPACE_BEGIN
79

810
namespace {
911

10-
TEST(NumericCast, CompileTime) { static_assert(utils::numeric_cast<unsigned int>(1) == 1u); }
12+
TEST(NumericCast, CompileTime) {
13+
static_assert(utils::numeric_cast<unsigned int>(1) == 1u);
14+
static_assert(utils::numeric_cast<float>(1.0) == 1.0f);
15+
static_assert(utils::numeric_cast<double>(1.0f) == 1.0);
16+
static_assert(utils::numeric_cast<long double>(1.0) == 1.0L);
17+
}
1118

1219
TEST(NumericCast, Smoke) {
1320
EXPECT_EQ(utils::numeric_cast<unsigned int>(1), 1u);
1421
EXPECT_EQ(utils::numeric_cast<std::size_t>(1), static_cast<std::size_t>(1));
22+
23+
EXPECT_EQ(utils::numeric_cast<float>(1.0f), 1.0f);
24+
EXPECT_EQ(utils::numeric_cast<float>(1.0), 1.0f);
25+
EXPECT_EQ(utils::numeric_cast<float>(1.0L), 1.0f);
26+
27+
EXPECT_EQ(utils::numeric_cast<double>(1.0f), 1.0);
28+
EXPECT_EQ(utils::numeric_cast<double>(1.0), 1.0);
29+
EXPECT_EQ(utils::numeric_cast<double>(1.0L), 1.0);
30+
31+
EXPECT_EQ(utils::numeric_cast<long double>(1.0f), 1.0L);
32+
EXPECT_EQ(utils::numeric_cast<long double>(1.0), 1.0L);
33+
EXPECT_EQ(utils::numeric_cast<long double>(1.0L), 1.0L);
1534
}
1635

1736
TEST(NumericCast, SignedToUnsignedOverflow) {
@@ -32,6 +51,29 @@ TEST(NumericCast, UnsignedToSignedOverflow) {
3251
EXPECT_THROW(utils::numeric_cast<std::int16_t>(0x8000), std::runtime_error);
3352
}
3453

54+
TEST(NumericCast, FloatingPointOverflow) {
55+
/// [Sample utils::numeric_cast usage]
56+
using FloatLimits = std::numeric_limits<float>;
57+
58+
EXPECT_EQ(utils::numeric_cast<float>(double(FloatLimits::max())), FloatLimits::max());
59+
EXPECT_THROW(utils::numeric_cast<float>(2.0 * FloatLimits::max()), std::runtime_error);
60+
EXPECT_EQ(utils::numeric_cast<float>(static_cast<long double>(FloatLimits::max())), FloatLimits::max());
61+
EXPECT_THROW(utils::numeric_cast<float>(2.0L * FloatLimits::max()), std::runtime_error);
62+
63+
EXPECT_EQ(utils::numeric_cast<float>(double(FloatLimits::lowest())), FloatLimits::lowest());
64+
EXPECT_THROW(utils::numeric_cast<float>(2.0 * FloatLimits::lowest()), std::runtime_error);
65+
EXPECT_EQ(utils::numeric_cast<float>(static_cast<long double>(FloatLimits::lowest())), FloatLimits::lowest());
66+
EXPECT_THROW(utils::numeric_cast<float>(2.0L * FloatLimits::lowest()), std::runtime_error);
67+
68+
using DoubleLimits = std::numeric_limits<double>;
69+
70+
EXPECT_EQ(utils::numeric_cast<double>(static_cast<long double>(DoubleLimits::max())), DoubleLimits::max());
71+
EXPECT_THROW(utils::numeric_cast<double>(2.0L * DoubleLimits::max()), std::runtime_error);
72+
EXPECT_EQ(utils::numeric_cast<double>(static_cast<long double>(DoubleLimits::lowest())), DoubleLimits::lowest());
73+
EXPECT_THROW(utils::numeric_cast<double>(2.0L * DoubleLimits::lowest()), std::runtime_error);
74+
/// [Sample utils::numeric_cast usage]
75+
}
76+
3577
} // namespace
3678

3779
USERVER_NAMESPACE_END

0 commit comments

Comments
 (0)