From 91e45ca21b9490fe46d39aa6240a289a59304ab8 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Tue, 18 Sep 2018 18:51:48 +0300 Subject: [PATCH] SQL: Better handling of number parsing exceptions (#33776) Add proper exceptions in case the parsing of numbers (too large, invalid format) fails. Close #33622 --- .../xpack/sql/parser/ExpressionBuilder.java | 42 ++++++++++++++++--- .../xpack/sql/parser/ExpressionTests.java | 19 +++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java index 0c7ecbc7ddf..2719d39bbec 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java @@ -95,7 +95,7 @@ import org.joda.time.format.DateTimeFormatter; import org.joda.time.format.DateTimeFormatterBuilder; import org.joda.time.format.ISODateTimeFormat; -import java.math.BigDecimal; +import java.math.BigInteger; import java.util.List; import java.util.Locale; import java.util.Map; @@ -458,7 +458,13 @@ abstract class ExpressionBuilder extends IdentifierBuilder { @Override public Expression visitBooleanLiteral(BooleanLiteralContext ctx) { - return new Literal(source(ctx), Booleans.parseBoolean(ctx.getText().toLowerCase(Locale.ROOT), false), DataType.BOOLEAN); + boolean value; + try { + value = Booleans.parseBoolean(ctx.getText().toLowerCase(Locale.ROOT), false); + } catch(IllegalArgumentException iae) { + throw new ParsingException(source(ctx), iae.getMessage()); + } + return new Literal(source(ctx), Boolean.valueOf(value), DataType.BOOLEAN); } @Override @@ -472,14 +478,40 @@ abstract class ExpressionBuilder extends IdentifierBuilder { @Override public Literal visitDecimalLiteral(DecimalLiteralContext ctx) { - return new Literal(source(ctx), new BigDecimal(ctx.getText()).doubleValue(), DataType.DOUBLE); + double value; + try { + value = Double.parseDouble(ctx.getText()); + } catch (NumberFormatException nfe) { + throw new ParsingException(source(ctx), "Cannot parse number [{}]", ctx.getText()); + } + if (Double.isInfinite(value)) { + throw new ParsingException(source(ctx), "Number [{}] is too large", ctx.getText()); + } + if (Double.isNaN(value)) { + throw new ParsingException(source(ctx), "[{}] cannot be parsed as a number (NaN)", ctx.getText()); + } + return new Literal(source(ctx), Double.valueOf(value), DataType.DOUBLE); } @Override public Literal visitIntegerLiteral(IntegerLiteralContext ctx) { - BigDecimal bigD = new BigDecimal(ctx.getText()); + long value; + try { + value = Long.parseLong(ctx.getText()); + } catch (NumberFormatException nfe) { + try { + BigInteger bi = new BigInteger(ctx.getText()); + try { + bi.longValueExact(); + } catch (ArithmeticException ae) { + throw new ParsingException(source(ctx), "Number [{}] is too large", ctx.getText()); + } + } catch (NumberFormatException ex) { + // parsing fails, go through + } + throw new ParsingException(source(ctx), "Cannot parse number [{}]", ctx.getText()); + } - long value = bigD.longValueExact(); DataType type = DataType.LONG; // try to downsize to int if possible (since that's the most common type) if ((int) value == value) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java index 004118e8cd2..466e749c9a3 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java @@ -22,6 +22,15 @@ public class ExpressionTests extends ESTestCase { assertEquals("LEFT", uf.functionName()); } + + public void testLiteralBoolean() throws Exception { + Expression lt = parser.createExpression("TRUE"); + assertEquals(Literal.class, lt.getClass()); + Literal l = (Literal) lt; + assertEquals(Boolean.TRUE, l.value()); + assertEquals(DataType.BOOLEAN, l.dataType()); + } + public void testLiteralDouble() throws Exception { Expression lt = parser.createExpression(String.valueOf(Double.MAX_VALUE)); assertEquals(Literal.class, lt.getClass()); @@ -92,4 +101,14 @@ public class ExpressionTests extends ESTestCase { assertEquals(Integer.valueOf(Byte.MAX_VALUE), l.value()); assertEquals(DataType.INTEGER, l.dataType()); } + + public void testLiteralIntegerInvalid() throws Exception { + ParsingException ex = expectThrows(ParsingException.class, () -> parser.createExpression("123456789098765432101")); + assertEquals("Number [123456789098765432101] is too large", ex.getErrorMessage()); + } + + public void testLiteralDecimalTooBig() throws Exception { + ParsingException ex = expectThrows(ParsingException.class, () -> parser.createExpression("1.9976931348623157e+308")); + assertEquals("Number [1.9976931348623157e+308] is too large", ex.getErrorMessage()); + } } \ No newline at end of file