From 5e2ef13c640d1d9c0aa926e61e692582d8529c83 Mon Sep 17 00:00:00 2001 From: Matthew J Greenwood Date: Mon, 14 Dec 2015 09:05:57 -0500 Subject: [PATCH] added negative test cases - removed readPosition from function parameter parser, ready for review --- .../jsonpath/internal/path/PathCompiler.java | 49 ++++++++++++------- .../internal/function/NestedFunctionTest.java | 45 +++++++++++++++++ 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/path/PathCompiler.java b/json-path/src/main/java/com/jayway/jsonpath/internal/path/PathCompiler.java index 71586743..4deebc2a 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/path/PathCompiler.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/path/PathCompiler.java @@ -204,8 +204,10 @@ public class PathCompiler { // read the next token to determine if we have a simple no-args function call char c = path.charAt(readPosition + 1); if (c != CLOSE_PARENTHESIS) { + path.setPosition(endPosition+1); // parse the arguments of the function - arguments that are inner queries or JSON document(s) - functionParameters = parseFunctionParameters(readPosition); + String functionName = path.subSequence(startPosition, endPosition).toString(); + functionParameters = parseFunctionParameters(functionName); } else { path.setPosition(readPosition + 1); } @@ -252,14 +254,12 @@ public class PathCompiler { * * The above is a valid function call, we're first summing 10 + avg of 1...10 (5.5) so the total should be 15.5 * - * @param readPosition - * The current position within the stream we've advanced - TODO remove the need for this... * @return * An ordered list of parameters that are to processed via the function. Typically functions either process * an array of values and/or can consume parameters in addition to the values provided from the consumption of * an array. */ - private List parseFunctionParameters(int readPosition) { + private List parseFunctionParameters(String funcName) { PathToken currentToken; ParamType type = null; @@ -270,8 +270,9 @@ public class PathCompiler { char priorChar = 0; List parameters = new ArrayList(); StringBuffer parameter = new StringBuffer(); - while (path.inBounds(readPosition) && !endOfStream) { - char c = path.charAt(readPosition++); + while (path.inBounds() && !endOfStream) { + char c = path.currentChar(); + path.incrementPosition(1); // we're at the start of the stream, and don't know what type of parameter we have if (type == null) { @@ -290,6 +291,9 @@ public class PathCompiler { switch (c) { case DOUBLE_QUOTE: if (priorChar != '\\' && groupQuote > 0) { + if (groupQuote == 0) { + throw new InvalidPathException("Unexpected quote '\"' at character position: " + path.position()); + } groupQuote--; } else { @@ -307,9 +311,15 @@ public class PathCompiler { break; case CLOSE_BRACE: + if (0 == groupBrace) { + throw new InvalidPathException("Unexpected close brace '}' at character position: " + path.position()); + } groupBrace--; break; case CLOSE_SQUARE_BRACKET: + if (0 == groupBracket) { + throw new InvalidPathException("Unexpected close bracket ']' at character position: " + path.position()); + } groupBracket--; break; @@ -323,26 +333,27 @@ public class PathCompiler { case COMMA: // In this state we've reach the end of a function parameter and we can pass along the parameter string // to the parser - if ((0 == groupQuote && 0 == groupBrace && 0 == groupBracket && ((0 == groupParen && CLOSE_PARENTHESIS == c) || 1 == groupParen))) { + if ((0 == groupQuote && 0 == groupBrace && 0 == groupBracket + && ((0 == groupParen && CLOSE_PARENTHESIS == c) || 1 == groupParen))) { endOfStream = (0 == groupParen); if (null != type) { Parameter param = null; - if (type == ParamType.JSON) { - // parse the json and set the value - param = new Parameter(parameter.toString()); - } else if (type == ParamType.PATH) { - LinkedList predicates = new LinkedList(); - PathCompiler compiler = new PathCompiler(parameter.toString(), predicates); - param = new Parameter(compiler.compile()); + switch (type) { + case JSON: + // parse the json and set the value + param = new Parameter(parameter.toString()); + break; + case PATH: + LinkedList predicates = new LinkedList(); + PathCompiler compiler = new PathCompiler(parameter.toString(), predicates); + param = new Parameter(compiler.compile()); + break; } if (null != param) { parameters.add(param); - } else { - // TODO: raise error... } parameter.delete(0, parameter.length()); - type = null; } } @@ -354,7 +365,9 @@ public class PathCompiler { } priorChar = c; } - path.setPosition(readPosition); + if (0 != groupBrace || 0 != groupParen || 0 != groupBracket) { + throw new InvalidPathException("Arguments to function: '" + funcName + "' are not closed properly."); + } return parameters; } diff --git a/json-path/src/test/java/com/jayway/jsonpath/internal/function/NestedFunctionTest.java b/json-path/src/test/java/com/jayway/jsonpath/internal/function/NestedFunctionTest.java index 1b36b362..bed77869 100644 --- a/json-path/src/test/java/com/jayway/jsonpath/internal/function/NestedFunctionTest.java +++ b/json-path/src/test/java/com/jayway/jsonpath/internal/function/NestedFunctionTest.java @@ -8,6 +8,9 @@ import org.junit.runners.Parameterized; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.jayway.jsonpath.JsonPath.using; +import static org.junit.Assert.assertTrue; + /** * Created by matt@mjgreenwood.net on 12/10/15. */ @@ -74,4 +77,46 @@ public class NestedFunctionTest extends BaseFunctionTest { public void testAppendNumber() { verifyMathFunction(conf, "$.numbers.append(11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 0).avg()", 10.0); } + + /** + * Aggregation function should ignore text values + */ + @Test + public void testAppendTextAndNumberThenSum() { + verifyMathFunction(conf, "$.numbers.append(\"0\", \"11\").sum()", 55.0); + } + + @Test + public void testErrantCloseBraceNegative() { + try { + using(conf).parse(this.NUMBER_SERIES).read("$.numbers.append(0, 1, 2}).avg()"); + assert(false); + } + catch (Exception e) { + assertTrue(e.getMessage().startsWith("Unexpected close brace")); + } + } + + @Test + public void testErrantCloseBracketNegative() { + try { + using(conf).parse(this.NUMBER_SERIES).read("$.numbers.append(0, 1, 2]).avg()"); + assert(false); + } + catch (Exception e) { + assertTrue(e.getMessage().startsWith("Unexpected close bracket")); + } + } + + @Test + public void testUnclosedFunctionCallNegative() { + try { + using(conf).parse(this.NUMBER_SERIES).read("$.numbers.append(0, 1, 2"); + assert(false); + } + catch (Exception e) { + assertTrue(e.getMessage().startsWith("Arguments to function: 'append'")); + } + } + }