diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index cfb2b6bf036..70b64a4595c 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -30,6 +30,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Added verification to `io()` ensuring a class named by `IO.reader`, `IO.writer` or `IO.registry` implements the expected interface before loading and initializing it. * Fixed `io().write()` to resolve its `GraphWriter` before opening the output file, so that a writer which cannot be constructed no longer truncates the target. * Fixed `TinkerGraph` GraphSON deserialization to fail fast with `JsonParseException` on malformed `tinker:graph` input. +* Changed GraphSON 1.0 typed deserialization to resolve a `@class` type id only when the name is in a set of exact class names. * Restricted Hadoop/Spark OLAP `io()` from a remotely submitted traversal to operator-approved reader/writer classes and `with()` keys, adding the `gremlin.io.trusted`, `gremlin.io.approvedClasses` and `gremlin.io.approvedGraphConfigKeys` options. * Restricted OLAP `GraphComputer.configure()` from a remotely submitted traversal to built-in and operator-approved keys, adding the `gremlin.io.approvedComputerConfigKeys` option. * Fixed `subgraph()` to throw a descriptive error identifying the required `Edge` input instead of an internal `ClassCastException` when the traversal produces a non-edge value. diff --git a/docker/gremlin-server/gremlin-server-integration.yaml b/docker/gremlin-server/gremlin-server-integration.yaml index d0394795083..e5b94459ad2 100644 --- a/docker/gremlin-server/gremlin-server-integration.yaml +++ b/docker/gremlin-server/gremlin-server-integration.yaml @@ -40,7 +40,7 @@ scriptEngines: { serializers: - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV3, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV3] }} - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV2, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV2] }} - - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV1] }} + - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV1], allowedTypeIdNames: [java.awt.Point] }} - { className: org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1 } - { className: org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1, config: { serializeResultToString: true }} processors: diff --git a/docs/src/dev/io/graphson.asciidoc b/docs/src/dev/io/graphson.asciidoc index e1ed00d7984..29e57cba735 100644 --- a/docs/src/dev/io/graphson.asciidoc +++ b/docs/src/dev/io/graphson.asciidoc @@ -140,7 +140,15 @@ Version 1.0 of GraphSON was released with TinkerPop 3.0.0. It is referred to by When types are embedded, GraphSON uses the standard link:https://github.com/FasterXML/jackson-databind[Jackson] type embedding approach that writes the full Java class name into a "@class" field in the JSON. While this approach isn't especially language agnostic it does at least give -some hint as to what the expected type is. +some hint as to what the expected type is. On read, that class name resolves only when it is in a set of exact class +names, derived from the types that GraphSON 2.0 and 3.0 register plus the concrete runtime classes that GraphSON 1.0 +writes and the registry does not declare. Names are matched by equality, and an array is decided by its component +name. A parameterized type id is always refused. The `java.lang.Class` type is refused by default but can be +explicitly allowed. A provider or application can add its own class names with +`GraphSONMapper.Builder.addAllowedTypeIdName(String...)`, for example `addAllowedTypeIdName("com.example.MyType")`. +For a very narrow set of types, typed GraphSON 1.0 writing and reading are asymmetric. The writer may emit a concrete +or parameterized Java type id that the reader refuses. Examples include `ByteBuffer` and `EnumMap`, which can be +written but cannot be read back. This section focuses on non-embedded types and their formats as there was little usage of embedded types in generalized object serialization use cases. The format was simply too cumbersome to parse of non-Jackson enabled libraries and the diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc index bdf8263b4d5..81c157247ff 100644 --- a/docs/src/reference/gremlin-applications.asciidoc +++ b/docs/src/reference/gremlin-applications.asciidoc @@ -1176,6 +1176,7 @@ The drivers are designed to use that type information to properly produce result system and may not function correctly without it. Generally speaking, `GraphBinary` is always the best choice for the drivers. +[[server-graphson]] ===== GraphSON The GraphSON serializer produces human-readable output in JSON format and is a good configuration choice for those @@ -1211,14 +1212,30 @@ Configuring GraphSON in the Gremlin Server configuration looks like this: - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV3 } ---- -Gremlin Server is configured by default with GraphSON 3.0 as shown above. It has the following configuration option: +Gremlin Server is configured by default with GraphSON 3.0 as shown above. It has the following configuration options: [width="100%",cols="3,10,^2",options="header"] |========================================================= |Key |Description |Default |ioRegistries |A list of `IoRegistry` implementations to be applied to the serializer. |_none_ +|allowedTypeIdNames |A list of fully qualified class names that `GraphSONMessageSerializerV1` is allowed to resolve from `@class` type ids, in addition to its defaults. |_none_ |========================================================= +The `allowedTypeIdNames` option only affects typed GraphSON 1.0 reads. The equivalent mapper configuration uses +`GraphSONMapper.Builder.addAllowedTypeIdName(String...)`: + +[source,java] +---- +GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName("com.example.MyType", "com.example.MyOtherType").create(); +---- + +Each configured name is matched exactly, so it does not allow subclasses or other classes in the same package. An +array is checked by its component class name, so allowing `com.example.MyType` also allows arrays of that class. +Parameterized type ids are rejected before class-name matching. Therefore, a type id such as +`java.util.EnumMap<...>` cannot be enabled by adding either `java.util.EnumMap` or the parameterized type id to the +configured names. + It is worth noting that GraphSON 1.0 still has some appeal for some users as it can be configured to produce an untyped JSON format which is a bit easier to consume than its successors which embed data types into the output. This version of GraphSON tends to be the one that users like to utilize when <> and is still @@ -1228,7 +1245,10 @@ To configure Gremlin Server this way, the `GraphSONMessageSerializerV1` must be [source,yaml] ---- - - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1 } + - className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1 + config: + allowedTypeIdNames: + - com.example.MyType - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV3 } ---- diff --git a/docs/src/upgrade/release-3.7.x.asciidoc b/docs/src/upgrade/release-3.7.x.asciidoc index 05524f92997..a97738f3db1 100644 --- a/docs/src/upgrade/release-3.7.x.asciidoc +++ b/docs/src/upgrade/release-3.7.x.asciidoc @@ -88,6 +88,45 @@ GryoIo.build().graph(graph).onMapper(m -> ((GryoMapper.Builder) m) See: link:https://issues.apache.org/jira/browse/TINKERPOP-3278[TINKERPOP-3278] +==== GraphSON 1.0 Type Allow-List + +Typed GraphSON 1.0 now resolves a Java class named by an `@class` property only when that exact name is allowed. +This change is breaking on read for provider and application types outside the default set. Some values that GraphSON +1.0 can write, including `ByteBuffer` and `EnumMap`, also no longer round-trip. The written format is unchanged. + +The failure looks like this: + +[source,text] +---- +Could not resolve type id ...: Configured `PolymorphicTypeValidator` ... denied resolution +---- + +Further simple class names can be added on the mapper: + +[source,java] +---- +GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName("com.example.MyType", "com.example.MyOtherType").create(); +---- + +Expected application types can be added to the message serializer configuration: + +[source,yaml] +---- +serializers: + - className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1 + config: + allowedTypeIdNames: + - com.example.MyType +---- + +Allowlisting cannot restore a parameterized type id such as `java.util.EnumMap<...>` because GraphSON 1.0 rejects +all parameterized type ids unconditionally. Affected applications can convert values to non-parameterized +representations such as `HashMap`, or move to GraphSON 2.0, GraphSON 3.0 or GraphBinary. + +See: link:https://tinkerpop.apache.org/docs/3.7.7/reference/#server-graphson[GraphSON Server Configuration], +link:https://tinkerpop.apache.org/docs/3.7.7/dev/io/#graphson-1d0[GraphSON 1.0 IO Documentation] + ==== OLAP Restrictions The OLAP `io()` step on `HadoopGraph` (`g.withComputer().io(path)...`) historically transferred every diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java index cda357caaeb..bab483d1d29 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java @@ -27,6 +27,13 @@ import org.apache.tinkerpop.shaded.jackson.core.StreamReadConstraints; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; import org.apache.tinkerpop.shaded.jackson.databind.SerializationFeature; +import org.apache.tinkerpop.shaded.jackson.databind.JavaType; +import org.apache.tinkerpop.shaded.jackson.databind.DatabindContext; +import org.apache.tinkerpop.shaded.jackson.databind.DeserializationContext; +import org.apache.tinkerpop.shaded.jackson.databind.cfg.MapperConfig; +import org.apache.tinkerpop.shaded.jackson.databind.jsontype.PolymorphicTypeValidator; +import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeIdResolver; +import org.apache.tinkerpop.shaded.jackson.databind.jsontype.NamedType; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeResolverBuilder; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.impl.StdTypeResolverBuilder; import org.apache.tinkerpop.shaded.jackson.databind.module.SimpleModule; @@ -35,12 +42,17 @@ import java.sql.Timestamp; import java.util.ArrayList; +import java.util.Collection; +import java.io.IOException; import java.util.Arrays; import java.util.Calendar; import java.util.Date; import java.util.List; import java.util.Map; import java.util.TimeZone; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.Set; import java.util.UUID; /** @@ -63,6 +75,49 @@ * @author Stephen Mallette (http://stephen.genoprime.com) */ public class GraphSONMapper implements Mapper { + + // Java base value types registered for GraphSON 2.0/3.0, shared by registerJavaBaseTypes and the V1 name set. + private static final List GRAPHSON_JAVA_BASE_TYPES = Arrays.asList( + UUID.class, Class.class, Calendar.class, Date.class, TimeZone.class, Timestamp.class); + + // concrete runtime class names GraphSON 1.0 writes that registeredV2V3Types() does not supply, as it declares + // only the java.util.List, Map and Set interfaces + private static final List GRAPHSON_1_0_ALLOWED_EXTRA_TYPE_NAMES = Arrays.asList( + "java.lang.Boolean", + "java.lang.Object", + "java.lang.String", + "java.net.URI", + "java.sql.Date", + "java.sql.Time", + "java.time.DayOfWeek", + "java.time.Month", + "java.util.ArrayDeque", + "java.util.ArrayList", + "java.util.Arrays$ArrayList", + "java.util.Collections$EmptyList", + "java.util.Collections$EmptyMap", + "java.util.Collections$EmptySet", + "java.util.Collections$SingletonList", + "java.util.Collections$SingletonMap", + "java.util.Collections$SingletonSet", + "java.util.Collections$UnmodifiableList", + "java.util.Collections$UnmodifiableMap", + "java.util.Collections$UnmodifiableRandomAccessList", + "java.util.Collections$UnmodifiableSet", + "java.util.Currency", + "java.util.GregorianCalendar", + "java.util.HashMap", + "java.util.HashSet", + "java.util.LinkedHashMap", + "java.util.LinkedHashSet", + "java.util.LinkedList", + "java.util.Locale", + "java.util.Properties", + "java.util.TreeMap", + "java.util.TreeSet", + // gremlin-util depends on gremlin-core, so the V1 message envelope is named here without a class reference + "org.apache.tinkerpop.gremlin.util.message.RequestMessage"); + private static final Set GRAPHSON_1_0_ALLOWED_TYPE_NAMES = graphSON1dAllowedTypeNames(); public static final int DEFAULT_MAX_NUMBER_LENGTH = 10000; private final List customModules; @@ -71,6 +126,7 @@ public class GraphSONMapper implements Mapper { private final GraphSONVersion version; private final TypeInfo typeInfo; private final StreamReadConstraints streamReadConstraints; + final List allowedTypeIdNames; private GraphSONMapper(final Builder builder) { this.customModules = builder.customModules; @@ -79,6 +135,7 @@ private GraphSONMapper(final Builder builder) { this.version = builder.version; this.streamReadConstraints = builder.streamReadConstraintsBuilder.build(); this.typeInfo = builder.typeInfo; + this.allowedTypeIdNames = builder.allowedTypeIdNames; } @Override @@ -128,8 +185,24 @@ public ObjectMapper createMapper() { om.setDefaultTyping(typer); } else if (version == GraphSONVersion.V1_0 || version == GraphSONVersion.V2_0) { if (typeInfo == TypeInfo.PARTIAL_TYPES) { - final TypeResolverBuilder typer = new StdTypeResolverBuilder() - .init(JsonTypeInfo.Id.CLASS, null) + final Set allowedNames = new HashSet<>(GRAPHSON_1_0_ALLOWED_TYPE_NAMES); + allowedNames.addAll(allowedTypeIdNames); + final PolymorphicTypeValidator typeValidator = graphSON1dTypeValidator(allowedNames); + final TypeResolverBuilder typer = new StdTypeResolverBuilder() { + @Override + public PolymorphicTypeValidator subTypeValidator(final MapperConfig config) { + return typeValidator; + } + + @Override + protected TypeIdResolver idResolver(final MapperConfig config, final JavaType baseType, + final PolymorphicTypeValidator subtypeValidator, + final Collection subtypes, + final boolean forSer, final boolean forDeser) { + return new GraphSON1dScreeningIdResolver( + super.idResolver(config, baseType, subtypeValidator, subtypes, forSer, forDeser)); + } + }.init(JsonTypeInfo.Id.CLASS, null) .inclusion(JsonTypeInfo.As.PROPERTY) .typeProperty(GraphSONTokens.CLASS); om.setDefaultTyping(typer); @@ -152,6 +225,141 @@ public ObjectMapper createMapper() { return om; } + /** + * A {@link PolymorphicTypeValidator} for GraphSON 1.0 embedded types that decides a simple type id from its name + * alone. Parameterized type ids are handled by {@link GraphSON1dScreeningIdResolver}, as the validator is not + * shown a type id's arguments. + */ + private static PolymorphicTypeValidator graphSON1dTypeValidator(final Set allowedNames) { + return new PolymorphicTypeValidator.Base() { + @Override + public Validity validateBaseType(final MapperConfig config, final JavaType baseType) { + // must stay INDETERMINATE: on ALLOWED, Jackson substitutes LaissezFaireSubTypeValidator and the + // name set below is no longer consulted + return Validity.INDETERMINATE; + } + + @Override + public Validity validateSubClassName(final MapperConfig config, final JavaType baseType, + final String subClassName) { + return isAllowedTypeName(subClassName, allowedNames) ? Validity.ALLOWED : Validity.DENIED; + } + + @Override + public Validity validateSubType(final MapperConfig config, final JavaType baseType, + final JavaType subType) { + // reached only when validateSubClassName returns INDETERMINATE, which it never does, so this is not + // where a name is decided today + return isAllowedTypeName(subType.getRawClass().getName(), allowedNames) + ? Validity.ALLOWED : Validity.DENIED; + } + }; + } + + /** + * Wraps the class-name {@link TypeIdResolver} to refuse a parameterized GraphSON 1.0 type id, meaning one + * containing '{@code <}', rather than hand it to the delegate. + */ + private static final class GraphSON1dScreeningIdResolver implements TypeIdResolver { + private final TypeIdResolver delegate; + private JavaType baseType; + + private GraphSON1dScreeningIdResolver(final TypeIdResolver delegate) { + this.delegate = delegate; + } + + @Override + public void init(final JavaType baseType) { + this.baseType = baseType; + delegate.init(baseType); + } + + @Override + public JavaType typeFromId(final DatabindContext context, final String id) throws IOException { + if (id.indexOf('<') >= 0) { + if (context instanceof DeserializationContext) + throw ((DeserializationContext) context).invalidTypeIdException(baseType, id, + "GraphSON 1.0 does not permit a parameterized type id"); + throw new IOException("GraphSON 1.0 does not permit a parameterized type id: " + id); + } + return delegate.typeFromId(context, id); + } + + @Override + public String idFromValue(final Object value) { + return delegate.idFromValue(value); + } + + @Override + public String idFromValueAndType(final Object value, final Class suggestedType) { + return delegate.idFromValueAndType(value, suggestedType); + } + + @Override + public String idFromBaseType() { + return delegate.idFromBaseType(); + } + + @Override + public String getDescForKnownTypeIds() { + return delegate.getDescForKnownTypeIds(); + } + + @Override + public JsonTypeInfo.Id getMechanism() { + return delegate.getMechanism(); + } + } + + // TypeInfo does not affect the (static) type-definition map, so NO_TYPES is passed + private static Set registeredV2V3Types() { + final Set registered = new LinkedHashSet<>(GRAPHSON_JAVA_BASE_TYPES); + registered.addAll(GraphSONVersion.V2_0.getBuilder().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + registered.addAll(GraphSONVersion.V3_0.getBuilder().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + registered.addAll(GraphSONXModuleV2.build().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + registered.addAll(GraphSONXModuleV3.build().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + return registered; + } + + /** + * The exact class names GraphSON 1.0 embedded-type deserialization resolves by default. + */ + private static Set graphSON1dAllowedTypeNames() { + final Set names = new HashSet<>(GRAPHSON_1_0_ALLOWED_EXTRA_TYPE_NAMES); + names.addAll(graphSON1dDerivedTypeNames()); + return names; + } + + /** + * The GraphSON 1.0 allowed names derived from {@link #registeredV2V3Types()}. Package private so a test can pin + * it, since a new type definition in any GraphSON 2.0/3.0 module widens this set as a side effect. + */ + static Set graphSON1dDerivedTypeNames() { + final Set names = new HashSet<>(); + for (final Class c : registeredV2V3Types()) { + // a java.lang.Class value resolves its own content as a class name, which GraphSON 1.0 does not need + if (Class.class != c) + names.add(c.getName()); + } + return names; + } + + private static boolean isAllowedTypeName(final String typeName, final Set allowedNames) { + // unwrap array descriptors: "[Ljava.lang.String;" -> "java.lang.String", "[[B" -> primitive element + String name = typeName; + while (name.startsWith("[")) + name = name.substring(1); + if (name.length() <= 1) + return true; // primitive array element (e.g. [B, [I) names no class + if (name.startsWith("L") && name.endsWith(";")) + name = name.substring(1, name.length() - 1); + return allowedNames.contains(name); + } + public GraphSONVersion getVersion() { return this.version; } @@ -174,6 +382,7 @@ public static Builder build(final GraphSONMapper mapper) { builder.normalize = mapper.normalize; builder.typeInfo = mapper.typeInfo; builder.streamReadConstraintsBuilder = mapper.streamReadConstraints.rebuild(); + builder.allowedTypeIdNames.addAll(mapper.allowedTypeIdNames); return builder; } @@ -183,14 +392,8 @@ public TypeInfo getTypeInfo() { } private void registerJavaBaseTypes(final GraphSONTypeIdResolver graphSONTypeIdResolver) { - Arrays.asList( - UUID.class, - Class.class, - Calendar.class, - Date.class, - TimeZone.class, - Timestamp.class - ).forEach(e -> graphSONTypeIdResolver.addCustomType(String.format("%s:%s", GraphSONTokens.GREMLIN_TYPE_NAMESPACE, e.getSimpleName()), e)); + GRAPHSON_JAVA_BASE_TYPES.forEach(e -> graphSONTypeIdResolver.addCustomType( + String.format("%s:%s", GraphSONTokens.GREMLIN_TYPE_NAMESPACE, e.getSimpleName()), e)); } public static class Builder implements Mapper.Builder { @@ -204,6 +407,7 @@ public static class Builder implements Mapper.Builder { private StreamReadConstraints.Builder streamReadConstraintsBuilder = StreamReadConstraints.builder() .maxNumberLength(DEFAULT_MAX_NUMBER_LENGTH); private TypeInfo typeInfo = null; + private final List allowedTypeIdNames = new ArrayList<>(); private Builder() { } @@ -288,6 +492,19 @@ public Builder typeInfo(final TypeInfo typeInfo) { return this; } + /** + * Adds a fully qualified class name that GraphSON 1.0 embedded-type deserialization resolves in a + * {@code @class} property, in addition to the default names. A name is matched by equality, so it covers the + * class named and its arrays, but not its subclasses or its package. Adding the same name more than once has + * no additional effect. + *

+ * Has no effect on GraphSON 2.0 or 3.0, which resolve types through a fixed registry rather than by name. + */ + public Builder addAllowedTypeIdName(final String... names) { + this.allowedTypeIdNames.addAll(Arrays.asList(names)); + return this; + } + public Builder maxNumberLength(final int maxNumLength) { this.streamReadConstraintsBuilder.maxNumberLength(maxNumLength); return this; diff --git a/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java b/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java new file mode 100644 index 00000000000..3c112f436df --- /dev/null +++ b/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 com.example.gadget; + +import java.util.Objects; + +/** + * Test-only types whose names are not among the GraphSON 1.0 allowed type names, used to exercise the GraphSON 1.0 + * embedded-type rules and the {@code addAllowedTypeIdName(...)} opt-out. + */ +public final class GraphSONTestGadgets { + + private GraphSONTestGadgets() { + } + + /** + * Records that its static initializer ran through a system property, so a test can observe initialization + * without referencing the class, which would itself trigger it. + */ + public static class StaticInitCanary { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanary"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } + + /** + * Counterpart of {@link StaticInitCanary} for a class named by a collection element. + * Each scenario needs a distinct canary because a class's static initializer runs only once per JVM. + */ + public static class StaticInitCanaryElement { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryElement"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } + + /** + * A plain bean with no static-initializer side effect, used to check the opt-out. + */ + public static class SamplePojo { + public int x; + + public SamplePojo() { + } + + public SamplePojo(final int x) { + this.x = x; + } + + @Override + public boolean equals(final Object o) { + return o instanceof SamplePojo && ((SamplePojo) o).x == this.x; + } + + @Override + public int hashCode() { + return Objects.hashCode(x); + } + } + + /** + * A subclass of {@link SamplePojo}, used to verify that configuring the base class name does not permit its + * subclasses. + */ + public static class SamplePojoSubclass extends SamplePojo { + public SamplePojoSubclass() { + } + + public SamplePojoSubclass(final int x) { + super(x); + } + } + + /** + * Counterpart of {@link StaticInitCanary} for an enum named as a generic type argument, which Jackson does not + * otherwise validate. + */ + public enum StaticInitCanaryEnum { + A, B; + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryEnum"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + } + + /** + * Counterpart of {@link StaticInitCanary} for a class named as a generic type argument. + * Each scenario needs a distinct canary because a class's static initializer runs only once per JVM. + */ + public static class StaticInitCanaryArg { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryArg"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } + + /** + * Counterpart of {@link StaticInitCanary} for the class a {@code java.lang.Class} value names. + */ + public static class StaticInitCanaryValue { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryValue"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } +} diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperBuilderTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperBuilderTest.java new file mode 100644 index 00000000000..c978bd53bd9 --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperBuilderTest.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.tinkerpop.gremlin.structure.io.graphson; + +import org.junit.Test; + +import java.util.Collections; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; + +public class GraphSONMapperBuilderTest { + + @Test + public void shouldPreserveAllowedTypeIdNamesWhenRebuilding() { + final GraphSONMapper original = GraphSONMapper.build() + .addAllowedTypeIdName("java.lang.Class") + .create(); + final GraphSONMapper rebuilt = GraphSONMapper.build(original).create(); + + assertEquals(Collections.singletonList("java.lang.Class"), rebuilt.allowedTypeIdNames); + assertNotSame(original.allowedTypeIdNames, rebuilt.allowedTypeIdNames); + } +} diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java index 523cdc95c86..34948e6f3de 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java @@ -23,8 +23,19 @@ import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.TextP; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.gremlin.structure.Direction; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.util.star.StarGraph; import org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException; +import com.example.gadget.GraphSONTestGadgets.SamplePojo; +import com.example.gadget.GraphSONTestGadgets.SamplePojoSubclass; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanary; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanaryElement; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanaryEnum; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanaryValue; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; +import org.apache.tinkerpop.shaded.jackson.databind.exc.InvalidFormatException; +import org.apache.tinkerpop.shaded.jackson.databind.exc.InvalidTypeIdException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -33,21 +44,36 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.math.BigDecimal; +import java.nio.ByteBuffer; +import java.time.DayOfWeek; import java.time.Instant; import java.time.ZoneOffset; import java.time.ZonedDateTime; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.Currency; +import java.util.EnumMap; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import java.util.UUID; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.hamcrest.core.StringContains.containsString; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.fail; @@ -60,6 +86,11 @@ @RunWith(Parameterized.class) public class GraphSONMapperPartialEmbeddedTypeTest extends AbstractGraphSONTest { + /** + * The key the value under test is held under in the GraphSON 1.0 cases that nest it in a {@code Map}. + */ + private static final String MAP_VALUE_KEY = "v"; + @Parameterized.Parameters(name = "{0}") public static Iterable data() { return Arrays.asList(new Object[][]{ @@ -248,6 +279,567 @@ public void shouldHandleMapWithTypesUsingEmbedTypeSettingV1() throws Exception { assertEquals(100L, read.get("test")); } + @Test + public void shouldRejectNetworkPackageTypeWithEmbedTypeSettingV1() { + // a name resolves only by being listed, so java.net.URL does not, even though the sibling + // java.net.InetAddress that GraphSON 2.0/3.0 register does + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"v\":{\"@class\":\"java.net.URL\",\"u\":\"http://example.com\"}}"); + } + + @Test + public void shouldRoundTripArraysWithEmbedTypeSettingV1() throws Exception { + // an array type id is decided by its component name, and a nested array carries one per level. A primitive + // component ("[B") names no class, while an object component ("[Ljava.lang.String;") names one. + final ObjectMapper mapper = v1Typed(); + assertArrayEquals(new byte[]{1, 2, 3}, (byte[]) roundTripInMap(mapper, new byte[]{1, 2, 3})); + assertArrayEquals(new Boolean[]{true, false}, + (Boolean[]) roundTripInMap(mapper, new Boolean[]{true, false})); + assertArrayEquals(new String[]{"a", "b"}, (String[]) roundTripInMap(mapper, new String[]{"a", "b"})); + assertThat(Arrays.deepEquals(new String[][]{{"a"}, {"b"}}, + (String[][]) roundTripInMap(mapper, new String[][]{{"a"}, {"b"}})), is(true)); + } + + @Test + public void shouldRoundTripSqlAndUtilValueTypesWithEmbedTypeSettingV1() throws Exception { + // java.sql.Time is written as its toString, which formats and parses back in the default time zone, so + // valueOf of a fixed literal round-trips anywhere + final ObjectMapper mapper = v1Typed(); + assertRoundTripsInMap(mapper, Arrays.asList( + new java.sql.Timestamp(0L), + java.sql.Time.valueOf("12:34:56"), + Locale.US, + Currency.getInstance("USD"))); + + // java.util.ArrayDeque does not define value equality, so it is compared element-wise + final Object read = roundTripInMap(mapper, new ArrayDeque<>(Arrays.asList("a", "b"))); + assertThat(read, instanceOf(ArrayDeque.class)); + assertEquals(Arrays.asList("a", "b"), new ArrayList<>((ArrayDeque) read)); + } + + @Test + public void shouldRejectArrayOfDisallowedComponentWithEmbedTypeSettingV1() { + // an array type id is decided by its component name, so an unlisted component does not resolve + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"v\":[\"[Ljava.io.File;\",[\"/tmp/x\"]]}"); + } + + @Test + public void shouldAllowConfiguredTypeIdNameWithEmbedTypeSettingV1() throws Exception { + final String json = "{\"@class\":\"java.util.HashMap\",\"p\":{\"@class\":\"com.example.gadget.GraphSONTestGadgets$SamplePojo\",\"x\":42}}"; + + // not among the allowed names by default + assertDeniedByTypeValidator(v1Typed(), json); + + final ObjectMapper mapper = GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName("com.example.gadget.GraphSONTestGadgets$SamplePojo").create().createMapper(); + final Map read = mapper.readValue(json, HashMap.class); + assertEquals(new SamplePojo(42), read.get("p")); + } + + @Test + public void shouldAllowConfiguredTypeIdNameInsideCollectionWithEmbedTypeSettingV1() throws Exception { + final ObjectMapper mapper = GraphSONMapper.build().version(GraphSONVersion.V1_0) + .typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName(SamplePojo.class.getName()) + .create().createMapper(); + + final Object read = roundTripInMap(mapper, Collections.singletonList(new SamplePojo(42))); + assertEquals(Collections.singletonList(new SamplePojo(42)), read); + assertEquals(SamplePojo.class, ((List) read).get(0).getClass()); + } + + @Test + public void shouldAllowArrayOfConfiguredTypeIdNameButRejectSubclassWithEmbedTypeSettingV1() throws Exception { + final ObjectMapper mapper = GraphSONMapper.build().version(GraphSONVersion.V1_0) + .typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName(SamplePojo.class.getName()) + .create().createMapper(); + + assertArrayEquals(new SamplePojo[]{new SamplePojo(42)}, + (SamplePojo[]) roundTripInMap(mapper, new SamplePojo[]{new SamplePojo(42)})); + + final String subclassTypeId = SamplePojoSubclass.class.getName(); + assertTypeIdDeniedByTypeValidator(mapper, + "{\"@class\":\"java.util.HashMap\",\"p\":{\"@class\":\"" + subclassTypeId + "\",\"x\":42}}", + subclassTypeId); + } + + @Test + public void shouldRejectDisallowedCollectionElementWithoutInitializingClassV1() { + final String typeId = StaticInitCanaryElement.class.getName(); + final String json = "{\"@class\":\"java.util.HashMap\",\"v\":[\"java.util.ArrayList\",[" + + "{\"@class\":\"" + typeId + "\",\"x\":1}]]}"; + + System.clearProperty(StaticInitCanaryElement.FIRED_PROPERTY); + assertTypeIdDeniedByTypeValidator(v1Typed(), json, typeId); + assertNull("type id resolution must not initialize the class named by a refused collection element", + System.getProperty(StaticInitCanaryElement.FIRED_PROPERTY)); + } + + @Test + public void shouldRejectEnumTypeParameterAndNotLoadItV1() { + System.clearProperty(StaticInitCanaryEnum.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"A\":\"v\"}"); + assertNull("type id resolution must not load the enum named as a type argument", + System.getProperty(StaticInitCanaryEnum.FIRED_PROPERTY)); + } + + @Test + public void shouldRejectClassValueAndNotLoadItV1() { + // java.lang.Class is held out of the derived names by graphSON1dDerivedTypeNames() + System.clearProperty(StaticInitCanaryValue.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"c\":[\"java.lang.Class\",\"com.example.gadget.GraphSONTestGadgets$StaticInitCanaryValue\"]}"); + assertNull("type id resolution must not load the class named by a java.lang.Class value", + System.getProperty(StaticInitCanaryValue.FIRED_PROPERTY)); + } + + @Test + public void shouldNotLoadDisallowedClassWhenRefusingV1() { + // a @class outside the allowed names is decided from the name alone + System.clearProperty(StaticInitCanary.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"g\":{\"@class\":\"com.example.gadget.GraphSONTestGadgets$StaticInitCanary\",\"x\":1}}"); + assertNull("type id resolution must not load the class named by a refused @class", + System.getProperty(StaticInitCanary.FIRED_PROPERTY)); + } + + @Test + public void shouldRoundTripInetAddressWithEmbedTypeSettingV1() throws Exception { + // java.net.InetAddress is a derived name, since GraphSON 2.0/3.0 register it + final ObjectMapper mapper = v1Typed(); + final Map m = new HashMap<>(); + m.put("a", java.net.InetAddress.getByAddress(new byte[]{127, 0, 0, 1})); + + final Map read = mapper.readValue(mapper.writeValueAsString(m), HashMap.class); + assertEquals(java.net.InetAddress.getByAddress(new byte[]{127, 0, 0, 1}), read.get("a")); + } + + @Test + public void shouldRoundTripUriWithEmbedTypeSettingV1() throws Exception { + // java.net.URI is listed rather than derived, as GraphSON 2.0/3.0 do not register it + final ObjectMapper mapper = v1Typed(); + final Map m = new HashMap<>(); + m.put("u", new java.net.URI("http://example.com/x")); + + final Map read = mapper.readValue(mapper.writeValueAsString(m), HashMap.class); + assertEquals(new java.net.URI("http://example.com/x"), read.get("u")); + } + + @Test + public void shouldRoundTripBoxedPrimitivesWithEmbedTypeSettingV1() throws Exception { + // String, Integer, Double and Boolean are written bare, so only the boxed types JSON cannot represent + // natively carry a type id + assertRoundTripsInMap(v1Typed(), Arrays.asList( + Character.valueOf('c'), + BigDecimal.ONE)); + } + + @Test + public void shouldRoundTripCollectionTypesWithEmbedTypeSettingV1() throws Exception { + // the concrete collection class names GraphSON 1.0 writes for a Map-nested value. Each case checks the type + // id in the text written as well as the class read back, since AbstractMap.equals and AbstractList.equals + // are structural and would keep passing with no type id written at all. + final ObjectMapper mapper = v1Typed(); + final Map entry = Collections.singletonMap("a", "b"); + final List one = Collections.singletonList("a"); + + assertTypedRoundTripInMap(mapper, new LinkedHashMap<>(entry)); + assertTypedRoundTripInMap(mapper, new LinkedHashSet<>(one)); + + // Jackson cannot rebuild either class named, so it reads the type id back through a stand-in it can + // construct: Arrays.asList comes back as a plain ArrayList, and either unmodifiable list name comes back as a + // wrapper around an ArrayList. unmodifiableList writes Collections$UnmodifiableList over a LinkedList but + // Collections$UnmodifiableRandomAccessList over an ArrayList, so the allowed names carry both. + assertTypedRoundTripInMap(mapper, Arrays.asList("a", "b"), ArrayList.class); + assertTypedRoundTripInMap(mapper, Collections.unmodifiableList(new LinkedList<>(one)), + Collections.unmodifiableList(new ArrayList<>(one)).getClass()); + + // a Map in a List in a Map, so a type id is resolved at every depth + final Map nested = new HashMap<>(); + nested.put("inner", new ArrayList<>(Collections.singletonList(new HashMap(entry)))); + final String nestedJson = writeInMap(mapper, nested); + assertThat(nestedJson, containsString("\"inner\":[\"java.util.ArrayList\",[")); + + final Object readNested = readMapValue(mapper, nestedJson); + assertEquals(nested, readNested); + assertEquals(HashMap.class, readNested.getClass()); + final Object readInner = ((Map) readNested).get("inner"); + assertEquals(ArrayList.class, readInner.getClass()); + assertEquals(HashMap.class, ((List) readInner).get(0).getClass()); + } + + @Test + public void shouldRoundTripEnumTypesWithEmbedTypeSettingV1() throws Exception { + // T uses per-constant subclasses, whose type id is still the declaring enum. DayOfWeek is listed in + // GRAPHSON_1_0_ALLOWED_EXTRA_TYPE_NAMES rather than derived from the GraphSON 2.0/3.0 registry. + assertRoundTripsInMap(v1Typed(), Arrays.asList( + Direction.OUT, + T.id, + DayOfWeek.MONDAY)); + } + + @Test + public void shouldReadAllowedBaseTypeIdsWithEmbedTypeSettingV1() throws Exception { + // allowed names GraphSON 1.0 does not write itself, since it writes the concrete runtime class instead. A + // document may still name them, and what comes back is the concrete type Jackson picks for the base type. + final ObjectMapper mapper = v1Typed(); + + assertEquals(Collections.singletonMap("a", "b"), + readMapValue(mapper, "{\"@class\":\"java.util.HashMap\",\"v\":{\"@class\":\"java.util.Map\",\"a\":\"b\"}}")); + assertEquals(ByteBuffer.wrap(new byte[]{1, 2}), + readMapValue(mapper, "{\"@class\":\"java.util.HashMap\",\"v\":[\"java.nio.ByteBuffer\",\"AQI=\"]}")); + } + + @Test + public void shouldRefuseStarGraphBecauseV1WritesItsStarVertexAsAMapV1() throws Exception { + // GraphSON 1.0 writes a StarGraph as a bean whose "starVertex" property carries the type id + // java.util.HashMap rather than a StarVertex, so Jackson cannot rebuild a StarGraph from what GraphSON 1.0 + // writes and the name is not among the allowed names. + final ObjectMapper mapper = v1Typed(); + final String json; + try (final StarGraph starGraph = StarGraph.open()) { + starGraph.addVertex("label", "person"); + json = writeInMap(mapper, starGraph); + } + + assertThat(json, containsString("\"" + GraphSONTokens.CLASS + "\":\"" + StarGraph.class.getName() + "\"")); + assertThat(json, containsString("\"starVertex\":{\"" + GraphSONTokens.CLASS + "\":\"java.util.HashMap\"")); + assertTypeIdDeniedByTypeValidator(mapper, json, StarGraph.class.getName()); + } + + @Test + public void shouldRefuseByteBufferBecauseV1WritesItsConcreteHeapClassV1() throws Exception { + // java.nio.ByteBuffer is a derived name and does resolve (see + // shouldReadAllowedBaseTypeIdsWithEmbedTypeSettingV1), but what GraphSON 1.0 writes for wrap() or allocate() + // is the concrete java.nio.HeapByteBuffer, which the allowed names do not carry, so V1 cannot read what it + // writes. + final ObjectMapper mapper = v1Typed(); + + final String json = writeInMap(mapper, ByteBuffer.wrap(new byte[]{1, 2})); + assertThat(json, containsString("[\"java.nio.HeapByteBuffer\",")); + assertTypeIdDeniedByTypeValidator(mapper, json, "java.nio.HeapByteBuffer"); + } + + @Test + public void shouldRefuseEnumMapBecauseV1WritesAParameterizedTypeIdV1() throws Exception { + // GraphSON 1.0 writes an EnumMap as the parameterized type id "java.util.EnumMap<...,...>", which + // GraphSON1dScreeningIdResolver refuses, so an EnumMap no longer reads back. That is a round-trip regression + // introduced by this change. An EnumSet is written as a parameterized type id too, but it did not read back + // beforehand either, as jackson-databind#4849 leaves the type id EnumSet writes unresolvable. + final ObjectMapper mapper = v1Typed(); + final String typeId = "java.util.EnumMap<" + Direction.class.getName() + ",java.lang.Object>"; + + final EnumMap value = new EnumMap<>(Direction.class); + value.put(Direction.OUT, "x"); + + final String json = writeInMap(mapper, value); + assertThat(json, containsString("\"" + GraphSONTokens.CLASS + "\":\"" + typeId + "\"")); + assertParameterizedTypeIdRefused(mapper, json, typeId); + } + + @Test + public void shouldRejectUnresolvableHostnameWithoutLookupWithEmbedTypeSettingV1() { + // java.net.InetAddress is among the allowed names, so a document may name it, but a value that is not an IP + // address literal is refused on its syntax rather than looked up. The message is the evidence of that, as a + // lookup would report an UnknownHostException instead. It is Jackson's own wording, so an upgrade can move it. + final String json = "{\"@class\":\"java.util.HashMap\",\"v\":" + + "[\"java.net.InetAddress\",\"this-name-should-not-resolve.invalid\"]}"; + try { + v1Typed().readValue(json, HashMap.class); + fail("an InetAddress value that is not an IP address literal must be refused"); + } catch (Exception e) { + assertThat(e, instanceOf(InvalidFormatException.class)); + assertThat(e.getMessage(), containsString("Not a valid IP address string literal")); + } + } + + @Test + public void shouldAllowConfiguredClassValueWithEmbedTypeSettingV1() throws Exception { + // java.lang.Class is held out of the derived names, and no separate list holds it out permanently, so a + // caller that wants GraphSON 1.0 to read one names it like any other name + final String json = "{\"@class\":\"java.util.HashMap\",\"c\":[\"java.lang.Class\",\"java.lang.String\"]}"; + assertDeniedByTypeValidator(v1Typed(), json); + + final ObjectMapper mapper = GraphSONMapper.build().version(GraphSONVersion.V1_0) + .typeInfo(TypeInfo.PARTIAL_TYPES).addAllowedTypeIdName("java.lang.Class").create().createMapper(); + assertEquals(String.class, mapper.readValue(json, HashMap.class).get("c")); + } + + @Test + public void shouldAllowSeveralConfiguredTypeIdNamesWithEmbedTypeSettingV1() throws Exception { + // addAllowedTypeIdName takes several names at once, and successive calls add to earlier ones rather than + // replacing them + final String json = "{\"@class\":\"java.util.HashMap\"," + + "\"p\":{\"@class\":\"com.example.gadget.GraphSONTestGadgets$SamplePojo\",\"x\":42}," + + "\"u\":[\"java.net.URL\",\"http://example.com/x\"]}"; + + assertDeniedByTypeValidator(v1Typed(), json); + + assertConfiguredNamesResolve(GraphSONMapper.build().version(GraphSONVersion.V1_0) + .typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName(SamplePojo.class.getName(), "java.net.URL") + .create().createMapper(), json); + + assertConfiguredNamesResolve(GraphSONMapper.build().version(GraphSONVersion.V1_0) + .typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName(SamplePojo.class.getName()) + .addAllowedTypeIdName("java.net.URL") + .create().createMapper(), json); + } + + @Test + public void shouldIgnoreDuplicateConfiguredTypeIdNamesWithEmbedTypeSettingV1() throws Exception { + final String json = "{\"@class\":\"java.util.HashMap\"," + + "\"p\":{\"@class\":\"com.example.gadget.GraphSONTestGadgets$SamplePojo\",\"x\":42}}"; + + final ObjectMapper mapper = GraphSONMapper.build().version(GraphSONVersion.V1_0) + .typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdName(SamplePojo.class.getName()) + .addAllowedTypeIdName(SamplePojo.class.getName()) + .create().createMapper(); + + assertEquals(new SamplePojo(42), mapper.readValue(json, HashMap.class).get("p")); + } + + @Test + public void shouldDeriveTypeIdNamesFromTheRegisteredGraphSON2And3TypesV1() { + // a new put(...) in GraphSONModuleV2, GraphSONModuleV3, GraphSONXModuleV2 or GraphSONXModuleV3 widens what + // GraphSON 1.0 reads as a side effect, which pinning the derived names makes visible. + // + // The set is classpath dependent: GraphSONModule.tryLoadSparqlStrategy() contributes SparqlStrategy when + // sparql-gremlin is present, which it is not on the gremlin-core test classpath. A new name that is genuinely + // wanted belongs in the expected set below. + final Set expected = new TreeSet<>(Arrays.asList( + "java.lang.Byte", + "java.lang.Character", + "java.lang.Double", + "java.lang.Float", + "java.lang.Integer", + "java.lang.Long", + "java.lang.Short", + "java.math.BigDecimal", + "java.math.BigInteger", + "java.net.InetAddress", + "java.nio.ByteBuffer", + "java.sql.Timestamp", + "java.time.Duration", + "java.time.Instant", + "java.time.LocalDate", + "java.time.LocalDateTime", + "java.time.LocalTime", + "java.time.MonthDay", + "java.time.OffsetDateTime", + "java.time.OffsetTime", + "java.time.Period", + "java.time.Year", + "java.time.YearMonth", + "java.time.ZoneOffset", + "java.time.ZonedDateTime", + "java.util.Calendar", + "java.util.Date", + "java.util.List", + "java.util.Map", + "java.util.Set", + "java.util.TimeZone", + "java.util.UUID", + "org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.decoration.VertexProgramStrategy", + "org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.optimization.GraphFilterStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.Bytecode", + "org.apache.tinkerpop.gremlin.process.traversal.Bytecode$Binding", + "org.apache.tinkerpop.gremlin.process.traversal.DT", + "org.apache.tinkerpop.gremlin.process.traversal.Merge", + "org.apache.tinkerpop.gremlin.process.traversal.Operator", + "org.apache.tinkerpop.gremlin.process.traversal.Order", + "org.apache.tinkerpop.gremlin.process.traversal.P", + "org.apache.tinkerpop.gremlin.process.traversal.Path", + "org.apache.tinkerpop.gremlin.process.traversal.Pick", + "org.apache.tinkerpop.gremlin.process.traversal.Pop", + "org.apache.tinkerpop.gremlin.process.traversal.SackFunctions$Barrier", + "org.apache.tinkerpop.gremlin.process.traversal.Scope", + "org.apache.tinkerpop.gremlin.process.traversal.TextP", + "org.apache.tinkerpop.gremlin.process.traversal.Traverser", + "org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet", + "org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.ConnectiveStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.ElementIdStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.EventStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.HaltedTraverserStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.PartitionStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SeedStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.finalization.MatchAlgorithmStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.AdjacentToIncidentStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.ByModulatorOptimizationStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.CountStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.EarlyLimitStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.FilterRankingStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.IdentityRemovalStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.IncidentToAdjacentStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.InlineFilterStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.LazyBarrierStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.MatchPredicateStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.OrderLimitStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.PathProcessorStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.PathRetractionStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.ProductiveByStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.RepeatUnrollStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ComputerVerificationStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.EdgeLabelVerificationStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.LambdaRestrictionStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReadOnlyStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReservedKeysVerificationStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.StandardVerificationStrategy", + "org.apache.tinkerpop.gremlin.process.traversal.util.AndP", + "org.apache.tinkerpop.gremlin.process.traversal.util.Metrics", + "org.apache.tinkerpop.gremlin.process.traversal.util.OrP", + "org.apache.tinkerpop.gremlin.process.traversal.util.TraversalExplanation", + "org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics", + "org.apache.tinkerpop.gremlin.structure.Column", + "org.apache.tinkerpop.gremlin.structure.Direction", + "org.apache.tinkerpop.gremlin.structure.Edge", + "org.apache.tinkerpop.gremlin.structure.Property", + "org.apache.tinkerpop.gremlin.structure.T", + "org.apache.tinkerpop.gremlin.structure.Vertex", + "org.apache.tinkerpop.gremlin.structure.VertexProperty", + "org.apache.tinkerpop.gremlin.structure.VertexProperty$Cardinality", + "org.apache.tinkerpop.gremlin.util.function.Lambda")); + + final Set derived = new TreeSet<>(GraphSONMapper.graphSON1dDerivedTypeNames()); + final Set added = new TreeSet<>(derived); + added.removeAll(expected); + final Set dropped = new TreeSet<>(expected); + dropped.removeAll(derived); + assertEquals("derived but not expected: " + added + "; expected but not derived: " + dropped, + expected, derived); + } + + private static ObjectMapper v1Typed() { + return GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES).create().createMapper(); + } + + /** + * Writes a value as a {@code Map} value and reads it back. That is the shape real payloads use, and it exercises + * the untyped {@code Object} value path rather than a declared concrete class. + */ + private static Object roundTripInMap(final ObjectMapper mapper, final Object value) throws Exception { + return readMapValue(mapper, writeInMap(mapper, value)); + } + + /** + * Writes a value as a {@code Map} value and returns the text, so the type id GraphSON 1.0 emitted for it can be + * asserted on directly. + */ + private static String writeInMap(final ObjectMapper mapper, final Object value) throws Exception { + final Map m = new HashMap<>(); + m.put(MAP_VALUE_KEY, value); + return mapper.writeValueAsString(m); + } + + private static void assertRoundTripsInMap(final ObjectMapper mapper, final List values) throws Exception { + for (final Object value : values) { + assertEquals(value.getClass().getName(), value, roundTripInMap(mapper, value)); + } + } + + private static void assertTypedRoundTripInMap(final ObjectMapper mapper, final Object value) throws Exception { + assertTypedRoundTripInMap(mapper, value, value.getClass()); + } + + /** + * Asserts that a {@code Map} or {@code Collection} value reads back equal and as {@code expectedClass}, and that + * the type id GraphSON 1.0 wrote for it is in the text written. Equality alone is no evidence of typing, as an + * equal value of any other concrete {@code Map} or {@code List} class satisfies it. {@code expectedClass} is what + * Jackson rebuilds for the type id written, which is not always the class that was written. + */ + private static void assertTypedRoundTripInMap(final ObjectMapper mapper, final Object value, + final Class expectedClass) throws Exception { + final String name = value.getClass().getName(); + final String json = writeInMap(mapper, value); + assertThat("no type id written for " + name + " in " + json, json, containsString(typeIdInMapOf(value))); + + final Object read = readMapValue(mapper, json); + assertEquals(name, value, read); + assertEquals("concrete class read back for " + name, expectedClass, read.getClass()); + } + + /** + * The text GraphSON 1.0 writes for the type id of a {@code Map} or {@code Collection} held as a {@code Map} + * value: a {@code Map} carries it as an {@code "@class"} field of its own object, a {@code Collection} as the + * first element of a wrapper array. Both are anchored to the key the value sits under, so neither can be + * satisfied by the type id of the enclosing {@code Map}. + */ + private static String typeIdInMapOf(final Object value) { + final String prefix = "\"" + MAP_VALUE_KEY + "\":"; + final String name = value.getClass().getName(); + return value instanceof Map + ? prefix + "{\"" + GraphSONTokens.CLASS + "\":\"" + name + "\"" + : prefix + "[\"" + name + "\","; + } + + private static Object readMapValue(final ObjectMapper mapper, final String json) throws Exception { + return mapper.readValue(json, HashMap.class).get(MAP_VALUE_KEY); + } + + /** + * Asserts that both configured names resolved. A {@code java.net.URL} is compared as text, since + * {@code URL.equals} can consult the network. + */ + private static void assertConfiguredNamesResolve(final ObjectMapper mapper, final String json) throws Exception { + final Map read = mapper.readValue(json, HashMap.class); + assertEquals(new SamplePojo(42), read.get("p")); + assertEquals("http://example.com/x", read.get("u").toString()); + } + + private static void assertDeniedByTypeValidator(final ObjectMapper mapper, final String json) { + try { + mapper.readValue(json, HashMap.class); + fail("a @class outside the allowed names must not resolve"); + } catch (InvalidTypeIdException expected) { + } catch (Exception other) { + throw new AssertionError("expected InvalidTypeIdException, got " + other, other); + } + } + + /** + * Asserts that {@code typeId} in particular is what the read was refused on, and that the allowed names are what + * refused it. A listed type id that names no loadable class is also an {@code InvalidTypeIdException}, reported as + * "no such class found" rather than as a denial. Both strings matched are Jackson's own wording, so a Jackson + * upgrade can move them. + */ + private static void assertTypeIdDeniedByTypeValidator(final ObjectMapper mapper, final String json, + final String typeId) { + try { + mapper.readValue(json, HashMap.class); + fail("resolution of the type id " + typeId + " must be refused"); + } catch (InvalidTypeIdException expected) { + assertThat(expected.getMessage(), containsString("Could not resolve type id '" + typeId + "'")); + assertThat(expected.getMessage(), containsString("denied resolution")); + } catch (Exception other) { + throw new AssertionError("expected InvalidTypeIdException, got " + other, other); + } + } + + /** + * Asserts that {@code typeId} was refused for being parameterized, which {@code GraphSON1dScreeningIdResolver} + * does before the allowed names are consulted at all. + */ + private static void assertParameterizedTypeIdRefused(final ObjectMapper mapper, final String json, + final String typeId) { + try { + mapper.readValue(json, HashMap.class); + fail("a parameterized type id must be refused: " + typeId); + } catch (InvalidTypeIdException expected) { + assertThat(expected.getMessage(), containsString("Could not resolve type id '" + typeId + "'")); + assertThat(expected.getMessage(), containsString("GraphSON 1.0 does not permit a parameterized type id")); + } catch (Exception other) { + throw new AssertionError("expected InvalidTypeIdException, got " + other, other); + } + } + @Test public void shouldNotHandleMapWithTypesUsingEmbedTypeSettingV1() throws Exception { final ObjectMapper mapper = GraphSONMapper.build() diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterConfigTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterConfigTest.java index f6a5dbfd00d..418f883e1e3 100644 --- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterConfigTest.java +++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterConfigTest.java @@ -20,17 +20,63 @@ import org.apache.commons.configuration2.BaseConfiguration; import org.apache.commons.configuration2.Configuration; +import org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1; import org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV2; import org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV3; import org.apache.tinkerpop.shaded.jackson.core.StreamReadConstraints; +import org.apache.tinkerpop.shaded.jackson.databind.exc.InvalidTypeIdException; import org.junit.Test; +import java.awt.Point; import java.util.Arrays; +import java.util.Map; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class ClusterConfigTest { + private static final String POINT_JSON = + "{\"@class\":\"java.util.HashMap\",\"point\":{\"@class\":\"java.awt.Point\",\"x\":1.0,\"y\":2.0}}"; + + @Test + public void shouldPropagateAllowedTypeIdNamesForGraphSON1() throws Exception { + final Configuration config = createGraphSON1Config(); + config.setProperty("serializer.config.allowedTypeIdNames", Arrays.asList("java.awt.Point")); + + final Cluster cluster = Cluster.open(config); + try { + assertTrue(cluster.getSerializer() instanceof GraphSONMessageSerializerV1); + final GraphSONMessageSerializerV1 serializer = + (GraphSONMessageSerializerV1) cluster.getSerializer(); + final Map deserialized = serializer.getMapper().readValue(POINT_JSON, Map.class); + + assertEquals(new Point(1, 2), deserialized.get("point")); + } finally { + cluster.close(); + } + } + + @Test + public void shouldRejectUnregisteredTypeIdNamesForGraphSON1() { + final Cluster cluster = Cluster.open(createGraphSON1Config()); + try { + assertTrue(cluster.getSerializer() instanceof GraphSONMessageSerializerV1); + final GraphSONMessageSerializerV1 serializer = + (GraphSONMessageSerializerV1) cluster.getSerializer(); + + try { + serializer.getMapper().readValue(POINT_JSON, Map.class); + fail("an unregistered type id name should not resolve"); + } catch (Exception ex) { + assertThat(ex, instanceOf(InvalidTypeIdException.class)); + } + } finally { + cluster.close(); + } + } @Test public void shouldPropagateSerializerConstraintsForGraphSON3() { @@ -69,4 +115,12 @@ public void shouldPropagateSerializerConstraintsForGraphSON2() { assertEquals(123456, constraints.getMaxStringLength()); assertEquals(55, constraints.getMaxNestingDepth()); } + + private static Configuration createGraphSON1Config() { + final Configuration config = new BaseConfiguration(); + config.setProperty("serializer.className", GraphSONMessageSerializerV1.class.getCanonicalName()); + // Cluster construction requires a host, but these tests do not connect to it. + config.setProperty("hosts", Arrays.asList("localhost")); + return config; + } } diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java index 5b83e2d4b24..afff9236fec 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java @@ -969,7 +969,21 @@ public void shouldSerializeToStringWhenRequestedGraphBinaryV1() throws Exception } @Test - public void shouldWorkWithGraphSONV1Serialization() throws Exception { + public void shouldWorkWithGraphSONV1TypedSerialization() throws Exception { + final Cluster cluster = TestClientFactory.build().serializer(Serializers.GRAPHSON_V1).create(); + final Client client = cluster.connect(); + + try { + final List results = client.submit("g.inject(2)").all().join(); + assertEquals(1, results.size()); + assertEquals(2, results.get(0).getInt()); + } finally { + cluster.close(); + } + } + + @Test + public void shouldWorkWithGraphSONV1UntypedSerialization() throws Exception { final Cluster cluster = TestClientFactory.build().serializer(Serializers.GRAPHSON_V1_UNTYPED).create(); final Client client = cluster.connect(); diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/SettingsTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/SettingsTest.java index d399a5ad657..5642be5ccef 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/SettingsTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/SettingsTest.java @@ -18,13 +18,22 @@ */ package org.apache.tinkerpop.gremlin.server; +import org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1; +import org.apache.tinkerpop.shaded.jackson.databind.exc.InvalidTypeIdException; import org.junit.Test; import org.yaml.snakeyaml.Yaml; import org.yaml.snakeyaml.constructor.Constructor; +import java.awt.Dimension; +import java.awt.Point; import java.io.InputStream; +import java.util.Collections; +import java.util.Map; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; public class SettingsTest { @@ -56,4 +65,39 @@ public void defaultCustomValuesAreHandledCorrectly() throws Exception { assertEquals("localhost", settings.customValue); } + + @Test + public void shouldConfigureAllowedTypeIdNamesFromYaml() throws Exception { + final GraphSONMessageSerializerV1 serializer = createGraphSONV1SerializerFromYaml(); + final String json = "{\"@class\":\"java.util.HashMap\",\"point\":{\"@class\":\"java.awt.Point\"," + + "\"x\":1.0,\"y\":2.0}}"; + final Map deserialized = serializer.getMapper().readValue(json, Map.class); + + assertEquals(new Point(1, 2), deserialized.get("point")); + } + + @Test + public void shouldRejectUnregisteredTypeIdNamesFromYaml() { + final GraphSONMessageSerializerV1 serializer = createGraphSONV1SerializerFromYaml(); + final String json = "{\"@class\":\"java.util.HashMap\",\"dimension\":{\"@class\":\"" + + Dimension.class.getName() + "\",\"width\":1,\"height\":2}}"; + + try { + serializer.getMapper().readValue(json, Map.class); + fail("an unregistered type id name should not resolve"); + } catch (Exception ex) { + assertThat(ex, instanceOf(InvalidTypeIdException.class)); + } + } + + private static GraphSONMessageSerializerV1 createGraphSONV1SerializerFromYaml() { + final InputStream stream = SettingsTest.class.getResourceAsStream("gremlin-server-integration.yaml"); + final Settings settings = Settings.read(stream); + final Settings.SerializerSettings serializerSettings = settings.serializers.stream() + .filter(s -> GraphSONMessageSerializerV1.class.getName().equals(s.className)) + .findFirst().orElseThrow(IllegalStateException::new); + final GraphSONMessageSerializerV1 serializer = new GraphSONMessageSerializerV1(); + serializer.configure(serializerSettings.config, Collections.emptyMap()); + return serializer; + } } diff --git a/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml b/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml index 15904893cdd..23481cf3d15 100644 --- a/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml +++ b/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml @@ -53,6 +53,7 @@ serializers: - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV3, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV3] }} - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV2, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV2] }} - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONUntypedMessageSerializerV1, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV1] }} + - { className: org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1, config: { ioRegistries: [org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV1], allowedTypeIdNames: [java.awt.Point] }} - { className: org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1 } - { className: org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1, config: { serializeResultToString: true }} processors: diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoCustomTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoCustomTest.java index 882cd0ee4e9..e70a297f2fd 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoCustomTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoCustomTest.java @@ -64,9 +64,10 @@ public static Iterable data() { final SimpleModule modulev3 = new CustomId.CustomIdTinkerPopJacksonModuleV3(); return Arrays.asList(new Object[][]{ + // CustomId is not among the default GraphSON 1.0 type id names, so it is added {"graphson-v1-embedded", true, - (Function) g -> g.io(GraphSONIo.build(GraphSONVersion.V1_0)).reader().mapper(g.io(GraphSONIo.build(GraphSONVersion.V1_0)).mapper().addCustomModule(moduleV1).typeInfo(TypeInfo.PARTIAL_TYPES).create()).create(), - (Function) g -> g.io(GraphSONIo.build(GraphSONVersion.V1_0)).writer().mapper(g.io(GraphSONIo.build(GraphSONVersion.V1_0)).mapper().addCustomModule(moduleV1).typeInfo(TypeInfo.PARTIAL_TYPES).create()).create()}, + (Function) g -> g.io(GraphSONIo.build(GraphSONVersion.V1_0)).reader().mapper(g.io(GraphSONIo.build(GraphSONVersion.V1_0)).mapper().addCustomModule(moduleV1).typeInfo(TypeInfo.PARTIAL_TYPES).addAllowedTypeIdName(CustomId.class.getName()).create()).create(), + (Function) g -> g.io(GraphSONIo.build(GraphSONVersion.V1_0)).writer().mapper(g.io(GraphSONIo.build(GraphSONVersion.V1_0)).mapper().addCustomModule(moduleV1).typeInfo(TypeInfo.PARTIAL_TYPES).addAllowedTypeIdName(CustomId.class.getName()).create()).create()}, {"graphson-v2-embedded", true, (Function) g -> g.io(GraphSONIo.build(GraphSONVersion.V2_0)).reader().mapper(g.io(GraphSONIo.build(GraphSONVersion.V2_0)).mapper().addCustomModule(moduleV2).typeInfo(TypeInfo.PARTIAL_TYPES).create()).create(), (Function) g -> g.io(GraphSONIo.build(GraphSONVersion.V2_0)).writer().mapper(g.io(GraphSONIo.build(GraphSONVersion.V2_0)).mapper().addCustomModule(moduleV2).typeInfo(TypeInfo.PARTIAL_TYPES).create()).create()}, diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java index 6078b1459b7..73046f31466 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java @@ -578,8 +578,10 @@ public void shouldProperlySerializeCustomIdWithGraphSON() throws Exception { final SimpleModule module = new SimpleModule(); module.addSerializer(CustomId.class, new CustomId.CustomIdJacksonSerializerV1()); + // CustomId is not among the default GraphSON 1.0 type id names, so it is added below and again on the + // reader final GraphWriter writer = graph.io(graphson).writer().mapper( - graph.io(graphson).mapper().version(GraphSONVersion.V1_0).addCustomModule(module).typeInfo(TypeInfo.PARTIAL_TYPES).create()).create(); + graph.io(graphson).mapper().version(GraphSONVersion.V1_0).addCustomModule(module).typeInfo(TypeInfo.PARTIAL_TYPES).addAllowedTypeIdName(CustomId.class.getName()).create()).create(); try (final ByteArrayOutputStream baos = new ByteArrayOutputStream()) { writer.writeGraph(baos, graph); @@ -598,7 +600,7 @@ public void shouldProperlySerializeCustomIdWithGraphSON() throws Exception { try (final InputStream is = new ByteArrayInputStream(baos.toByteArray())) { final GraphReader reader = graph.io(graphson).reader() - .mapper(graph.io(graphson).mapper().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES).addCustomModule(module).create()).create(); + .mapper(graph.io(graphson).mapper().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES).addCustomModule(module).addAllowedTypeIdName(CustomId.class.getName()).create()).create(); reader.readGraph(is, g2); } diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV1.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV1.java index c2a0de62fdf..da9d111d21e 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV1.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV1.java @@ -44,6 +44,7 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.UUID; @@ -51,6 +52,8 @@ * @author Stephen Mallette (http://stephen.genoprime.com) */ public abstract class AbstractGraphSONMessageSerializerV1 extends AbstractMessageSerializer { + static final String TOKEN_ALLOWED_TYPE_ID_NAMES = "allowedTypeIdNames"; + private static final Logger logger = LoggerFactory.getLogger(AbstractGraphSONMessageSerializerV1.class); protected ObjectMapper mapper; @@ -76,6 +79,7 @@ public void configure(final Map config, final Map final GraphSONMapper.Builder initialBuilder = initBuilder(null); addIoRegistries(config, initialBuilder); applyMaxTokenLimits(initialBuilder, config); + applyAllowedTypeIdNames(initialBuilder, config); mapper = configureBuilder(initialBuilder).create().createMapper(); } @@ -183,6 +187,13 @@ private GraphSONMapper.Builder applyMaxTokenLimits(final GraphSONMapper.Builder return builder; } + private GraphSONMapper.Builder applyAllowedTypeIdNames(final GraphSONMapper.Builder builder, + final Map config) { + final List allowedTypeIdNames = getListStringFromConfig(TOKEN_ALLOWED_TYPE_ID_NAMES, config); + builder.addAllowedTypeIdName(allowedTypeIdNames.toArray(new String[0])); + return builder; + } + @Override public ObjectMapper getMapper() { return this.mapper; diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractMessageSerializer.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractMessageSerializer.java index 03f570aa5c6..76139b0fddb 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractMessageSerializer.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractMessageSerializer.java @@ -88,9 +88,13 @@ protected Method tryInstanceMethod(final Class clazz) { * Gets a {@link List} of strings from the configuration object. */ protected List getListStringFromConfig(final String token, final Map config) { + final Object value = config.getOrDefault(token, Collections.emptyList()); + if (value instanceof String) + return Collections.singletonList((String) value); + final List classNameList; try { - classNameList = (List) config.getOrDefault(token, Collections.emptyList()); + classNameList = (List) value; } catch (Exception ex) { throw new IllegalStateException(String.format("Invalid configuration value of [%s] for [%s] setting on %s serialization configuration", config.getOrDefault(token, ""), token, this.getClass().getName()), ex); @@ -98,4 +102,4 @@ protected List getListStringFromConfig(final String token, final Map serializer = new GraphSONMessageSerializerV1(); + @Test + public void shouldRejectUnregisteredTypeIdNamesAtMessageBoundaries() throws Exception { + final GraphSONMessageSerializerV1 defaultSerializer = new GraphSONMessageSerializerV1(); + final RequestMessage request = RequestMessage.build("eval").addArg("sample", new SamplePojo(123)).create(); + final ByteBuf requestBuffer = defaultSerializer.serializeRequestAsBinary(request, allocator); + // The server decoder removes the MIME header before passing the JSON payload to deserializeRequest(). + final int mimeLength = requestBuffer.readByte(); + requestBuffer.skipBytes(mimeLength); + + try { + defaultSerializer.deserializeRequest(requestBuffer); + fail("an unregistered request type id name should not resolve"); + } catch (SerializationException expected) { + assertEquals(InvalidTypeIdException.class, expected.getCause().getClass()); + assertTrue(expected.getCause().getMessage().contains(SamplePojo.class.getName())); + } + + final ResponseMessage response = ResponseMessage.build(requestId).result(new SamplePojo(123)).create(); + final ByteBuf responseBuffer = defaultSerializer.serializeResponseAsBinary(response, allocator); + try { + defaultSerializer.deserializeResponse(responseBuffer); + fail("an unregistered response type id name should not resolve"); + } catch (SerializationException expected) { + assertEquals(InvalidTypeIdException.class, expected.getCause().getClass()); + assertTrue(expected.getCause().getMessage().contains(SamplePojo.class.getName())); + } + } + + @Test + public void shouldConfigureAllowedTypeIdNamesAtMessageBoundaries() throws Exception { + final GraphSONMessageSerializerV1 configuredSerializer = new GraphSONMessageSerializerV1(); + final Map config = new HashMap<>(); + config.put(AbstractGraphSONMessageSerializerV1.TOKEN_ALLOWED_TYPE_ID_NAMES, + Collections.singletonList(SamplePojo.class.getName())); + configuredSerializer.configure(config, null); + + final RequestMessage request = RequestMessage.build("eval").addArg("sample", new SamplePojo(123)).create(); + final ByteBuf requestBuffer = configuredSerializer.serializeRequestAsBinary(request, allocator); + // The server decoder removes the MIME header before passing the JSON payload to deserializeRequest(). + final int mimeLength = requestBuffer.readByte(); + requestBuffer.skipBytes(mimeLength); + final SamplePojo requestSample = configuredSerializer.deserializeRequest(requestBuffer).getArg("sample"); + + final ResponseMessage response = ResponseMessage.build(requestId).result(new SamplePojo(123)).create(); + final ByteBuf responseBuffer = configuredSerializer.serializeResponseAsBinary(response, allocator); + final SamplePojo responseSample = (SamplePojo) configuredSerializer.deserializeResponse(responseBuffer) + .getResult().getData(); + + assertEquals(123, requestSample.value); + assertEquals(123, responseSample.value); + } + @Test public void shouldSerializeIterable() throws Exception { final ArrayList list = new ArrayList<>(); @@ -332,4 +388,15 @@ private ResponseMessage convert(final Object toSerialize) throws SerializationEx final ByteBuf bb = serializer.serializeResponseAsBinary(responseMessageBuilder.result(toSerialize).create(), allocator); return serializer.deserializeResponse(bb); } + + public static class SamplePojo { + public int value; + + public SamplePojo() { + } + + public SamplePojo(final int value) { + this.value = value; + } + } }