diff --git a/hist/hist/src/TFormula.cxx b/hist/hist/src/TFormula.cxx index 3a1df9b26d918..b248fc1bc9878 100644 --- a/hist/hist/src/TFormula.cxx +++ b/hist/hist/src/TFormula.cxx @@ -22,13 +22,13 @@ #include "ROOT/StringUtils.hxx" -#include #include #include #include #include #include #include +#include #include #include @@ -49,6 +49,43 @@ std::string doubleToString(double val) return ss.str(); } +bool areMatching(char opening, char closing) +{ + return (opening == '(' && closing == ')') || (opening == '{' && closing == '}') || + (opening == '[' && closing == ']') || (opening == '\"' && closing == '\"'); +} + +bool isBalanced(const TString &s) +{ + std::stack i; + auto sLength = s.Length(); + for (Ssiz_t c = 0; c < sLength; ++c) { + if (s[c] == '[' || s[c] == '{' || s[c] == '(') { + i.push(s[c]); + } else if (s[c] == ']' || s[c] == '}' || s[c] == ')') { + if (i.empty() || !areMatching(i.top(), s[c])) { + Error("TFormula", "Found unbalanced char '%c' (expected closing of '%c') at index %d of '%s'", s[c], i.top(), c, + s.Data()); + return false; + } else + i.pop(); + } else if (s[c] == '\"') { + if (i.empty()) + i.push(s[c]); + else if (areMatching(i.top(), s[c])) + i.pop(); + else { + i.push(s[c]); + } + } + } + if (!i.empty()) { + Error("TFormula", "String '%s' with %zu unbalanced chars.", s.Data(), i.size()); + return false; + } + return true; +} + // In the interpreter, we must match the SIMD width used by compiled ROOT. // ROOT::Double_v aliases the best available native SIMD type, which may differ // between compiled and interpreted contexts (e.g. with -march=native). @@ -374,9 +411,9 @@ Bool_t TFormula::IsHexadecimal(const TString & formula, int i) // } return false; } + //////////////////////////////////////////////////////////////////////////// -// check is given position is in a parameter name i.e. within "[ ]" -//// +/// check if given position is in a parameter name i.e. within "[ ]" Bool_t TFormula::IsAParameterName(const TString & formula, int pos) { Bool_t foundOpenParenthesis = false; @@ -1250,7 +1287,7 @@ void TFormula::HandleParametrizedFunctions(TString &formula) // check if function has specified the [...] e.g. gaus[x,y] Int_t openingBracketPos = funPos + funName.Length() + (isNormalized ? 1 : 0); Int_t closingBracketPos = kNPOS; - if (openingBracketPos > formula.Length() || formula[openingBracketPos] != '[') { + if (openingBracketPos >= formula.Length() || formula[openingBracketPos] != '[') { dim = funDim; variables.resize(dim); for (Int_t idim = 0; idim < dim; ++idim) @@ -1428,15 +1465,15 @@ void TFormula::HandleFunctionArguments(TString &formula) // ignore things that start with square brackets if (formula[i] == '[') { - while (formula[i] != ']') + while (i < formula.Length() && formula[i] != ']') i++; continue; } // ignore strings - if (formula[i] == '\"') { + if (i < formula.Length() && formula[i] == '\"') { do i++; - while (formula[i] != '\"'); + while (i < formula.Length() && formula[i] != '\"'); continue; } // ignore numbers (scientific notation) @@ -1444,13 +1481,13 @@ void TFormula::HandleFunctionArguments(TString &formula) continue; // ignore x in hexadecimal number if (IsHexadecimal(formula, i)) { - while (!IsOperator(formula[i]) && i < formula.Length()) + while (i < formula.Length() && !IsOperator(formula[i])) i++; continue; } // investigate possible start of function name - if (isalpha(formula[i]) && !IsOperator(formula[i])) { + if (i < formula.Length() && isalpha(formula[i]) && !IsOperator(formula[i])) { // std::cout << "character : " << i << " " << formula[i] << " is not an operator and is alpha" << std::endl; int j; // index to end of name @@ -1796,7 +1833,7 @@ void TFormula::HandleExponentiation(TString &formula) //////////////////////////////////////////////////////////////////////////////// -/// Handle linear functions defined with the operator ++. +/// Handle linear functions defined with the operator ++ (@ is the shorthand). void TFormula::HandleLinear(TString &formula) { @@ -1808,13 +1845,15 @@ void TFormula::HandleLinear(TString &formula) TString expandedFormula = ""; int delimeterPos = 0; for(std::size_t iTerm = 0; iTerm < terms.size(); ++iTerm) { - // determine the position of the "@" operator in the formula - delimeterPos += terms[iTerm].size() + (iTerm == 0); - if(IsAParameterName(formula, delimeterPos)) { - // append the current term and the remaining formula unchanged to the expanded formula - expandedFormula += terms[iTerm]; - expandedFormula += formula(delimeterPos, formula.Length() - (delimeterPos + 1)); - break; + if (iTerm < terms.size() - 1) { // N terms, N - 1 @ + // determine the position of the "@" operator in the formula + delimeterPos += terms[iTerm].size() + iTerm; + if(IsAParameterName(formula, delimeterPos)) { + // append the current term and the remaining formula unchanged to the expanded formula + expandedFormula += terms[iTerm]; + expandedFormula += formula(delimeterPos, formula.Length() - delimeterPos); + break; + } } SetBit(kLinear, true); auto termName = std::string("__linear") + std::to_string(iTerm+1); @@ -1829,7 +1868,7 @@ void TFormula::HandleLinear(TString &formula) //////////////////////////////////////////////////////////////////////////////// /// Preprocessing of formula -/// Replace all ** by ^, and removes spaces. +/// Replace all ** by ^, all ++ by @, and removes spaces. /// Handle also parametrized functions like polN,gaus,expo,landau /// and exponentiation. /// Similar functionality should be added here. @@ -1844,7 +1883,7 @@ void TFormula::PreProcessFormula(TString &formula) HandleParamRanges(formula); HandleFunctionArguments(formula); HandleExponentiation(formula); - // "++" wil be dealt with Handle Linear + // "++" (now @ since they we just called ReplaceAll a few lines above) wil be dealt with Handle Linear HandleLinear(formula); // special case for "--" and "++" // ("++" needs to be written with whitespace that is removed before but then we re-add it again @@ -1860,6 +1899,12 @@ Bool_t TFormula::PrepareFormula(TString &formula) { fFuncs.clear(); fReadyToExecute = false; + + // Check for balanced parentheses + if (!isBalanced(formula)) { + return false; + } + ExtractFunctors(formula); // update the expression with the new formula @@ -1956,7 +2001,7 @@ void TFormula::ExtractFunctors(TString &formula) // look for next instance of "\" do { i++; - } while (formula[i] != '\"'); + } while (i < formula.Length() && formula[i] != '\"'); } // case of e or E for numbers in exponential notation (e.g. 2.2e-3) if (IsScientificNotation(formula, i)) @@ -1965,7 +2010,7 @@ void TFormula::ExtractFunctors(TString &formula) if (IsHexadecimal(formula, i)) { // find position of operator // do not check cases if character is not only a to f, but accept anything - while (!IsOperator(formula[i]) && i < formula.Length()) { + while (i < formula.Length() && !IsOperator(formula[i])) { i++; } continue; @@ -1997,7 +2042,7 @@ void TFormula::ExtractFunctors(TString &formula) // printf(" build a name %s \n",name.Data() ); if (formula[i] == '(') { i++; - if (formula[i] == ')') { + if (i < formula.Length() && formula[i] == ')') { fFuncs.push_back(TFormulaFunction(name, body, 0)); name = body = ""; continue; @@ -3622,8 +3667,8 @@ TString TFormula::GetExpFormula(Option_t *option) const // look for p[number if (clingFormula[i] == 'p' && clingFormula[i+1] == '[' && isdigit(clingFormula[i+2]) ) { int j = i+3; - while ( isdigit(clingFormula[j]) ) { j++;} - if (clingFormula[j] != ']') { + while ( j < clingFormula.Length() && isdigit(clingFormula[j]) ) { j++;} + if ( j >= clingFormula.Length() || clingFormula[j] != ']') { Error("GetExpFormula","Parameters not found - invalid expression - return default cling formula"); return clingFormula; } @@ -3647,8 +3692,8 @@ TString TFormula::GetExpFormula(Option_t *option) const // look for [parName] if (expFormula[i] == '[') { int j = i+1; - while ( expFormula[j] != ']' ) { j++;} - if (expFormula[j] != ']') { + while ( j < expFormula.Length() && expFormula[j] != ']' ) { j++;} + if ( j >= expFormula.Length() || expFormula[j] != ']') { Error("GetExpFormula","Parameter names not found - invalid expression - return default formula"); return expFormula; } diff --git a/hist/hist/test/test_TFormula.cxx b/hist/hist/test/test_TFormula.cxx index 8d0650905d766..288af5f8cb936 100644 --- a/hist/hist/test/test_TFormula.cxx +++ b/hist/hist/test/test_TFormula.cxx @@ -98,3 +98,17 @@ TEST(TFormulaPolTest, CompoundExpressions) TFormula f1("f1", "pol1(x,0) + pol1(y,2)"); EXPECT_EQ(f1.GetExpFormula(), TString("([p0]+[p1]*x)+([p2]+[p3]*y)")); } + +// https://github.com/root-project/root/issues/21104 +TEST(TFormula, SingleOpeningBracket) +{ + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TFormula", "String '[' with 1 unbalanced chars."); + TFormula f1("f1", "["); + EXPECT_FALSE(f1.IsValid()); + EXPECT_EQ(f1.GetNdim(), 0); + diags.requiredDiag(kError, "TFormula", "String '(' with 1 unbalanced chars."); + TFormula f2("f2", "("); + EXPECT_FALSE(f2.IsValid()); + EXPECT_EQ(f2.GetNdim(), 0); +}