From ce78925732f17a3628a356133cbd75b71fc79c57 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 3 Jul 2018 13:55:33 +0300 Subject: [PATCH] JDBC: Fix stackoverflow on getObject and timestamp conversion (#31735) StackOverflowError fix in JdbcResultSet getObject method. Fix Timestamp conversion bug when getting the value of a time column. --- .../xpack/sql/jdbc/jdbc/JdbcResultSet.java | 2 +- .../xpack/sql/jdbc/jdbc/TypeConverter.java | 2 +- .../sql/jdbc/jdbc/TypeConverterTests.java | 5 +- .../qa/sql/jdbc/JdbcIntegrationTestCase.java | 6 +- .../xpack/qa/sql/jdbc/ResultSetTestCase.java | 82 +++++++++++++++++++ 5 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java index 351ac73a88f..201ae251ca0 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java @@ -344,7 +344,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper { throw new SQLException("type is null"); } - return getObject(columnIndex, type); + return convert(columnIndex, type); } private T convert(int columnIndex, Class type) throws SQLException { diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java index 1e24a03c8b3..782a17257d4 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java @@ -254,7 +254,7 @@ final class TypeConverter { case REAL: return floatValue(v); // Float might be represented as string for infinity and NaN values case TIMESTAMP: - return ((Number) v).longValue(); + return new Timestamp(((Number) v).longValue()); default: throw new SQLException("Unexpected column type [" + columnType.getName() + "]"); diff --git a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java index 0182ea63f63..51c130a3911 100644 --- a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java +++ b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java @@ -14,6 +14,7 @@ import org.joda.time.DateTime; import org.joda.time.ReadableDateTime; import java.sql.JDBCType; +import java.sql.Timestamp; import static org.hamcrest.Matchers.instanceOf; @@ -41,8 +42,8 @@ public class TypeConverterTests extends ESTestCase { public void testTimestampAsNative() throws Exception { DateTime now = DateTime.now(); - assertThat(convertAsNative(now, JDBCType.TIMESTAMP), instanceOf(Long.class)); - assertEquals(now.getMillis(), convertAsNative(now, JDBCType.TIMESTAMP)); + assertThat(convertAsNative(now, JDBCType.TIMESTAMP), instanceOf(Timestamp.class)); + assertEquals(now.getMillis(), ((Timestamp) convertAsNative(now, JDBCType.TIMESTAMP)).getTime()); } private Object convertAsNative(Object value, JDBCType type) throws Exception { diff --git a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java index a2b524c20b0..a339222445a 100644 --- a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java +++ b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java @@ -82,7 +82,11 @@ public abstract class JdbcIntegrationTestCase extends ESRestTestCase { } public static void index(String index, CheckedConsumer body) throws IOException { - Request request = new Request("PUT", "/" + index + "/doc/1"); + index(index, "1", body); + } + + public static void index(String index, String documentId, CheckedConsumer body) throws IOException { + Request request = new Request("PUT", "/" + index + "/doc/" + documentId); request.addParameter("refresh", "true"); XContentBuilder builder = JsonXContent.contentBuilder().startObject(); body.accept(builder); diff --git a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java new file mode 100644 index 00000000000..861a6dccaba --- /dev/null +++ b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.qa.sql.jdbc; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import java.sql.Timestamp; +import java.util.Date; + +public class ResultSetTestCase extends JdbcIntegrationTestCase { + public void testGettingTimestamp() throws Exception { + long randomMillis = randomLongBetween(0, System.currentTimeMillis()); + + index("library", "1", builder -> { + builder.field("name", "Don Quixote"); + builder.field("page_count", 1072); + builder.timeField("release_date", new Date(randomMillis)); + builder.timeField("republish_date", null); + }); + index("library", "2", builder -> { + builder.field("name", "1984"); + builder.field("page_count", 328); + builder.timeField("release_date", new Date(-649036800000L)); + builder.timeField("republish_date", new Date(599616000000L)); + }); + + try (Connection connection = esJdbc()) { + try (PreparedStatement statement = connection.prepareStatement("SELECT name, release_date, republish_date FROM library")) { + try (ResultSet results = statement.executeQuery()) { + ResultSetMetaData resultSetMetaData = results.getMetaData(); + + results.next(); + assertEquals(3, resultSetMetaData.getColumnCount()); + assertEquals(randomMillis, results.getTimestamp("release_date").getTime()); + assertEquals(randomMillis, results.getTimestamp(2).getTime()); + assertTrue(results.getObject(2) instanceof Timestamp); + assertEquals(randomMillis, ((Timestamp) results.getObject("release_date")).getTime()); + + assertNull(results.getTimestamp(3)); + assertNull(results.getObject("republish_date")); + + assertTrue(results.next()); + assertEquals(599616000000L, results.getTimestamp("republish_date").getTime()); + assertEquals(-649036800000L, ((Timestamp) results.getObject(2)).getTime()); + + assertFalse(results.next()); + } + } + } + } + + /* + * Checks StackOverflowError fix for https://github.com/elastic/elasticsearch/pull/31735 + */ + public void testNoInfiniteRecursiveGetObjectCalls() throws SQLException, IOException { + index("library", "1", builder -> { + builder.field("name", "Don Quixote"); + builder.field("page_count", 1072); + }); + Connection conn = esJdbc(); + PreparedStatement statement = conn.prepareStatement("SELECT * FROM library"); + ResultSet results = statement.executeQuery(); + + try { + results.next(); + results.getObject("name"); + results.getObject("page_count"); + results.getObject(1); + results.getObject(1, String.class); + results.getObject("page_count", Integer.class); + } catch (StackOverflowError soe) { + fail("Infinite recursive call on getObject() method"); + } + } +}