From 28ced1bc33f8ed6fa6e84fe5fb779d3f9836bab7 Mon Sep 17 00:00:00 2001 From: taler21 <99262969+taler21@users.noreply.github.com> Date: Tue, 16 Jan 2024 14:25:01 +0100 Subject: [PATCH] Make checks on character entry in numeric validators less relaxed Restrict the input to not allow values that are greater than the positive maximum or less than the negative minimum. In many cases it is not necessary to allow invalid characters to be entered. For example, if the specified range is 0 to 100. See #24220. --- include/wx/valnum.h | 45 +++++++++++++++++++++++++++++++++++-- src/common/valnum.cpp | 20 +++++++++-------- tests/validators/valnum.cpp | 24 +++++++++++++++----- 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index 6b05eeec58..5b7fe7cf5f 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -330,6 +330,15 @@ protected: virtual bool IsInRange(LongestValueType value) const = 0; + // Check whether the value is in the extended range allowed on input. + // + // Notice that the minimal and maximal values that have to be accepted on + // input may differ from the actually defined range to allow entering + // a temporarily invalid number because otherwise it would not be possible + // to enter any digits in an initially empty control limited to the values + // between "10" and "20". + virtual bool IsInInputRange(LongestValueType value) const = 0; + // Implement wxNumValidatorBase pure virtual method. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const override; @@ -379,6 +388,20 @@ public: virtual wxObject *Clone() const override { return new wxIntegerValidator(*this); } virtual bool IsInRange(LongestValueType value) const override + { + return IsValueInRange( value, this->GetMin(), this->GetMax() ); + } + + virtual bool IsInInputRange(LongestValueType value) const override + { + ValueType min = wxMin( 1, this->GetMin() ); + ValueType max = wxMax( 0, this->GetMax() ); + + return IsValueInRange( value, min, max ); + } + +private: + bool IsValueInRange(LongestValueType value, ValueType min, ValueType max ) const { // LongestValueType is used as a container for the values of any type // which can be used in type-independent wxIntegerValidatorBase code, @@ -393,10 +416,9 @@ public: return false; } - return this->GetMin() <= valueT && valueT <= this->GetMax(); + return min <= valueT && valueT <= max; } -private: wxDECLARE_NO_ASSIGN_DEF_COPY(wxIntegerValidator); }; @@ -448,6 +470,15 @@ protected: virtual bool IsInRange(LongestValueType value) const = 0; + // Check whether the value is in the extended range allowed on input. + // + // Notice that the minimal and maximal values that have to be accepted on + // input may differ from the actually defined range to allow entering + // a temporarily invalid number because otherwise it would not be possible + // to enter any digits in an initially empty control limited to the values + // between "10" and "20". + virtual bool IsInInputRange(LongestValueType value) const = 0; + // Implement wxNumValidatorBase pure virtual method. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const override; @@ -506,6 +537,16 @@ public: return this->GetMin() <= valueT && valueT <= this->GetMax(); } + virtual bool IsInInputRange(LongestValueType value) const override + { + const ValueType valueT = static_cast(value); + + ValueType min = wxMin( 0, this->GetMin() ); + ValueType max = wxMax( 0, this->GetMax() ); + + return min <= valueT && valueT <= max; + } + private: void DoSetMinMax() { diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp index c599dabec4..473f22510c 100644 --- a/src/common/valnum.cpp +++ b/src/common/valnum.cpp @@ -269,8 +269,8 @@ wxIntegerValidatorBase::FromString(const wxString& s, } bool -wxIntegerValidatorBase::IsCharOk(const wxString& WXUNUSED(val), - int WXUNUSED(pos), +wxIntegerValidatorBase::IsCharOk(const wxString& val, + int pos, wxChar ch) const { // We only accept digits here (remember that '-' is taken care of by the @@ -278,10 +278,13 @@ wxIntegerValidatorBase::IsCharOk(const wxString& WXUNUSED(val), if ( ch < '0' || ch > '9' ) return false; - // Accept anything that looks like a number here, notably do _not_ call - // IsInRange() because this would prevent entering any digits in an - // initially empty control limited to the values between "10" and "20". - return true; + // And the value after insertion needs to be at least in the range + // allowed on input. + LongestValueType value; + if ( !FromString(GetValueAfterInsertingChar(val, pos, ch), &value) ) + return false; + + return IsInInputRange(value); } // ============================================================================ @@ -360,9 +363,8 @@ wxFloatingPointValidatorBase::IsCharOk(const wxString& val, if ( posSep != wxString::npos && newval.length() - posSep - 1 > m_precision ) return false; - // Note that we do _not_ check if it's in range here, see the comment in - // wxIntegerValidatorBase::IsCharOk(). - return true; + // Finally check whether it is at least in the range allowed on input. + return IsInInputRange(value); } #endif // wxUSE_VALIDATORS && wxUSE_TEXTCTRL diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index 1702d30486..e688fe4e93 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -338,29 +338,41 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") // Also test the range constraint. + valFloat.SetRange(10., 20.); + text2->SetValidator(valFloat); text2->Clear(); + // Entering a value which is out of range but within + // the extended input range is allowed. sim.Char('9'); wxYield(); CHECK( text2->GetValue() == "9" ); - // Entering a value which is out of range is allowed. + // Entering a value greater than the positive range maximum + // is not allowed. sim.Char('9'); wxYield(); - CHECK( text2->GetValue() == "99" ); + CHECK( text2->GetValue() == "9" ); - // But it must be clamped to the valid range on focus loss. + // A value that is out of range but within the extended input + // range must be clamped to the valid range on focus loss. m_text->SetFocus(); wxYield(); CHECK( text2->GetValue() == "10.000" ); - // Repeat the test with a too small invalid value. + // Repeat the test with a negative invalid value. + valFloat.SetRange(-20., -10.); + text2->SetValidator(valFloat); text2->Clear(); text2->SetFocus(); - sim.Text("-22"); + sim.Text("-2"); wxYield(); - CHECK( text2->GetValue() == "-22" ); + CHECK( text2->GetValue() == "-2" ); + + sim.Char('2'); + wxYield(); + CHECK( text2->GetValue() == "-2" ); m_text->SetFocus(); wxYield();