Fix ignoring trailing JSON content in various places

client/trino-client/src/main/java/io/trino/client/JsonCodec.java

@@ -13,6 +13,7 @@
  */
 package io.trino.client;
 
+import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.JavaType;
@@ -22,9 +23,11 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.lang.reflect.Type;
 import java.util.function.Supplier;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
 
 public class JsonCodec<T>
@@ -67,12 +70,26 @@ public Type getType()
     public T fromJson(String json)
             throws JsonProcessingException
     {
-        return mapper.readerFor(javaType).readValue(json);
+        try (JsonParser parser = mapper.createParser(json)) {
+            T value = mapper.readerFor(javaType).readValue(parser);
+            checkArgument(parser.nextToken() == null, "Found characters after the expected end of input");
+            return value;
+        }
+        catch (JsonProcessingException e) {
+            throw e;
+        }
+        catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
     }
 
     public T fromJson(InputStream inputStream)
             throws IOException, JsonProcessingException
     {
-        return mapper.readerFor(javaType).readValue(inputStream);
+        try (JsonParser parser = mapper.createParser(inputStream)) {
+            T value = mapper.readerFor(javaType).readValue(parser);
+            checkArgument(parser.nextToken() == null, "Found characters after the expected end of input");
+            return value;
+        }
     }
 }

client/trino-client/src/test/java/io/trino/client/TestJsonCodec.java

@@ -0,0 +1,55 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.trino.client;
+
+import com.fasterxml.jackson.core.JsonParseException;
+import org.testng.annotations.Test;
+
+import java.io.ByteArrayInputStream;
+
+import static io.trino.client.JsonCodec.jsonCodec;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.testng.Assert.assertEquals;
+
+public class TestJsonCodec
+{
+    @Test
+    public void testTrailingContent()
+            throws Exception
+    {
+        JsonCodec<ClientTypeSignature> codec = jsonCodec(ClientTypeSignature.class);
+
+        String json = "{\"rawType\":\"bigint\",\"arguments\":[]}";
+        assertEquals(codec.fromJson(json).getRawType(), "bigint");
+
+        String jsonWithTrailingContent = json + " trailer";
+        assertThatThrownBy(() -> codec.fromJson(jsonWithTrailingContent))
+                .isInstanceOf(JsonParseException.class)
+                .hasMessageStartingWith("Unrecognized token 'trailer': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')");
+
+        assertThatThrownBy(() -> codec.fromJson(new ByteArrayInputStream(jsonWithTrailingContent.getBytes(UTF_8))))
+                .isInstanceOf(JsonParseException.class)
+                .hasMessageStartingWith("Unrecognized token 'trailer': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')");
+
+        String jsonWithTrailingJsonContent = json + " \"valid json value\"";
+        assertThatThrownBy(() -> codec.fromJson(jsonWithTrailingJsonContent))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Found characters after the expected end of input");
+
+        assertThatThrownBy(() -> codec.fromJson(new ByteArrayInputStream(jsonWithTrailingJsonContent.getBytes(UTF_8))))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Found characters after the expected end of input");
+    }
+}

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonUtils.java

@@ -19,18 +19,20 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.annotations.VisibleForTesting;
 import io.airlift.json.ObjectMapperProvider;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.UncheckedIOException;
+import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.lang.String.format;
 import static java.nio.file.Files.exists;
 import static java.nio.file.Files.isReadable;
+import static java.util.Objects.requireNonNull;
 
 public final class JsonUtils
 {
@@ -53,18 +55,64 @@ public static <T> T parseJson(Path path, Class<T> javaType)
             byte[] json = Files.readAllBytes(path);
             return parseJson(json, javaType);
         }
-        catch (IOException e) {
+        catch (IOException | RuntimeException e) {
             throw new IllegalArgumentException(format("Invalid JSON file '%s' for '%s'", path, javaType), e);
         }
     }
 
-    public static JsonNode parseJson(String json)
+    public static <T> T parseJson(String json, Class<T> javaType)
     {
-        try {
-            return OBJECT_MAPPER.readTree(json);
+        return parseJson(OBJECT_MAPPER, json, javaType);
+    }
+
+    public static <T> T parseJson(ObjectMapper mapper, String json, Class<T> javaType)
+    {
+        return parseJson(mapper, ObjectMapper::createParser, json, javaType);
+    }
+
+    public static <T> T parseJson(byte[] jsonBytes, Class<T> javaType)
+    {
+        return parseJson(OBJECT_MAPPER, jsonBytes, javaType);
+    }
+
+    public static <T> T parseJson(ObjectMapper mapper, byte[] jsonBytes, Class<T> javaType)
+    {
+        return parseJson(mapper, ObjectMapper::createParser, jsonBytes, javaType);
+    }
+
+    public static <T> T parseJson(InputStream inputStream, Class<T> javaType)
+    {
+        return parseJson(OBJECT_MAPPER, inputStream, javaType);
+    }
+
+    public static <T> T parseJson(ObjectMapper mapper, InputStream inputStream, Class<T> javaType)
+    {
+        return parseJson(mapper, ObjectMapper::createParser, inputStream, javaType);
+    }
+
+    public static <T> T parseJson(URL url, Class<T> javaType)
+    {
+        return parseJson(OBJECT_MAPPER, url, javaType);
+    }
+
+    public static <T> T parseJson(ObjectMapper mapper, URL url, Class<T> javaType)
+    {
+        return parseJson(mapper, ObjectMapper::createParser, url, javaType);
+    }
+
+    private static <I, T> T parseJson(ObjectMapper mapper, ParserConstructor<I> parserConstructor, I input, Class<T> javaType)
+    {
+        requireNonNull(mapper, "mapper is null");
+        requireNonNull(parserConstructor, "parserConstructor is null");
+        requireNonNull(input, "input is null");
+        requireNonNull(javaType, "javaType is null");
+        try (JsonParser parser = parserConstructor.createParser(mapper, input)) {
+            T value = mapper.readValue(parser, javaType);
+            checkArgument(parser.nextToken() == null, "Found characters after the expected end of input");
+            return value;
         }
         catch (IOException e) {
-            throw new UncheckedIOException("Could not parse JSON node from given byte array", e);
+            throw new UncheckedIOException("Could not parse JSON", e);
         }
     }
 
@@ -74,18 +122,13 @@ public static <T> T jsonTreeToValue(JsonNode treeNode, Class<T> javaType)
             return OBJECT_MAPPER.treeToValue(treeNode, javaType);
         }
         catch (JsonProcessingException e) {
-            throw new UncheckedIOException("Failed to parse JSON tree node", e);
+            throw new UncheckedIOException("Failed to convert JSON tree node", e);
         }
     }
 
-    @VisibleForTesting
-    static <T> T parseJson(byte[] jsonBytes, Class<T> javaType)
-            throws IOException
+    private interface ParserConstructor<I>
     {
-        try (JsonParser parser = OBJECT_MAPPER.createParser(jsonBytes)) {
-            T value = OBJECT_MAPPER.readValue(parser, javaType);
-            checkArgument(parser.nextToken() == null, "Found characters after the expected end of input");
-            return value;
-        }
+        JsonParser createParser(ObjectMapper mapper, I input)
+                throws IOException;
     }
 }

lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/util/TestJsonUtils.java

@@ -14,10 +14,11 @@
 package io.trino.plugin.base.util;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.core.JsonParseException;
+import com.fasterxml.jackson.databind.JsonNode;
 import org.testng.annotations.Test;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 
 import static io.trino.plugin.base.util.JsonUtils.parseJson;
 import static io.trino.plugin.base.util.TestJsonUtils.TestEnum.OPTION_A;
@@ -51,12 +52,22 @@ public void testLowercaseEnum()
     public void testTrailingContent()
             throws IOException
     {
-        assertThatThrownBy(() -> parseJson("{} {}}".getBytes(US_ASCII), TestObject.class))
+        // parseJson(String)
+        assertThatThrownBy(() -> parseJson("{} {}}", JsonNode.class))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessage("Found characters after the expected end of input");
+        assertThatThrownBy(() -> parseJson("{} not even a JSON here", JsonNode.class))
+                .isInstanceOf(UncheckedIOException.class)
+                .hasMessage("Could not parse JSON")
+                .hasStackTraceContaining("Unrecognized token 'not': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')");
 
+        // parseJson(byte[], Class)
+        assertThatThrownBy(() -> parseJson("{} {}}".getBytes(US_ASCII), TestObject.class))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Found characters after the expected end of input");
         assertThatThrownBy(() -> parseJson("{} not even a JSON here".getBytes(US_ASCII), TestObject.class))
-                .isInstanceOf(JsonParseException.class)
-                .hasMessageStartingWith("Unrecognized token 'not': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')");
+                .isInstanceOf(UncheckedIOException.class)
+                .hasMessage("Could not parse JSON")
+                .hasStackTraceContaining("Unrecognized token 'not': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')");
     }
 }

plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/metadata/ZooKeeperMetadataManager.java

@@ -13,6 +13,7 @@
  */
 package io.trino.plugin.accumulo.metadata;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -37,8 +38,8 @@
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static io.trino.plugin.accumulo.AccumuloErrorCode.ZOOKEEPER_ERROR;
+import static io.trino.plugin.base.util.JsonUtils.parseJson;
 import static java.lang.String.format;
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 import static org.apache.zookeeper.KeeperException.Code.NONODE;
 
@@ -320,29 +321,26 @@ private boolean isAccumuloView(SchemaTableName tableName)
     }
 
     private boolean isAccumuloTable(byte[] data)
-            throws IOException
     {
         // AccumuloTable does not contain a 'data' node
-        return !mapper.reader().readTree(new String(data, UTF_8)).has("data");
+        return !parseJson(mapper, data, JsonNode.class).has("data");
     }
 
     private boolean isAccumuloView(byte[] data)
-            throws IOException
     {
         // AccumuloView contains a 'data' node
-        return mapper.reader().readTree(new String(data, UTF_8)).has("data");
+        return parseJson(mapper, data, JsonNode.class).has("data");
     }
 
     private AccumuloTable toAccumuloTable(byte[] data)
-            throws IOException
     {
-        return mapper.readValue(new String(data, UTF_8), AccumuloTable.class);
+        return parseJson(mapper, data, AccumuloTable.class);
     }
 
     private AccumuloView toAccumuloView(byte[] data)
             throws IOException
     {
-        return mapper.readValue(new String(data, UTF_8), AccumuloView.class);
+        return parseJson(mapper, data, AccumuloView.class);
     }
 
     private byte[] toJsonBytes(Object obj)

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/TransactionLogParser.java

@@ -19,6 +19,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.airlift.json.ObjectMapperProvider;
 import io.airlift.log.Logger;
+import io.trino.plugin.base.util.JsonUtils;
 import io.trino.plugin.deltalake.DeltaLakeColumnHandle;
 import io.trino.plugin.deltalake.transactionlog.checkpoint.LastCheckpoint;
 import io.trino.spi.TrinoException;
@@ -35,7 +36,7 @@
 import javax.annotation.Nullable;
 
 import java.io.IOException;
-import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.math.BigDecimal;
 import java.time.Duration;
 import java.time.LocalDate;
@@ -136,7 +137,7 @@ public static DeltaLakeTransactionLogEntry parseJson(String json)
         if (json.endsWith("x")) {
             json = json.substring(0, json.length() - 1);
         }
-        return OBJECT_MAPPER.readValue(json, DeltaLakeTransactionLogEntry.class);
+        return JsonUtils.parseJson(OBJECT_MAPPER, json, DeltaLakeTransactionLogEntry.class);
     }
 
     private static Object parseDecimal(DecimalType type, String valueString)
@@ -230,18 +231,18 @@ static Optional<LastCheckpoint> readLastCheckpoint(FileSystem fileSystem, Path t
     }
 
     private static Optional<LastCheckpoint> tryReadLastCheckpoint(FileSystem fileSystem, Path tableLocation)
-            throws IOException
+            throws JsonParseException, JsonMappingException
     {
         Path transactionLogDirectory = getTransactionLogDir(tableLocation);
         try (FSDataInputStream lastCheckpointInput = fileSystem.open(new Path(transactionLogDirectory, LAST_CHECKPOINT_FILENAME))) {
             // Note: there apparently is 8K buffering applied and _last_checkpoint should be much smaller.
-            return Optional.of(OBJECT_MAPPER.readValue((InputStream) lastCheckpointInput, LastCheckpoint.class));
+            return Optional.of(JsonUtils.parseJson(OBJECT_MAPPER, lastCheckpointInput, LastCheckpoint.class));
         }
         catch (JsonParseException | JsonMappingException e) {
             // The _last_checkpoint file is malformed, it's probably in the middle of a rewrite (file rewrites on Azure are NOT atomic)
             throw e;
         }
-        catch (IOException e) {
+        catch (IOException | UncheckedIOException e) {
             // _last_checkpoint file was not found, we need to find latest checkpoint manually
             // ideally, we'd detect the condition by catching FileNotFoundException, but some file system implementations
             // will throw different exceptions if the checkpoint is not found

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/statistics/DeltaLakeJsonFileStatistics.java

@@ -37,6 +37,7 @@
 
 import static com.google.common.base.MoreObjects.toStringHelper;
 import static io.airlift.slice.SizeOf.estimatedSizeOf;
+import static io.trino.plugin.base.util.JsonUtils.parseJson;
 import static io.trino.plugin.deltalake.transactionlog.TransactionLogAccess.toCanonicalNameKeyedMap;
 import static io.trino.plugin.deltalake.transactionlog.TransactionLogParser.JSON_STATISTICS_TIMESTAMP_FORMATTER;
 import static io.trino.plugin.deltalake.transactionlog.TransactionLogParser.START_OF_MODERN_ERA;
@@ -62,7 +63,7 @@ public class DeltaLakeJsonFileStatistics
     public static DeltaLakeJsonFileStatistics create(String jsonStatistics)
             throws JsonProcessingException
     {
-        return OBJECT_MAPPER.readValue(jsonStatistics, DeltaLakeJsonFileStatistics.class);
+        return parseJson(OBJECT_MAPPER, jsonStatistics, DeltaLakeJsonFileStatistics.class);
     }
 
     @JsonCreator

plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/S3SecurityMappingsParser.java

@@ -14,8 +14,9 @@
 package io.trino.plugin.hive.s3;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import io.trino.plugin.base.util.JsonUtils;
 
+import static io.trino.plugin.base.util.JsonUtils.jsonTreeToValue;
+import static io.trino.plugin.base.util.JsonUtils.parseJson;
 import static java.util.Objects.requireNonNull;
 
 public class S3SecurityMappingsParser
@@ -29,8 +30,8 @@ protected S3SecurityMappingsParser(S3SecurityMappingConfig config)
 
     public S3SecurityMappings parseJSONString(String jsonString)
     {
-        JsonNode node = JsonUtils.parseJson(jsonString);
+        JsonNode node = parseJson(jsonString, JsonNode.class);
         JsonNode mappingsNode = node.at(this.jsonPointer);
-        return JsonUtils.jsonTreeToValue(mappingsNode, S3SecurityMappings.class);
+        return jsonTreeToValue(mappingsNode, S3SecurityMappings.class);
     }
 }