From 1b8d399de4367322356416915c6cc9b515278fbe Mon Sep 17 00:00:00 2001 From: Mike Farah Date: Sat, 8 Oct 2022 12:44:47 +1100 Subject: [PATCH] Detect and fail on missing closing brackets #1366 --- pkg/yqlib/expression_parser.go | 2 +- pkg/yqlib/expression_parser_test.go | 19 ++++++++++++------- pkg/yqlib/expression_postfix.go | 16 ++++++++++++---- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/pkg/yqlib/expression_parser.go b/pkg/yqlib/expression_parser.go index 28ac4f18..47226290 100644 --- a/pkg/yqlib/expression_parser.go +++ b/pkg/yqlib/expression_parser.go @@ -70,7 +70,7 @@ func (p *expressionParserImpl) createExpressionTree(postFixPath []*Operation) (* stack = append(stack, &newNode) } if len(stack) != 1 { - return nil, fmt.Errorf("Bad expression, please check expression syntax") + return nil, fmt.Errorf("bad expression, please check expression syntax") } return stack[0], nil } diff --git a/pkg/yqlib/expression_parser_test.go b/pkg/yqlib/expression_parser_test.go index 4c89d207..2718a16a 100644 --- a/pkg/yqlib/expression_parser_test.go +++ b/pkg/yqlib/expression_parser_test.go @@ -11,33 +11,38 @@ func getExpressionParser() ExpressionParserInterface { return ExpressionParser } +func TestParserNoMatchingCloseBracket(t *testing.T) { + _, err := getExpressionParser().ParseExpression(".cat | with(.;.bob") + test.AssertResultComplex(t, "bad expression - probably missing close bracket on WITH", err.Error()) +} + func TestParserNoMatchingCloseCollect(t *testing.T) { _, err := getExpressionParser().ParseExpression("[1,2") - test.AssertResultComplex(t, "Bad expression, could not find matching `]`", err.Error()) + test.AssertResultComplex(t, "bad expression, could not find matching `]`", err.Error()) } func TestParserNoMatchingCloseObjectInCollect(t *testing.T) { _, err := getExpressionParser().ParseExpression(`[{"b": "c"]`) - test.AssertResultComplex(t, "Bad expression, could not find matching `}`", err.Error()) + test.AssertResultComplex(t, "bad expression, could not find matching `}`", err.Error()) } func TestParserNoMatchingCloseInCollect(t *testing.T) { _, err := getExpressionParser().ParseExpression(`[(.a]`) - test.AssertResultComplex(t, "Bad expression, could not find matching `)`", err.Error()) + test.AssertResultComplex(t, "bad expression, could not find matching `)`", err.Error()) } func TestParserNoMatchingCloseCollectObject(t *testing.T) { _, err := getExpressionParser().ParseExpression(`{"a": "b"`) - test.AssertResultComplex(t, "Bad expression, could not find matching `}`", err.Error()) + test.AssertResultComplex(t, "bad expression, could not find matching `}`", err.Error()) } func TestParserNoMatchingCloseCollectInCollectObject(t *testing.T) { _, err := getExpressionParser().ParseExpression(`{"b": [1}`) - test.AssertResultComplex(t, "Bad expression, could not find matching `]`", err.Error()) + test.AssertResultComplex(t, "bad expression, could not find matching `]`", err.Error()) } func TestParserNoMatchingCloseBracketInCollectObject(t *testing.T) { _, err := getExpressionParser().ParseExpression(`{"b": (1}`) - test.AssertResultComplex(t, "Bad expression, could not find matching `)`", err.Error()) + test.AssertResultComplex(t, "bad expression, could not find matching `)`", err.Error()) } func TestParserNoArgsForTwoArgOp(t *testing.T) { @@ -72,5 +77,5 @@ func TestParserOneArgForOneArgOp(t *testing.T) { func TestParserExtraArgs(t *testing.T) { _, err := getExpressionParser().ParseExpression("sortKeys(.) explode(.)") - test.AssertResultComplex(t, "Bad expression, please check expression syntax", err.Error()) + test.AssertResultComplex(t, "bad expression, please check expression syntax", err.Error()) } diff --git a/pkg/yqlib/expression_postfix.go b/pkg/yqlib/expression_postfix.go index e875d8f9..fa7afa33 100644 --- a/pkg/yqlib/expression_postfix.go +++ b/pkg/yqlib/expression_postfix.go @@ -27,11 +27,11 @@ func popOpToResult(opStack []*token, result []*Operation) ([]*token, []*Operatio func validateNoOpenTokens(token *token) error { if token.TokenType == openCollect { - return fmt.Errorf(("Bad expression, could not find matching `]`")) + return fmt.Errorf(("bad expression, could not find matching `]`")) } else if token.TokenType == openCollectObject { - return fmt.Errorf(("Bad expression, could not find matching `}`")) + return fmt.Errorf(("bad expression, could not find matching `}`")) } else if token.TokenType == openBracket { - return fmt.Errorf(("Bad expression, could not find matching `)`")) + return fmt.Errorf(("bad expression, could not find matching `)`")) } return nil } @@ -104,7 +104,7 @@ func (p *expressionPostFixerImpl) ConvertToPostfix(infixTokens []*token) ([]*Ope opStack, result = popOpToResult(opStack, result) } if len(opStack) == 0 { - return nil, errors.New("Bad path expression, got close brackets without matching opening bracket") + return nil, errors.New("bad expression, got close brackets without matching opening bracket") } // now we should have ( as the last element on the opStack, get rid of it opStack = opStack[0 : len(opStack)-1] @@ -124,6 +124,14 @@ func (p *expressionPostFixerImpl) ConvertToPostfix(infixTokens []*token) ([]*Ope } log.Debugf("opstackLen: %v", len(opStack)) + if len(opStack) > 0 { + log.Debugf("opstack:") + for _, token := range opStack { + log.Debugf("- %v", token.toString(true)) + } + + return nil, fmt.Errorf("bad expression - probably missing close bracket on %v", opStack[len(opStack)-1].toString(false)) + } if log.IsEnabledFor(logging.DEBUG) { log.Debugf("PostFix Result:")