Skip to content

Commit a7f4bf4

Browse files
author
lexeyo
committed
fix postgres: fail on possible overflows during narrowing type conversions
Adds an explicit `numeric_cast` to `IntegralBinaryParser` that throws an exception on overflow when casting to the user-specified C++ type. This change can potentially break client code that relies on implicit overflows. commit_hash:ff55e525ae322e07d592468b78c2c01de1600fcb
1 parent 1148ce6 commit a7f4bf4

File tree

2 files changed

+72
-10
lines changed

2 files changed

+72
-10
lines changed

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <userver/storages/postgres/io/buffer_io_base.hpp>
1313
#include <userver/storages/postgres/io/traits.hpp>
1414
#include <userver/storages/postgres/io/type_mapping.hpp>
15+
#include <userver/utils/numeric_cast.hpp>
1516

1617
USERVER_NAMESPACE_BEGIN
1718

@@ -53,17 +54,21 @@ template <typename T>
5354
struct IntegralBinaryParser : BufferParserBase<T> {
5455
using BaseType = BufferParserBase<T>;
5556
using BaseType::BaseType;
57+
using ValueType = typename BaseType::ValueType;
5658

5759
void operator()(const FieldBuffer& buf) {
5860
switch (buf.length) {
5961
case 2:
60-
this->value = IntegralBySizeParser<2>::ParseBuffer(buf);
62+
this->value = USERVER_NAMESPACE::utils::numeric_cast<ValueType>(IntegralBySizeParser<2>::ParseBuffer(buf
63+
));
6164
break;
6265
case 4:
63-
this->value = IntegralBySizeParser<4>::ParseBuffer(buf);
66+
this->value = USERVER_NAMESPACE::utils::numeric_cast<ValueType>(IntegralBySizeParser<4>::ParseBuffer(buf
67+
));
6468
break;
6569
case 8:
66-
this->value = IntegralBySizeParser<8>::ParseBuffer(buf);
70+
this->value = USERVER_NAMESPACE::utils::numeric_cast<ValueType>(IntegralBySizeParser<8>::ParseBuffer(buf
71+
));
6772
break;
6873
default:
6974
throw InvalidInputBufferSize{fmt::format(

postgresql/src/storages/postgres/tests/integral_test.cpp

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
#include <gtest/gtest.h>
22

3+
#include <limits>
4+
#include <stdexcept>
5+
36
#include <storages/postgres/detail/connection.hpp>
47
#include <storages/postgres/tests/test_buffers.hpp>
58
#include <storages/postgres/tests/util_pgtest.hpp>
@@ -20,6 +23,23 @@ class PostgreIOIntegral : public ::testing::Test {};
2023

2124
using IntTypes = ::testing::Types<pg::Smallint, pg::Integer, pg::Bigint, short, int, long, long long>;
2225

26+
template <typename TPgType, typename TValueType>
27+
struct NarrowingTypes {
28+
using PgType = TPgType;
29+
using ValueType = TValueType;
30+
};
31+
32+
template <typename NarrowingTypes>
33+
class PostgreIOIntegralNarrowing : public ::testing::Test {
34+
using PgType = typename NarrowingTypes::PgType;
35+
using ValueType = typename NarrowingTypes::ValueType;
36+
};
37+
38+
using NarrowingTestTypes = ::testing::Types<
39+
NarrowingTypes<pg::Integer, pg::Smallint>,
40+
NarrowingTypes<pg::Bigint, pg::Smallint>,
41+
NarrowingTypes<pg::Bigint, pg::Integer> >;
42+
2343
} // namespace
2444

2545
TEST(PostgreIOIntegral, ParserRegistry) {
@@ -48,14 +68,51 @@ TYPED_TEST(PostgreIOIntegral, Int) {
4868
static_assert(tt::kIsMappedToPg<TypeParam>, "missing mapping");
4969
static_assert(tt::kHasFormatter<TypeParam>, "missing binary formatter");
5070
static_assert(tt::kHasParser<TypeParam>, "missing binary parser");
71+
using Limits = std::numeric_limits<TypeParam>;
5172

52-
pg::test::Buffer buffer;
53-
const TypeParam src{42};
54-
UEXPECT_NO_THROW(io::WriteBuffer(types, buffer, src));
55-
auto fb = pg::test::MakeFieldBuffer(buffer, io::BufferCategory::kPlainBuffer);
56-
TypeParam tgt{0};
57-
UEXPECT_NO_THROW(io::ReadBuffer(fb, tgt));
58-
EXPECT_EQ(src, tgt);
73+
for (const TypeParam src :
74+
{Limits::lowest(), TypeParam(-42), TypeParam(-1), TypeParam(0), TypeParam(1), TypeParam(42), Limits::max()})
75+
{
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+
TypeParam tgt{0};
80+
UEXPECT_NO_THROW(io::ReadBuffer(fb, tgt));
81+
EXPECT_EQ(src, tgt);
82+
}
83+
}
84+
85+
TYPED_TEST_SUITE(PostgreIOIntegralNarrowing, NarrowingTestTypes);
86+
87+
TYPED_TEST(PostgreIOIntegralNarrowing, Success) {
88+
using ValueType = typename TypeParam::ValueType;
89+
using Limits = std::numeric_limits<ValueType>;
90+
91+
for (const ValueType src :
92+
{Limits::lowest(), ValueType(-42), ValueType(-1), ValueType(0), ValueType(1), ValueType(42), Limits::max()})
93+
{
94+
pg::test::Buffer buffer;
95+
const typename TypeParam::PgType output = src;
96+
UEXPECT_NO_THROW(io::WriteBuffer(types, buffer, output));
97+
auto fb = pg::test::MakeFieldBuffer(buffer, io::BufferCategory::kPlainBuffer);
98+
ValueType tgt{0};
99+
UEXPECT_NO_THROW(io::ReadBuffer(fb, tgt));
100+
EXPECT_EQ(src, tgt);
101+
}
102+
}
103+
104+
TYPED_TEST(PostgreIOIntegralNarrowing, Fail) {
105+
using ValueType = typename TypeParam::ValueType;
106+
using PgType = typename TypeParam::PgType;
107+
using Limits = std::numeric_limits<ValueType>;
108+
109+
for (const PgType src : {static_cast<PgType>(Limits::lowest()) - 1, static_cast<PgType>(Limits::max()) + 1}) {
110+
pg::test::Buffer buffer;
111+
UEXPECT_NO_THROW(io::WriteBuffer(types, buffer, src));
112+
auto fb = pg::test::MakeFieldBuffer(buffer, io::BufferCategory::kPlainBuffer);
113+
ValueType tgt{0};
114+
UEXPECT_THROW(io::ReadBuffer(fb, tgt), std::runtime_error);
115+
}
59116
}
60117

61118
USERVER_NAMESPACE_END

0 commit comments

Comments
 (0)