Skip to content

Commit

Permalink
Merge pull request #8804 from Tomertech/out_of_boundary2
Browse files Browse the repository at this point in the history 
[Core] Validate range on controls change
  • Loading branch information
@ev-mp
ev-mp committed May 19, 2021
2 parents 24a02cb + 750565d commit 6399ca4
Show file tree
Hide file tree
Showing 4 changed files with 306 additions and 26 deletions.
4 changes: 2 additions & 2 deletions common/model-views.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,9 +682,9 @@ namespace rs2
ImGuiInputTextFlags_EnterReturnsTrue))
{
float new_value;
if (!string_to_int(buff, new_value))
if(!utilities::string::string_to_value<float>(buff, new_value))
{
error_message = "Invalid numeric input!";
error_message = "Invalid float input!";
}
else if (new_value < range.min || new_value > range.max)
{
Expand Down
32 changes: 8 additions & 24 deletions common/realsense-ui-advanced-mode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#include <librealsense2/rs_advanced_mode.hpp>
#include <types.h>
#include <type_traits>
#include "utilities/string/string-utilities.h"

#define TEXT_BUFF_SIZE 1024

Expand Down Expand Up @@ -56,24 +58,6 @@ bool* draw_edit_button(const char* id, T val, std::string*& val_str)
return &edit_mode[id];
}

inline bool string_to_int(const std::string& str, float& result)
{
try
{
std::size_t lastChar;
result = std::stof(str, &lastChar);
return lastChar == str.size();
}
catch (std::invalid_argument&)
{
return false;
}
catch (std::out_of_range&)
{
return false;
}
}

template<class T, class S>
inline void slider_int(std::string& error_message, const char* id, T* val, S T::* field, bool& to_set)
{
Expand All @@ -95,10 +79,10 @@ inline void slider_int(std::string& error_message, const char* id, T* val, S T::
if (ImGui::InputText(slider_id.c_str(), buff, TEXT_BUFF_SIZE,
ImGuiInputTextFlags_EnterReturnsTrue))
{
float new_value;
if (!string_to_int(buff, new_value))
int new_value{};
if (!utilities::string::string_to_value<int>(buff, new_value))
{
error_message = "Invalid numeric input!";
error_message = "Invalid integer input!";
}
else
{
Expand Down Expand Up @@ -160,10 +144,10 @@ inline void slider_float(std::string& error_message, const char* id, T* val, S T
if (ImGui::InputText(slider_id.c_str(), buff, TEXT_BUFF_SIZE,
ImGuiInputTextFlags_EnterReturnsTrue))
{
float new_value;
if (!string_to_int(buff, new_value))
int new_value;
if (!utilities::string::string_to_value<int>(buff, new_value))
{
error_message = "Invalid numeric input!";
error_message = "Invalid integer input!";
}
else
{
Expand Down
130 changes: 130 additions & 0 deletions common/utilities/string/string-utilities.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2021 Intel Corporation. All Rights Reserved.

#pragma once

#include <string>

namespace utilities {
namespace string {

template <typename T, typename std::enable_if <std::is_arithmetic<T>::value>::type* = nullptr> // check if T is arithmetic during compile
inline bool string_to_value(const std::string& str, T& result)
{
// Converts string to value via a given argument with desire type
// Input:
// - string of a number
// - argument with T type
// returns:true if conversion succeeded
// NOTE for unsigned types:
// if T is unsinged and the given string represents a negative number (meaning it starts with '-'),
// then string_to_value returns false (rather than send the string to the std conversion function).
// The reason for that is, stoul will return return a number in that case rather than throwing an exception.

try
{
size_t last_char_idx;
if (std::is_integral<T>::value)
{
if (std::is_same<T, unsigned long>::value)
{
if (str[0] == '-') return false; //for explanation see 'NOTE for unsigned types'
result = static_cast<T>(std::stoul(str, &last_char_idx));
}
else if (std::is_same<T, unsigned long long>::value)
{
if (str[0] == '-') return false;
result = static_cast<T>(std::stoull(str, &last_char_idx));
}
else if (std::is_same<T, long>::value)
{
result = static_cast<T>(std::stol(str, &last_char_idx));
}
else if (std::is_same<T, long long>::value)
{
result = static_cast<T>(std::stoll(str, &last_char_idx));
}
else if (std::is_same<T, int>::value)
{
result = static_cast<T>(std::stoi(str, &last_char_idx));
}
else if (std::is_same<T, short>::value)
{// no dedicated function fot short in std

int check_value = std::stoi(str, &last_char_idx);

if (check_value > std::numeric_limits<short>::max() || check_value < std::numeric_limits<short>::min())
throw std::out_of_range("short");

result = static_cast<T>(check_value);

}
else if (std::is_same<T, unsigned int>::value)
{// no dedicated function in std - unsgined corresponds to 16 bit, and unsigned long corresponds to 32 bit
if (str[0] == '-')
return false;

unsigned long check_value = std::stoul(str, &last_char_idx);

if (check_value > std::numeric_limits<unsigned int>::max())
throw std::out_of_range("unsigned int");

result = static_cast<T>(check_value);

}
else if (std::is_same<T, unsigned short>::value || std::is_same<T, unsigned short>::value)
{ // no dedicated function in std - unsgined corresponds to 16 bit, and unsigned long corresponds to 32 bit
if (str[0] == '-')
return false;

unsigned long check_value = std::stoul(str, &last_char_idx);

if (check_value > std::numeric_limits<unsigned short>::max())
throw std::out_of_range("unsigned short");

result = static_cast<T>(check_value);

}
else
{
return false;
}
}
else if (std::is_floating_point<T>::value)
{
if (std::is_same<T, float>::value)
{
result = static_cast<T>(std::stof(str, &last_char_idx));
if (!std::isfinite((float)result))
return false;
}
else if (std::is_same<T, double>::value)
{
result = static_cast<T>(std::stod(str, &last_char_idx));
if (!std::isfinite((double)result))
return false;
}
else if (std::is_same<T, long double>::value)
{
result = static_cast<T>(std::stold(str, &last_char_idx));
if (!std::isfinite((long double)result))
return false;
}
else
{
return false;
}
}
return str.size() == last_char_idx; // all the chars in the string are converted to the value (thus there are no other chars other than numbers)
}
catch (const std::invalid_argument&)
{
return false;
}
catch (const std::out_of_range&)
{
return false;
}
}
} // namespace string
} // namespace utilities
166 changes: 166 additions & 0 deletions unit-tests/utilities/string/test-string-to-value.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2020 Intel Corporation. All Rights Reserved.

//#cmake:add-file ../../../common/utilities/string/string-utilities.h

#include "common.h"
#include "../../../common/utilities/string/string-utilities.h"
#include <ostream>
using namespace utilities::string;

template <typename T, typename std::enable_if <std::is_arithmetic<T>::value>::type* = nullptr> // check if T is arithmetic during compile
std::string to_str(const T& t)
{
/// The std::to_string will make the min float value equal zero, so another function is
/// params: a value type T
/// returns: a string of the given number
std::ostringstream os;
os << t;
return os.str();
}

template<typename T>
struct string_checker
{
std::string str;
T value;
bool expected_res;
};

template<typename T>
void check_tests(std::vector<string_checker<T>>& tests)
{
T value;
for (const auto& test : tests)
{
CHECK(string_to_value<T>(test.str, value) == test.expected_res);
if (test.expected_res)
{
CHECK(value == test.value);
}
}
}

TEST_CASE("string to value", "[string]")
{
// int
std::vector<string_checker<int>> int_tests;
int_tests.push_back(string_checker<int>{ "123", 123, true });
int_tests.push_back(string_checker<int>{ "-123", -123, true });
int_tests.push_back(string_checker<int>{ "-123456789", -123456789, true });
int_tests.push_back(string_checker<int>{ std::to_string(std::numeric_limits<int>::max())+"0", 0, false });
int_tests.push_back(string_checker<int>{ std::to_string(std::numeric_limits<int>::min())+"0", 0, false });
int_tests.push_back(string_checker<int>{ "abc", 0, false });

check_tests<int>(int_tests);

// unsigned int
std::vector<string_checker<unsigned int>> uint_tests;
uint_tests.push_back(string_checker<unsigned int>{ "123", 123, true });
uint_tests.push_back(string_checker<unsigned int>{ "123456789", 123456789, true });
uint_tests.push_back(string_checker<unsigned int>{ "-123456789", 0, false });
uint_tests.push_back(string_checker<unsigned int>{ std::to_string(std::numeric_limits<unsigned int>::max())+"0", 0, false });
uint_tests.push_back(string_checker<unsigned int>{ "abc", 0, false });

check_tests<unsigned int>(uint_tests);

// short
std::vector<string_checker<short>> s_tests;
s_tests.push_back(string_checker<short>{ "123", 123, true });
s_tests.push_back(string_checker<short>{ "-123", -123, true });
s_tests.push_back(string_checker<short>{ std::to_string(std::numeric_limits<short>::max())+"0", 0, false });
s_tests.push_back(string_checker<short>{ std::to_string(std::numeric_limits<short>::min())+"0", 0, false });
s_tests.push_back(string_checker<short>{ "abc", 0, false });

check_tests<short>(s_tests);

// unsigned short
std::vector<string_checker<unsigned short>> us_tests;
us_tests.push_back(string_checker<unsigned short>{ "123", 123, true });
us_tests.push_back(string_checker<unsigned short>{ "-123", 0, false });
us_tests.push_back(string_checker<unsigned short>{std::to_string(std::numeric_limits<unsigned short>::max())+"0", 0, false });
us_tests.push_back(string_checker<unsigned short>{ "abc", 0, false });

check_tests<unsigned short>(us_tests);

// long
std::vector<string_checker<long>> l_tests;
l_tests.push_back(string_checker<long>{ "123", 123, true });
l_tests.push_back(string_checker<long>{ "-123", -123, true });
l_tests.push_back(string_checker<long>{ "-123456789", -123456789, true });
l_tests.push_back(string_checker<long>{ std::to_string(std::numeric_limits<long>::max())+"0", 0, false });
l_tests.push_back(string_checker<long>{ std::to_string(std::numeric_limits<long>::min())+"0", 0, false });
l_tests.push_back(string_checker<long>{ "abc", 0, false });

check_tests<long>(l_tests);

// long long
std::vector<string_checker<long long>> ll_tests;
ll_tests.push_back(string_checker<long long>{ "123", 123, true });
ll_tests.push_back(string_checker<long long>{ "-123", -123, true });
ll_tests.push_back(string_checker<long long>{ "12345", 12345, true });
ll_tests.push_back(string_checker<long long>{ "-12345", -12345, true });
ll_tests.push_back(string_checker<long long>{ std::to_string(std::numeric_limits<long long>::max())+"0", 0, false });
ll_tests.push_back(string_checker<long long>{ std::to_string(std::numeric_limits<long long>::min()) + "0", 0, false });
ll_tests.push_back(string_checker<long long>{ "abc", 0, false });

check_tests<long long>(ll_tests);

// ungined long long
std::vector<string_checker<unsigned long long>> ull_tests;
ull_tests.push_back(string_checker<unsigned long long>{ "123", 123, true });
ull_tests.push_back(string_checker<unsigned long long>{ "123456789", 123456789, true });
ull_tests.push_back(string_checker<unsigned long long>{ "-123456789", 0, false });
ull_tests.push_back(string_checker<unsigned long long>{ std::to_string(std::numeric_limits<unsigned long long>::max())+"0", 0, false });
ull_tests.push_back(string_checker<unsigned long long>{ "abc", 0, false });

check_tests<unsigned long long>(ull_tests);

// float
std::vector<string_checker<float>> f_tests;
f_tests.push_back(string_checker<float>{ "1.23456789", 1.23456789f, true });
f_tests.push_back(string_checker<float>{ "2.12121212", 2.12121212f, true });
f_tests.push_back(string_checker<float>{ "-1.23456789", -1.23456789f, true });
f_tests.push_back(string_checker<float>{ "-2.12121212", -2.12121212f, true });
f_tests.push_back(string_checker<float>{ "INF", 0.f, false });
f_tests.push_back(string_checker<float>{ "-INF", 0.f, false });
f_tests.push_back(string_checker<float>{ to_str(std::numeric_limits<float>::max()) + "0", 0.f, false });
f_tests.push_back(string_checker<float>{ to_str(std::numeric_limits<float>::min())+"0", 0.f, false });
f_tests.push_back(string_checker<float>{ to_str(std::numeric_limits<float>::lowest())+"0", 0.f, false });
f_tests.push_back(string_checker<float>{ "NaN", 0.f, false });
f_tests.push_back(string_checker<float>{ "abc", 0.f, false });

check_tests<float>(f_tests);

// double
std::vector<string_checker<double>> d_tests;
d_tests.push_back(string_checker<double>{ "1.2345", 1.2345, true });
d_tests.push_back(string_checker<double>{ "2.12121212", 2.12121212, true });
d_tests.push_back(string_checker<double>{ "-1.2345", -1.2345, true });
d_tests.push_back(string_checker<double>{ "-2.12121212", -2.12121212, true });
d_tests.push_back(string_checker<double>{ "INF", 0., false });
d_tests.push_back(string_checker<double>{ "-INF", 0., false });
d_tests.push_back(string_checker<double>{ to_str(std::numeric_limits<double>::max())+"0", 0., false });
d_tests.push_back(string_checker<double>{ to_str(std::numeric_limits<double>::min())+"0", 0., false });
d_tests.push_back(string_checker<double>{ to_str(std::numeric_limits<double>::lowest())+"0", 0., false });
d_tests.push_back(string_checker<double>{ "NaN", 0., false });
d_tests.push_back(string_checker<double>{ "abc", 0., false });

check_tests<double>(d_tests);

// long double
std::vector<string_checker<long double>> ld_tests;
ld_tests.push_back(string_checker<long double>{ "1.23456789", 1.23456789, true });
ld_tests.push_back(string_checker<long double>{ "-1.23456789", -1.23456789, true });
ld_tests.push_back(string_checker<long double>{ "2.12121212", 2.12121212, true });
ld_tests.push_back(string_checker<long double>{ "-2.12121212", -2.12121212, true });
ld_tests.push_back(string_checker<long double>{ "INF", 0., false });
ld_tests.push_back(string_checker<long double>{ "-INF", 0., false });
ld_tests.push_back(string_checker<long double>{ to_str(std::numeric_limits<long double>::max())+"0", 0., false });
ld_tests.push_back(string_checker<long double>{ to_str(std::numeric_limits<long double>::min())+"0", 0., false });
ld_tests.push_back(string_checker<long double>{ to_str(std::numeric_limits<long double>::lowest())+"0", 0., false });
ld_tests.push_back(string_checker<long double>{ "NaN", 0., false });
ld_tests.push_back(string_checker<long double>{ "abc", 0., false });

check_tests<long double>(ld_tests);
}

0 comments on commit 6399ca4

Please sign in to comment.