Skip to content

Commit

Permalink
SNOW-1003775: Fix parsing large string values with jackson databind >…
Browse files Browse the repository at this point in the history
… 2.15 (#1613)
  • Loading branch information
sfc-gh-ext-simba-jl authored Feb 12, 2024
1 parent e0f4fa2 commit e899319
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 35 deletions.
34 changes: 6 additions & 28 deletions src/main/java/net/snowflake/client/core/HttpUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,14 @@ public class HttpUtil {
@SnowflakeJdbcInternalApi
public static Duration getConnectionTimeout() {
return Duration.ofMillis(
convertSystemPropertyToIntValue(
SystemUtil.convertSystemPropertyToIntValue(
JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY, DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT_IN_MS));
}

@SnowflakeJdbcInternalApi
public static Duration getSocketTimeout() {
return Duration.ofMillis(
convertSystemPropertyToIntValue(
SystemUtil.convertSystemPropertyToIntValue(
JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY, DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT_IN_MS));
}

Expand Down Expand Up @@ -305,7 +305,7 @@ public static CloseableHttpClient buildHttpClient(
@Nullable HttpClientSettingsKey key, File ocspCacheFile, boolean downloadUnCompressed) {
// set timeout so that we don't wait forever.
// Setup the default configuration for all requests on this client
int timeToLive = convertSystemPropertyToIntValue(JDBC_TTL, DEFAULT_TTL);
int timeToLive = SystemUtil.convertSystemPropertyToIntValue(JDBC_TTL, DEFAULT_TTL);
logger.debug("time to live in connection pooling manager: {}", timeToLive);
long connectTimeout = getConnectionTimeout().toMillis();
long socketTimeout = getSocketTimeout().toMillis();
Expand Down Expand Up @@ -373,9 +373,10 @@ public static CloseableHttpClient buildHttpClient(
new PoolingHttpClientConnectionManager(
registry, null, null, null, timeToLive, TimeUnit.SECONDS);
int maxConnections =
convertSystemPropertyToIntValue(JDBC_MAX_CONNECTIONS_PROPERTY, DEFAULT_MAX_CONNECTIONS);
SystemUtil.convertSystemPropertyToIntValue(
JDBC_MAX_CONNECTIONS_PROPERTY, DEFAULT_MAX_CONNECTIONS);
int maxConnectionsPerRoute =
convertSystemPropertyToIntValue(
SystemUtil.convertSystemPropertyToIntValue(
JDBC_MAX_CONNECTIONS_PER_ROUTE_PROPERTY, DEFAULT_MAX_CONNECTIONS_PER_ROUTE);
logger.debug(
"Max connections total in connection pooling manager: {}; max connections per route: {}",
Expand Down Expand Up @@ -901,29 +902,6 @@ public Socket createSocket(HttpContext ctx) throws IOException {
}
}

/**
* Helper function to convert system properties to integers
*
* @param systemProperty name of the system property
* @param defaultValue default value used
* @return the value of the system property, else the default value
*/
static int convertSystemPropertyToIntValue(String systemProperty, int defaultValue) {
String systemPropertyValue = systemGetProperty(systemProperty);
int returnVal = defaultValue;
if (systemPropertyValue != null) {
try {
returnVal = Integer.parseInt(systemPropertyValue);
} catch (NumberFormatException ex) {
logger.info(
"Failed to parse the system parameter {} with value {}",
systemProperty,
systemPropertyValue);
}
}
return returnVal;
}

/**
* Helper function to attach additional headers to a request if present. This takes a (nullable)
* map of headers in <name,value> format and adds them to the incoming request using addHeader.
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/net/snowflake/client/core/ObjectMapperFactory.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,38 @@
package net.snowflake.client.core;

import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import net.snowflake.client.log.SFLogger;
import net.snowflake.client.log.SFLoggerFactory;

/**
* Factor method used to create ObjectMapper instance. All object mapper in JDBC should be created
* by this method.
*/
public class ObjectMapperFactory {
@SnowflakeJdbcInternalApi
// Snowflake allows up to 16M string size and returns base64 encoded value that makes it up to 23M
public static final int DEFAULT_MAX_JSON_STRING_LEN = 23_000_000;

public static final String MAX_JSON_STRING_LENGTH_JVM =
"net.snowflake.jdbc.objectMapper.maxJsonStringLength";

private static final SFLogger logger = SFLoggerFactory.getLogger(ObjectMapperFactory.class);

public static ObjectMapper getObjectMapper() {
ObjectMapper mapper = new ObjectMapper();
mapper.configure(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS, false);
mapper.configure(MapperFeature.CAN_OVERRIDE_ACCESS_MODIFIERS, false);

// override the maxStringLength value in ObjectMapper
int maxJsonStringLength =
SystemUtil.convertSystemPropertyToIntValue(
MAX_JSON_STRING_LENGTH_JVM, DEFAULT_MAX_JSON_STRING_LEN);
mapper
.getFactory()
.setStreamReadConstraints(
StreamReadConstraints.builder().maxStringLength(maxJsonStringLength).build());
return mapper;
}
}
3 changes: 1 addition & 2 deletions src/main/java/net/snowflake/client/core/SFSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public class SFSession extends SFBaseSession {
// Need to be removed when a better way to organize session parameter is introduced.
private static final String CLIENT_STORE_TEMPORARY_CREDENTIAL =
"CLIENT_STORE_TEMPORARY_CREDENTIAL";
private static final ObjectMapper mapper = ObjectMapperFactory.getObjectMapper();
private static final int MAX_SESSION_PARAMETERS = 1000;
// this constant was public - let's not change it
public static final int DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT =
Expand Down Expand Up @@ -931,7 +930,7 @@ protected void heartbeat() throws SFException, SQLException {

logger.debug("connection heartbeat response: {}", theResponse);

rootNode = mapper.readTree(theResponse);
rootNode = OBJECT_MAPPER.readTree(theResponse);

// check the response to see if it is session expiration response
if (rootNode != null
Expand Down
37 changes: 37 additions & 0 deletions src/main/java/net/snowflake/client/core/SystemUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2012-2024 Snowflake Computing Inc. All rights reserved.
*/

package net.snowflake.client.core;

import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;

import net.snowflake.client.log.SFLogger;
import net.snowflake.client.log.SFLoggerFactory;

class SystemUtil {
private static final SFLogger logger = SFLoggerFactory.getLogger(SystemUtil.class);

/**
* Helper function to convert system properties to integers
*
* @param systemProperty name of the system property
* @param defaultValue default value used
* @return the value of the system property, else the default value
*/
static int convertSystemPropertyToIntValue(String systemProperty, int defaultValue) {
String systemPropertyValue = systemGetProperty(systemProperty);
int returnVal = defaultValue;
if (systemPropertyValue != null) {
try {
returnVal = Integer.parseInt(systemPropertyValue);
} catch (NumberFormatException ex) {
logger.info(
"Failed to parse the system parameter {} with value {}",
systemProperty,
systemPropertyValue);
}
}
return returnVal;
}
}
92 changes: 92 additions & 0 deletions src/test/java/net/snowflake/client/core/ObjectMapperTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright (c) 2012-2024 Snowflake Computing Inc. All rights reserved.
*/

package net.snowflake.client.core;

import static org.junit.Assert.assertEquals;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.nio.charset.StandardCharsets;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collection;
import java.util.List;
import net.snowflake.client.jdbc.SnowflakeUtil;
import org.junit.After;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(Parameterized.class)
public class ObjectMapperTest {
private static final int jacksonDefaultMaxStringLength = 20_000_000;

@Parameterized.Parameters(name = "lobSizeInMB={0}, maxJsonStringLength={1}")
public static Collection<Object[]> data() {
int[] lobSizeInMB = new int[] {16, 16, 32, 64, 128};
// maxJsonStringLength to be set for the corresponding LOB size
int[] maxJsonStringLengths =
new int[] {jacksonDefaultMaxStringLength, 23_000_000, 45_000_000, 90_000_000, 180_000_000};
List<Object[]> ret = new ArrayList<>();
for (int i = 0; i < lobSizeInMB.length; i++) {
ret.add(new Object[] {lobSizeInMB[i], maxJsonStringLengths[i]});
}
return ret;
}

private final int lobSizeInBytes;
private final int maxJsonStringLength;

@After
public void clearProperty() {
System.clearProperty(ObjectMapperFactory.MAX_JSON_STRING_LENGTH_JVM);
}

public ObjectMapperTest(int lobSizeInMB, int maxJsonStringLength) {
// convert LOB size from MB to bytes
this.lobSizeInBytes = lobSizeInMB * 1024 * 1024;
this.maxJsonStringLength = maxJsonStringLength;
System.setProperty(
ObjectMapperFactory.MAX_JSON_STRING_LENGTH_JVM, Integer.toString(maxJsonStringLength));
}

@Test
public void testInvalidMaxJsonStringLength() throws SQLException {
System.setProperty(ObjectMapperFactory.MAX_JSON_STRING_LENGTH_JVM, "abc");
// calling getObjectMapper() should log the exception but not throw
// default maxJsonStringLength value will be used
ObjectMapper mapper = ObjectMapperFactory.getObjectMapper();
int stringLengthInMapper = mapper.getFactory().streamReadConstraints().getMaxStringLength();
Assert.assertEquals(ObjectMapperFactory.DEFAULT_MAX_JSON_STRING_LEN, stringLengthInMapper);
}

@Test
public void testObjectMapperWithLargeJsonString() {
ObjectMapper mapper = ObjectMapperFactory.getObjectMapper();
try {
JsonNode jsonNode = mapper.readTree(generateBase64EncodedJsonString(lobSizeInBytes));
Assert.assertNotNull(jsonNode);
} catch (Exception e) {
// exception is expected when jackson's default maxStringLength value is used while retrieving
// 16M string data
assertEquals(jacksonDefaultMaxStringLength, maxJsonStringLength);
}
}

private String generateBase64EncodedJsonString(int numChar) {
StringBuilder jsonStr = new StringBuilder();
String largeStr = SnowflakeUtil.randomAlphaNumeric(numChar);

// encode the string and put it into a JSON formatted string
jsonStr.append("[\"").append(encodeStringToBase64(largeStr)).append("\"]");
return jsonStr.toString();
}

private String encodeStringToBase64(String stringToBeEncoded) {
return Base64.getEncoder().encodeToString(stringToBeEncoded.getBytes(StandardCharsets.UTF_8));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ private Map<SFSessionProperty, Object> initConnectionPropertiesMap() {
public void testConvertSystemPropertyToIntValue() {
// SNOW-760642 - Test that new default for net.snowflake.jdbc.ttl is 60 seconds.
assertEquals(
60, HttpUtil.convertSystemPropertyToIntValue(HttpUtil.JDBC_TTL, HttpUtil.DEFAULT_TTL));
60, SystemUtil.convertSystemPropertyToIntValue(HttpUtil.JDBC_TTL, HttpUtil.DEFAULT_TTL));

// Test that TTL can be disabled
System.setProperty(HttpUtil.JDBC_TTL, "-1");
assertEquals(
-1, HttpUtil.convertSystemPropertyToIntValue(HttpUtil.JDBC_TTL, HttpUtil.DEFAULT_TTL));
-1, SystemUtil.convertSystemPropertyToIntValue(HttpUtil.JDBC_TTL, HttpUtil.DEFAULT_TTL));
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/net/snowflake/client/core/SessionUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,19 @@ public void testConvertSystemPropertyToIntValue() {
System.setProperty("net.snowflake.jdbc.max_connections", "500");
assertEquals(
500,
HttpUtil.convertSystemPropertyToIntValue(
SystemUtil.convertSystemPropertyToIntValue(
HttpUtil.JDBC_MAX_CONNECTIONS_PROPERTY, HttpUtil.DEFAULT_MAX_CONNECTIONS));
// Test that entering a non-int sets the value to the default
System.setProperty("net.snowflake.jdbc.max_connections", "notAnInteger");
assertEquals(
HttpUtil.DEFAULT_MAX_CONNECTIONS,
HttpUtil.convertSystemPropertyToIntValue(
SystemUtil.convertSystemPropertyToIntValue(
HttpUtil.JDBC_MAX_CONNECTIONS_PROPERTY, HttpUtil.DEFAULT_MAX_CONNECTIONS));
// Test another system property
System.setProperty("net.snowflake.jdbc.max_connections_per_route", "30");
assertEquals(
30,
HttpUtil.convertSystemPropertyToIntValue(
SystemUtil.convertSystemPropertyToIntValue(
HttpUtil.JDBC_MAX_CONNECTIONS_PER_ROUTE_PROPERTY,
HttpUtil.DEFAULT_MAX_CONNECTIONS_PER_ROUTE));
}
Expand Down
22 changes: 22 additions & 0 deletions src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import net.snowflake.client.RunningOnGithubAction;
import net.snowflake.client.TestUtil;
import net.snowflake.client.category.TestCategoryResultSet;
import net.snowflake.client.core.ObjectMapperFactory;
import net.snowflake.client.core.SFBaseSession;
import net.snowflake.client.core.SessionUtil;
import net.snowflake.client.jdbc.telemetry.*;
Expand Down Expand Up @@ -924,4 +925,25 @@ public void testGranularTimeFunctionsInUTC() throws SQLException {
connection.close();
}
}

/** Added in > 3.14.5 */
@Test
public void testLargeStringRetrieval() throws SQLException {
String tableName = "maxJsonStringLength_table";
int colLength = 16777216;
try (Connection con = getConnection();
Statement statement = con.createStatement()) {
statement.execute("create or replace table " + tableName + " (c1 string(" + colLength + "))");
statement.execute(
"insert into " + tableName + " select randstr(" + colLength + ", random())");
assertNull(System.getProperty(ObjectMapperFactory.MAX_JSON_STRING_LENGTH_JVM));
try (ResultSet rs = statement.executeQuery("select * from " + tableName)) {
assertTrue(rs.next());
assertEquals(colLength, rs.getString(1).length());
assertFalse(rs.next());
}
} catch (Exception e) {
fail("executeQuery should not fail");
}
}
}

0 comments on commit e899319

Please sign in to comment.