Skip to content

Commit ecb4de1

Browse files
authored
Merge pull request #161 from metsw24-max/safe-join-reserve-calculation
Use checked arithmetic for Join() reserve size calculation
2 parents ae29bb6 + f8ebfe7 commit ecb4de1

2 files changed

Lines changed: 188 additions & 4 deletions

File tree

‎args.hxx‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,16 +275,58 @@ namespace args
275275
std::string Join(const T& array, const std::string &delimiter)
276276
{
277277
std::string res;
278-
for (auto &element : array)
278+
279+
// Safely compute reservation size to avoid unbounded reallocations
280+
using size_type = std::string::size_type;
281+
size_type total = 0;
282+
size_type count = 0;
283+
const size_type delim_size = static_cast<size_type>(delimiter.size());
284+
bool can_reserve = true;
285+
286+
for (const auto &element : array)
279287
{
280-
if (!res.empty())
288+
const size_type elem_size = static_cast<size_type>(element.size());
289+
if (!SafeAdd<size_type>(total, elem_size, total))
290+
{
291+
can_reserve = false;
292+
break;
293+
}
294+
++count;
295+
}
296+
297+
if (can_reserve && count > 1)
298+
{
299+
size_type delim_count = count - 1;
300+
size_type delim_total = 0;
301+
if (!SafeMultiply<size_type>(delim_count, delim_size, delim_total) ||
302+
!SafeAdd<size_type>(total, delim_total, total))
303+
{
304+
can_reserve = false;
305+
}
306+
}
307+
308+
if (can_reserve && total > 0)
309+
{
310+
try
311+
{
312+
res.reserve(total);
313+
}
314+
catch (...) {
315+
// Fall back to default allocation
316+
}
317+
}
318+
319+
bool first = true;
320+
for (const auto &element : array)
321+
{
322+
if (!first)
281323
{
282324
res += delimiter;
283325
}
284-
285326
res += element;
327+
first = false;
286328
}
287-
329+
288330
return res;
289331
}
290332
}

‎test_join_debug.cpp‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
#include <iostream>
2+
#include <vector>
3+
#include <string>
4+
#include <limits>
5+
6+
// Minimal version of the safe functions
7+
namespace args
8+
{
9+
template<typename T>
10+
bool SafeAdd(T a, T b, T& out)
11+
{
12+
if (std::is_unsigned<T>::value || true)
13+
{
14+
if (b > std::numeric_limits<T>::max() - a)
15+
{
16+
return false;
17+
}
18+
}
19+
out = a + b;
20+
return true;
21+
}
22+
23+
template<typename T>
24+
bool SafeMultiply(T a, T b, T& out)
25+
{
26+
if (a == 0 || b == 0)
27+
{
28+
out = 0;
29+
return true;
30+
}
31+
32+
if (std::is_unsigned<T>::value || true)
33+
{
34+
if (b > std::numeric_limits<T>::max() / a)
35+
{
36+
return false;
37+
}
38+
}
39+
out = a * b;
40+
return true;
41+
}
42+
43+
namespace detail
44+
{
45+
template <typename T>
46+
std::string Join(const T& array, const std::string &delimiter)
47+
{
48+
// Calculate total size safely to avoid integer overflow and
49+
// excessive reallocations. If the total size would overflow,
50+
// return an empty string to fail safely.
51+
using size_type = std::string::size_type;
52+
size_type total = 0;
53+
const size_type delim_size = static_cast<size_type>(delimiter.size());
54+
size_type count = 0;
55+
56+
for (const auto &element : array)
57+
{
58+
const size_type elem_size = static_cast<size_type>(element.size());
59+
if (!SafeAdd<size_type>(total, elem_size, total))
60+
{
61+
std::cout << "FAIL: SafeAdd overflow in element sum" << std::endl;
62+
return std::string();
63+
}
64+
++count;
65+
}
66+
67+
std::cout << "After loop: count=" << count << " total=" << total << std::endl;
68+
69+
if (count > 0 && count > 1)
70+
{
71+
// total delimiters = count - 1 (safe since count > 1)
72+
const size_type delim_count = count - 1;
73+
74+
size_type delim_total = 0;
75+
if (!SafeMultiply<size_type>(delim_count, delim_size, delim_total))
76+
{
77+
std::cout << "FAIL: SafeMultiply overflow in delimiter" << std::endl;
78+
return std::string();
79+
}
80+
81+
std::cout << "Delimiters: count=" << delim_count << " delim_size=" << delim_size << " delim_total=" << delim_total << std::endl;
82+
83+
if (!SafeAdd<size_type>(total, delim_total, total))
84+
{
85+
std::cout << "FAIL: SafeAdd overflow in total+delimiter" << std::endl;
86+
return std::string();
87+
}
88+
}
89+
90+
std::cout << "Final total before reserve: " << total << std::endl;
91+
92+
std::string res;
93+
try
94+
{
95+
res.reserve(total);
96+
}
97+
catch (...) {
98+
std::cout << "FAIL: reserve threw" << std::endl;
99+
return std::string();
100+
}
101+
102+
bool first = true;
103+
for (const auto &element : array)
104+
{
105+
if (!first)
106+
{
107+
res += delimiter;
108+
}
109+
res += element;
110+
first = false;
111+
}
112+
113+
std::cout << "Final result: '" << res << "' (length=" << res.length() << ")" << std::endl;
114+
return res;
115+
}
116+
}
117+
}
118+
119+
int main()
120+
{
121+
std::vector<std::string> v1 = {"a", "b", "c"};
122+
std::cout << "Test 1: {a, b, c} with ', '" << std::endl;
123+
auto r1 = args::detail::Join(v1, ", ");
124+
std::cout << "Result: '" << r1 << "' (expected 'a, b, c')" << std::endl << std::endl;
125+
126+
std::vector<std::string> v2 = {"alpha", "beta"};
127+
std::cout << "Test 2: {alpha, beta} with '\\n'" << std::endl;
128+
auto r2 = args::detail::Join(v2, "\n");
129+
std::cout << "Result: '" << r2 << "' (expected 'alpha\\nbeta')" << std::endl << std::endl;
130+
131+
std::vector<std::string> v3 = {};
132+
std::cout << "Test 3: {} with ', '" << std::endl;
133+
auto r3 = args::detail::Join(v3, ", ");
134+
std::cout << "Result: '" << r3 << "' (expected '')" << std::endl << std::endl;
135+
136+
std::vector<std::string> v4 = {"single"};
137+
std::cout << "Test 4: {single} with ', '" << std::endl;
138+
auto r4 = args::detail::Join(v4, ", ");
139+
std::cout << "Result: '" << r4 << "' (expected 'single')" << std::endl;
140+
141+
return 0;
142+
}

0 commit comments

Comments
 (0)