This is an automated email from the ASF dual-hosted git repository.
Cole-Greer pushed a commit to branch 3.7-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.7-dev by this push:
new d557266e40 Restrict Class value deserialization in GraphBinary,
GraphSON, and Gryo
d557266e40 is described below
commit d557266e4046672cbb5ed98fab56135ab87c38d2
Author: Guian Gumpac <[email protected]>
AuthorDate: Fri Aug 28 16:07:47 2026 -0700
Restrict Class value deserialization in GraphBinary, GraphSON, and Gryo
Added class registry that builds on top of the TraversalStrategy registry
Assisted-by: Claude Opus 5
---
CHANGELOG.asciidoc | 1 +
docs/src/reference/gremlin-applications.asciidoc | 21 ++
docs/src/reference/the-traversal.asciidoc | 5 +-
docs/src/upgrade/release-3.7.x.asciidoc | 16 +
.../gremlin/structure/io/ClassRegistry.java | 119 +++++++
.../structure/io/binary/types/ClassSerializer.java | 10 +-
.../io/graphson/ClassJacksonDeserializer.java | 54 +++
.../structure/io/graphson/GraphSONModule.java | 6 +
.../gremlin/structure/io/gryo/UtilSerializers.java | 16 +-
.../gremlin/process/TraversalStrategiesTest.java | 2 +-
.../gremlin/structure/io/ClassRegistryTest.java | 246 ++++++++++++++
.../io/graphson/ClassJacksonDeserializerTest.java | 219 +++++++++++++
.../structure/io/gryo/ClassSerializerTest.java | 250 ++++++++++++++
.../gremlin/structure/io/gryo/GryoMapperTest.java | 3 +-
gremlin-go/driver/connection_test.go | 2 +-
.../tinkerpop/gremlin/structure/io/Model.java | 4 +-
.../graphbinary/GraphBinaryCompatibilityTest.java | 22 ++
.../gremlin/util/ser/AbstractRoundTripTest.java | 2 +-
.../util/ser/binary/types/ClassSerializerTest.java | 361 +++++++++++++++++++++
.../types/TraversalStrategySerializerTest.java | 29 ++
.../{class-v1.gbin => class-unregistered-v1.gbin} | Bin
.../gremlin/structure/io/graphbinary/class-v1.gbin | Bin 18 -> 89 bytes
.../gremlin/structure/io/graphson/class-v2.json | 2 +-
.../gremlin/structure/io/graphson/class-v3.json | 2 +-
24 files changed, 1367 insertions(+), 25 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index b6865fa065..21474400d4 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -51,6 +51,7 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Deprecated `ProductiveByStrategy` which was introduced as a temporary way to
mimic pre-3.5.0 null processing behavior.
* Backported `TraversalStrategy` registration mechanism in
`TraversalStrategies` from the 3.8.x line.
* Restricted GraphBinary `TraversalStrategy` deserialization to registered
strategies that have not been denied with `denyStrategy()`.
+* Restricted GraphBinary, GraphSON and Gryo `Class` deserialization to classes
registered with `ClassRegistry.register()`, `registerStrategy()` or
`registerStrategies()`.
* Fixed `gremlin-python` GraphBinary serialization of
`BigInteger`/`BigDecimal` negative boundary values (e.g. `-129`) that raised
`OverflowError`.
* Fixed `gremlin-go` GraphBinary serialization of zero
`BigInteger`/`BigDecimal` values, which were encoded with zero length and
rejected by Java servers.
diff --git a/docs/src/reference/gremlin-applications.asciidoc
b/docs/src/reference/gremlin-applications.asciidoc
index cc80ae32e9..bdf8263b4d 100644
--- a/docs/src/reference/gremlin-applications.asciidoc
+++ b/docs/src/reference/gremlin-applications.asciidoc
@@ -1294,6 +1294,27 @@ construct the strategy from its configuration, so all
strategies should be regis
As described above, there are multiple ways in which to register serializers
for GraphBinary-based serialization. Note
that the `ioRegistries` setting is applied first, followed by the `custom`
setting.
+[[class-registry]]
+===== Class Registry
+
+A `Class` value in GraphBinary, GraphSON or Gryo is resolved through
`ClassRegistry` rather than through a class
+loader, so it can only name a class registered in advance. A name the registry
does not hold is refused and the named
+class is never loaded.
+
+[source,java]
+----
+ClassRegistry.register(MyClass.class);
+ClassRegistry.unregister(MyClass.class);
+----
+
+Registration is required in Java on the deserialization path only.
`ClassRegistry` matches the fully qualified class
+name exactly, so registering a superclass or an interface does not admit its
subclasses or implementors. Each concrete
+class that is deserialized needs its own registration.
+
+A `TraversalStrategy` is not registered here. Strategies belong to
`TraversalStrategies.GlobalCache.GLOBAL_REGISTRY`,
+which `ClassRegistry.lookup` also reads, so a registered strategy is nameable
already and passing one to
+`ClassRegistry.register` or `unregister` throws `IllegalArgumentException`.
+
[[metrics]]
==== Metrics
diff --git a/docs/src/reference/the-traversal.asciidoc
b/docs/src/reference/the-traversal.asciidoc
index 52945808a8..18c53baa3e 100644
--- a/docs/src/reference/the-traversal.asciidoc
+++ b/docs/src/reference/the-traversal.asciidoc
@@ -2301,6 +2301,8 @@ g.io("graph.json").write().iterate();
NOTE: Additional documentation for GraphSON can be found in the
link:https://tinkerpop.apache.org/docs/x.y.z/dev/io/#graphson[IO Reference].
+NOTE: A `Class` stored as a property value can only be read back if it is
registered, see <<class-registry,Class Registry>>.
+
anchor:gryo-reader-writer[]
[[gryo]]
==== Gryo
@@ -2326,6 +2328,8 @@ g.io("graph.kryo").read().iterate()
g.io("graph.kryo").write().iterate()
----
+NOTE: A `Class` stored as a property value can only be read back if it is
registered, see <<class-registry,Class Registry>>.
+
*Additional References*
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.html#io(java.lang.String)++[`io(String)`]
@@ -6310,7 +6314,6 @@ Type ':help' or ':h' for help.
Display stack trace? [yN]
----
-
[[dsl]]
== Domain Specific Languages
diff --git a/docs/src/upgrade/release-3.7.x.asciidoc
b/docs/src/upgrade/release-3.7.x.asciidoc
index 93cd5fd283..f59e366613 100644
--- a/docs/src/upgrade/release-3.7.x.asciidoc
+++ b/docs/src/upgrade/release-3.7.x.asciidoc
@@ -225,6 +225,22 @@ Providers that send custom strategies over GraphBinary
must register every strat
using `registerStrategies()` or `registerStrategy()`, as described above. The
presence of a strategy on the application
class path is no longer sufficient.
+===== Class Values Must Be Registered
+
+For deserialization, a `Class` value must now name a registered class, in
GraphBinary, GraphSON and Gryo alike. An unregistered name is
+refused on read and the class is never loaded. Strategies registered with
`TraversalStrategies.GlobalCache.registerStrategy` already
+resolve, so only an application or provider that serializes its own classes
needs to act.
+
+[source,java]
+----
+ClassRegistry.register(MyClass.class);
+ClassRegistry.unregister(MyClass.class);
+----
+
+Registration is only required for in Java for deserialization, which typically
only impacts the server. Without
+registration, read attempts fail with a `Class not recognized` message naming
the class, raised as an
+`IOException` for GraphBinary, a `MismatchedInputException` for GraphSON and a
`RuntimeException` for Gryo.
+
== TinkerPop 3.7.6
*Release Date: April 1, 2026*
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/ClassRegistry.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/ClassRegistry.java
new file mode 100644
index 0000000000..931c7e81ab
--- /dev/null
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/ClassRegistry.java
@@ -0,0 +1,119 @@
+/*
+ * 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;
+
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep;
+import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceFactory;
+
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Holds the classes a GraphBinary, GraphSON or Gryo {@code Class} value may
name, so such a name is resolved from
+ * this registry rather than by a class loader. Registration is a Java API
only and refuses a
+ * {@link TraversalStrategy}, which belongs to {@link
TraversalStrategies.GlobalCache}.
+ * <p/>
+ * {@link #lookup(String)} also falls back to {@code
GlobalCache.getRegisteredStrategyClassByFullName}, so a strategy
+ * the selector can construct is nameable without being registered here.
+ */
+public final class ClassRegistry {
+
+ private ClassRegistry() {
+ }
+
+ /**
+ * Fully qualified names ({@link Class#getName()}) of the classes a
serialized traversal may name as a generic
+ * {@code Class} value but not construct. {@link #lookup(String)} guards a
{@code null} name itself, because a
+ * {@code ConcurrentHashMap} refuses a {@code null} key.
+ */
+ private static final Map<String, Class<?>> REGISTRY = new
ConcurrentHashMap<>();
+
+ // this block writes REGISTRY through register(), so the field above has
to be declared first.
+ static {
+ // Gryo names these two out of MatchAlgorithmStrategy's
matchAlgorithmClass field, the count one being what that
+ // strategy's builder holds when no algorithm is named.
+ register(MatchStep.GreedyMatchAlgorithm.class);
+ register(MatchStep.CountMatchAlgorithm.class);
+
+ // Gryo names these two out of HaltedTraverserStrategy's
haltedTraverserFactory field, which holds one or the
+ // other and nothing else.
+ register(DetachedFactory.class);
+ register(ReferenceFactory.class);
+ }
+
+ /**
+ * Registers a class as nameable as a generic {@code Class} value, without
making it constructible from a serialized
+ * traversal, so it cannot widen what the selector can construct. A {@link
TraversalStrategy} is refused with an
+ * {@link IllegalArgumentException} and a {@code null} with a {@link
NullPointerException}.
+ */
+ public static void register(final Class<?> clazz) {
+ Objects.requireNonNull(clazz, "clazz can not be null");
+ rejectStrategy(clazz);
+
+ REGISTRY.put(clazz.getName(), clazz);
+ }
+
+ /**
+ * Removes a class from this registry, so it is no longer nameable as a
generic {@code Class} value. A
+ * {@link TraversalStrategy} is refused because strategies belong to
{@link TraversalStrategies.GlobalCache}, and a
+ * {@code null} is refused with a {@link NullPointerException}.
+ */
+ public static void unregister(final Class<?> clazz) {
+ Objects.requireNonNull(clazz, "clazz can not be null");
+ rejectStrategy(clazz);
+
+ REGISTRY.remove(clazz.getName());
+ }
+
+ /**
+ * Looks up a class nameable as a generic {@code Class} value, without
loading it. Matching is on the exact
+ * {@link Class#getName()}, never assignability, so registering a
superclass or an interface does not admit its
+ * subtypes. A {@code null} name yields an empty {@link Optional}, since a
{@code Class} value can name nothing.
+ */
+ public static Optional<Class<?>> lookup(final String className) {
+ // a null key would throw, and it can match nothing, so it leaves
before the fall-back and its initialization.
+ if (null == className)
+ return Optional.empty();
+
+ final Class<?> clazz = REGISTRY.get(className);
+
+ // this call also forces GlobalCache initialization, which registers
the strategies from registerStrategies().
+ // Leave it here: a cached copy of what it reads would not.
+ return null != clazz
+ ? Optional.of(clazz)
+ :
TraversalStrategies.GlobalCache.getRegisteredStrategyClassByFullName(className)
+ .map(c -> (Class<?>) c);
+ }
+
+ /**
+ * Refuses a {@link TraversalStrategy}, so that a caller who reaches for
this registry is sent to the one the
+ * strategy selector reads and {@link #lookup(String)} falls back to. Both
callers guard {@code null} before this
+ * runs, so a class is assumed here.
+ */
+ private static void rejectStrategy(final Class<?> clazz) {
+ if (TraversalStrategy.class.isAssignableFrom(clazz))
+ throw new IllegalArgumentException("TraversalStrategy classes are
registered with " +
+ "TraversalStrategies.GlobalCache, not ClassRegistry - " +
clazz.getName());
+ }
+}
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/ClassSerializer.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/ClassSerializer.java
index 7d2a516843..12b914cc1b 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/ClassSerializer.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/ClassSerializer.java
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.structure.io.binary.types;
+import org.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
import org.apache.tinkerpop.gremlin.structure.io.binary.DataType;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryReader;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter;
@@ -26,18 +27,15 @@ import org.apache.tinkerpop.gremlin.structure.io.Buffer;
import java.io.IOException;
public class ClassSerializer extends SimpleTypeSerializer<Class> {
+
public ClassSerializer() {
super(DataType.CLASS);
}
-
@Override
protected Class readValue(final Buffer buffer, final GraphBinaryReader
context) throws IOException {
final String name = context.readValue(buffer, String.class, false);
- try {
- return Class.forName(name);
- } catch (Exception ex) {
- throw new RuntimeException(ex);
- }
+ return ClassRegistry.lookup(name).
+ orElseThrow(() -> new IOException("Class not recognized - " +
name));
}
@Override
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/ClassJacksonDeserializer.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/ClassJacksonDeserializer.java
new file mode 100644
index 0000000000..d4583a47ea
--- /dev/null
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/ClassJacksonDeserializer.java
@@ -0,0 +1,54 @@
+/*
+ * 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.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
+import org.apache.tinkerpop.shaded.jackson.core.JsonParser;
+import org.apache.tinkerpop.shaded.jackson.core.JsonToken;
+import org.apache.tinkerpop.shaded.jackson.databind.DeserializationContext;
+import org.apache.tinkerpop.shaded.jackson.databind.deser.std.StdDeserializer;
+
+import java.io.IOException;
+
+/**
+ * Resolves the name a {@code Class} value carries through {@link
ClassRegistry} rather than through a class loader, so
+ * the named class is never loaded. Registering this keeps Jackson's own
{@code Class} deserializer from running, which
+ * would resolve the name with {@code Class.forName(name, true, loader)} and
initialize whatever it reached.
+ */
+final class ClassJacksonDeserializer extends StdDeserializer<Class> {
+
+ ClassJacksonDeserializer() {
+ super(Class.class);
+ }
+
+ @Override
+ public Class deserialize(final JsonParser jsonParser, final
DeserializationContext deserializationContext) throws IOException {
+ // the parser arrives positioned on the value, so the name is the
current token and not the next one.
+ // A value that is not a string names no class, so it becomes a null
name and is refused by the same lookup.
+ final String name = JsonToken.VALUE_STRING ==
jsonParser.currentToken() ? jsonParser.getText() : null;
+
+ return ClassRegistry.lookup(name).
+ orElseThrow(() -> new IOException("Class not recognized - " +
name));
+ }
+
+ @Override
+ public boolean isCachable() {
+ return true;
+ }
+}
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONModule.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONModule.java
index 37a79f5c1c..585ec65c2b 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONModule.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONModule.java
@@ -298,6 +298,9 @@ abstract class GraphSONModule extends
TinkerPopJacksonModule {
addDeserializer(Set.class, new
JavaUtilSerializersV3.SetJacksonDeserializer());
}
+ // java.lang - a Class value names a registered strategy rather
than anything a class loader can reach
+ addDeserializer(Class.class, new ClassJacksonDeserializer());
+
// numbers
addDeserializer(Integer.class, new
GraphSONSerializersV3.IntegerJackonsDeserializer());
addDeserializer(Double.class, new
GraphSONSerializersV3.DoubleJacksonDeserializer());
@@ -533,6 +536,9 @@ abstract class GraphSONModule extends
TinkerPopJacksonModule {
addDeserializer(TraversalMetrics.class, new
GraphSONSerializersV2.TraversalMetricsJacksonDeserializer());
addDeserializer(Tree.class, new
GraphSONSerializersV2.TreeJacksonDeserializer());
+ // java.lang - a Class value names a registered strategy rather
than anything a class loader can reach
+ addDeserializer(Class.class, new ClassJacksonDeserializer());
+
// numbers
addDeserializer(Integer.class, new
GraphSONSerializersV2.IntegerJacksonDeserializer());
addDeserializer(Double.class, new
GraphSONSerializersV2.DoubleJacksonDeserializer());
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java
index bd7bc086e2..87a4f25407 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.structure.io.gryo;
+import org.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
import org.apache.tinkerpop.gremlin.structure.io.gryo.kryoshim.InputShim;
import org.apache.tinkerpop.gremlin.structure.io.gryo.kryoshim.KryoShim;
import org.apache.tinkerpop.gremlin.structure.io.gryo.kryoshim.OutputShim;
@@ -96,11 +97,8 @@ final class UtilSerializers {
@Override
public <I extends InputShim> Class read(final KryoShim<I, ?> kryo,
final I input, final Class<Class> clazz) {
final String name = input.readString();
- try {
- return Class.forName(name);
- } catch (Exception ex) {
- throw new RuntimeException(ex);
- }
+ return ClassRegistry.lookup(name).
+ orElseThrow(() -> new RuntimeException("Class not
recognized - " + name));
}
}
@@ -118,11 +116,9 @@ final class UtilSerializers {
final int size = input.readInt();
final Class[] clazzes = new Class[size];
for (int i = 0; i < size; i++) {
- try {
- clazzes[i] = Class.forName(input.readString());
- } catch (Exception ex) {
- throw new RuntimeException(ex);
- }
+ final String name = input.readString();
+ clazzes[i] = ClassRegistry.lookup(name).
+ orElseThrow(() -> new RuntimeException("Class not
recognized - " + name));
}
return clazzes;
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
index 1311fef017..7fd3d0111f 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
@@ -76,7 +76,7 @@ public class TraversalStrategiesTest {
TraversalStrategies.GlobalCache.registerStrategies(TestGraphComputer.class,
TraversalStrategies.GlobalCache.getStrategies(GraphComputer.class).clone().addStrategies(new
StrategyC()));
}
-
+
@Test
public void shouldAllowUserManipulationOfGlobalCache() {
///////////
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/ClassRegistryTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/ClassRegistryTest.java
new file mode 100644
index 0000000000..0933c7f1fa
--- /dev/null
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/ClassRegistryTest.java
@@ -0,0 +1,246 @@
+/*
+ * 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;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReadOnlyStrategy;
+import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceFactory;
+import org.junit.After;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static
org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies.GlobalCache.getRegisteredStrategyClassByFullName;
+import static
org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies.GlobalCache.unregisterStrategy;
+import static org.apache.tinkerpop.gremlin.structure.io.ClassRegistry.lookup;
+import static org.apache.tinkerpop.gremlin.structure.io.ClassRegistry.register;
+import static
org.apache.tinkerpop.gremlin.structure.io.ClassRegistry.unregister;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
+
+/**
+ * Covers what {@link ClassRegistry} holds, what it refuses, and how a name
resolves against it. Also covers
+ * {@link ClassRegistry#lookup(String)} falling back into {@code
TraversalStrategies.GlobalCache}, which is how a
+ * strategy the selector can construct is nameable without being registered
here.
+ */
+public class ClassRegistryTest {
+
+ @After
+ public void unregisterProviderType() {
+ unregister(ProviderType.class);
+ }
+
+ /**
+ * The registry holds the classes that are not strategies, which is what
it is for, so a registered one resolves by
+ * its fully qualified name without being known to the strategy selector.
+ */
+ @Test
+ public void shouldResolveRegisteredClassAsNameable() {
+ register(ProviderType.class);
+
+ assertEquals(ProviderType.class,
lookup(ProviderType.class.getName()).get());
+ }
+
+ /**
+ * Gryo names the two algorithms out of {@code MatchAlgorithmStrategy}'s
{@code matchAlgorithmClass} field and the
+ * two factories out of {@code HaltedTraverserStrategy}'s {@code
haltedTraverserFactory} field, so the seeds are
+ * what keep those readable. Nothing else here covers the seeds, only Gryo
suites in another module.
+ */
+ @Test
+ public void shouldSeedTheClassesGryoNamesFromStrategyFields() {
+ for (final Class<?> clazz :
Arrays.asList(MatchStep.GreedyMatchAlgorithm.class,
+ MatchStep.CountMatchAlgorithm.class, DetachedFactory.class,
ReferenceFactory.class)) {
+ assertEquals("the seed for " + clazz.getName() + " is missing",
clazz,
+ lookup(clazz.getName()).orElse(null));
+ }
+ }
+
+ /**
+ * A {@link TraversalStrategy} is registered with {@code GlobalCache},
which is what makes it nameable, so this
+ * registry refuses one rather than holding a second place to register it.
Both a strategy the selector knows and
+ * one it does not are refused, since the class being a strategy is the
whole of the test.
+ */
+ @Test
+ public void shouldRefuseToRegisterAStrategy() {
+ for (final Class<? extends TraversalStrategy> clazz :
Arrays.asList(ReadOnlyStrategy.class,
+ AbsentStrategy.class)) {
+ try {
+ register(clazz);
+ fail("A TraversalStrategy must not be registered with
ClassRegistry - " + clazz.getName());
+ } catch (IllegalArgumentException ex) {
+ assertThat(ex.getMessage(), containsString(clazz.getName()));
+ assertThat(ex.getMessage(),
containsString("TraversalStrategies.GlobalCache"));
+ }
+ }
+ }
+
+ /**
+ * A caller unregistering a strategy here is making the same mistake as
one registering it, since
+ * {@code GlobalCache.unregisterStrategy} is what takes a strategy's
nameability away.
+ */
+ @Test
+ public void shouldRefuseToUnregisterAStrategy() {
+ try {
+ unregister(ReadOnlyStrategy.class);
+ fail("A TraversalStrategy must not be unregistered from
ClassRegistry");
+ } catch (IllegalArgumentException ex) {
+ assertThat(ex.getMessage(),
containsString(ReadOnlyStrategy.class.getName()));
+ assertThat(ex.getMessage(),
containsString("TraversalStrategies.GlobalCache"));
+ }
+
+ assertEquals(ReadOnlyStrategy.class,
lookup(ReadOnlyStrategy.class.getName()).get());
+ }
+
+ /**
+ * A {@code null} reaching registration is a mistake in a caller's Java
code rather than anything a serialized
+ * traversal can send, so it fails with a message naming the parameter
instead of the bare
+ * {@link NullPointerException} that {@code isAssignableFrom} throws,
which names nothing at all. Contrast
+ * {@link #shouldNotResolveNullAsNameable}, where a {@code null} arrives
on a read path and must not throw.
+ */
+ @Test
+ public void shouldRefuseToRegisterNull() {
+ try {
+ register(null);
+ fail("A null must not be registered with ClassRegistry");
+ } catch (NullPointerException ex) {
+ assertThat(ex.getMessage(), containsString("clazz"));
+ }
+ }
+
+ /**
+ * Unregistration guards a {@code null} the same way, since both entry
points reach the same field and a caller
+ * passing one has made the same mistake in either direction.
+ */
+ @Test
+ public void shouldRefuseToUnregisterNull() {
+ try {
+ unregister(null);
+ fail("A null must not be unregistered from ClassRegistry");
+ } catch (NullPointerException ex) {
+ assertThat(ex.getMessage(), containsString("clazz"));
+ }
+ }
+
+ /**
+ * The other direction of the containment: construction implies
nameability, so a strategy registered for the
+ * selector resolves through both accessors without being registered as
nameable.
+ */
+ @Test
+ public void shouldResolveConstructibleStrategyAsNameable() {
+ assertEquals(ReadOnlyStrategy.class,
+
getRegisteredStrategyClassByFullName(ReadOnlyStrategy.class.getName()).get());
+ assertEquals(ReadOnlyStrategy.class,
+ lookup(ReadOnlyStrategy.class.getName()).get());
+ }
+
+ @Test
+ public void shouldNotResolveUnregisteredStrategyAsNameable() {
+ unregisterStrategy(AbsentStrategy.class);
+ assertFalse(lookup(AbsentStrategy.class.getName()).isPresent());
+ }
+
+ /**
+ * A {@code null} must reach an empty {@code Optional} rather than a
{@link NullPointerException}, since an
+ * unchecked throw on a deserialization read path escapes the request
handlers and leaves the client with no
+ * response at all.
+ */
+ @Test
+ public void shouldNotResolveNullAsNameable() {
+ assertFalse(lookup(null).isPresent());
+ }
+
+ @Test
+ public void shouldNotResolveSimpleNameAsNameable() {
+ register(ProviderType.class);
+
+
assertFalse(lookup(ReadOnlyStrategy.class.getSimpleName()).isPresent());
+ assertFalse(lookup(ProviderType.class.getSimpleName()).isPresent());
+ }
+
+ /**
+ * {@link ClassRegistry#lookup(String)} matches the exact {@link
Class#getName()}, never assignability, so
+ * registering a superclass admits that class alone. {@code Object} is the
widest case there is: were the match on
+ * assignability, one registration would make every class on the classpath
nameable. Registered here rather than in
+ * {@code unregisterProviderType} because no other test wants it, so it is
undone in a {@code finally}.
+ */
+ @Test
+ public void shouldNotResolveSubclassOfRegisteredClassAsNameable() {
+ register(Object.class);
+ try {
+ assertEquals(Object.class,
lookup(Object.class.getName()).orElse(null));
+
+ assertFalse(lookup(ProviderType.class.getName()).isPresent());
+ assertFalse(lookup(String.class.getName()).isPresent());
+ } finally {
+ unregister(Object.class);
+ }
+ }
+
+ /**
+ * The same exact match applies to an interface, so registering one does
not admit the classes that implement it.
+ * {@code ProviderType} implements {@code ProviderInterface} for this
test, so the registration being no help to it
+ * is the point rather than an accident of the two being unrelated.
+ */
+ @Test
+ public void shouldNotResolveImplementorOfRegisteredInterfaceAsNameable() {
+ register(ProviderInterface.class);
+ try {
+ assertEquals(ProviderInterface.class,
lookup(ProviderInterface.class.getName()).orElse(null));
+
+ assertFalse(lookup(ProviderType.class.getName()).isPresent());
+ } finally {
+ unregister(ProviderInterface.class);
+ }
+ }
+
+ /**
+ * Stands in for an interface a provider registers as nameable. {@code
ProviderType} implements it so that
+ * registering the interface can be shown not to admit the implementor.
+ */
+ private interface ProviderInterface {
+ }
+
+ /**
+ * Stands in for a class a provider registers as nameable, which is
deliberately not a {@link TraversalStrategy}
+ * because the registry refuses one. It is unregistered again in {@code
unregisterProviderType}.
+ */
+ private static final class ProviderType implements ProviderInterface {
+ }
+
+ /**
+ * A {@link TraversalStrategy} that the selector does not know, so
refusing to register it tests the class being a
+ * strategy rather than the selector already holding it.
+ */
+ private static final class AbsentStrategy
+ extends
AbstractTraversalStrategy<TraversalStrategy.DecorationStrategy>
+ implements TraversalStrategy.DecorationStrategy {
+
+ @Override
+ public void apply(final Traversal.Admin<?, ?> traversal) {
+ // do nothing
+ }
+ }
+}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/ClassJacksonDeserializerTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/ClassJacksonDeserializerTest.java
new file mode 100644
index 0000000000..50fc9c4bd4
--- /dev/null
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/ClassJacksonDeserializerTest.java
@@ -0,0 +1,219 @@
+/*
+ * 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.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy;
+import org.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
+import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
+import
org.apache.tinkerpop.shaded.jackson.databind.exc.MismatchedInputException;
+import org.junit.After;
+import org.junit.Test;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Each test reads through both the V2 and V3 mappers, which share one
deserializer. The file is not parameterized by
+ * version because the canary can report only one load, so a second run would
see a flag the first had already set.
+ */
+public class ClassJacksonDeserializerTest extends AbstractGraphSONTest {
+
+ private final ObjectMapper mapperV2 =
GraphSONMapper.build().version(GraphSONVersion.V2_0).
+ typeInfo(TypeInfo.PARTIAL_TYPES).create().createMapper();
+
+ private final ObjectMapper mapperV3 =
GraphSONMapper.build().version(GraphSONVersion.V3_0).
+ typeInfo(TypeInfo.PARTIAL_TYPES).create().createMapper();
+
+ @After
+ public void unregisterProviderType() {
+ ClassRegistry.unregister(ProviderType.class);
+ }
+
+ /**
+ * {@code SubgraphStrategy} is registered with {@code
TraversalStrategies.GlobalCache}, which the registry falls
+ * back to, so nothing has to register it here.
+ */
+ @Test
+ public void shouldReadClassOfRegisteredStrategy() throws Exception {
+ assertEquals(SubgraphStrategy.class, serializeDeserialize(mapperV2,
SubgraphStrategy.class, Class.class));
+ assertEquals(SubgraphStrategy.class, serializeDeserialize(mapperV3,
SubgraphStrategy.class, Class.class));
+ }
+
+ /**
+ * A provider makes a class of its own nameable by calling {@link
ClassRegistry#register(Class)}, so that call is
+ * pinned here through the deserializer rather than only through the
registry's fall-back to {@code GlobalCache}.
+ * {@code ProviderType} is registered rather than refused, so do not write
a refusal test against it.
+ */
+ @Test
+ public void shouldReadClassRegisteredThroughClassRegistry() throws
Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ assertEquals(ProviderType.class, serializeDeserialize(mapperV2,
ProviderType.class, Class.class));
+ assertEquals(ProviderType.class, serializeDeserialize(mapperV3,
ProviderType.class, Class.class));
+ }
+
+ @Test
+ public void shouldRejectClassThatIsNotRegistered() throws Exception {
+ final String fqcn = File.class.getName();
+
+ assertThat(refusalOf(mapperV2, fqcn).getMessage(),
containsString(fqcn));
+ assertThat(refusalOf(mapperV3, fqcn).getMessage(),
containsString(fqcn));
+ }
+
+ /**
+ * The counterpart to the refusal below, without which a blanket refusal
of every list holding a {@code Class} would
+ * pass that test. Both names resolve, one through {@code GlobalCache} and
one through {@link ClassRegistry}, so both
+ * resolution paths are covered inside a container, and both elements are
pinned by identity and in order so a
+ * partly read or empty list cannot pass.
+ */
+ @Test
+ public void shouldReadListElementsThatAreRegistered() throws Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ final String elements = classValue(SubgraphStrategy.class.getName()) +
"," +
+ classValue(ProviderType.class.getName());
+
+ // V2 writes a list as a bare JSON array while V3 wraps it in g:List,
as in the refusal below
+ final List<?> fromV2 = readList(mapperV2, "[" + elements + "]");
+ final List<?> fromV3 = readList(mapperV3,
"{\"@type\":\"g:List\",\"@value\":[" + elements + "]}");
+
+ for (final List<?> read : Arrays.asList(fromV2, fromV3)) {
+ assertEquals(2, read.size());
+ assertSame(SubgraphStrategy.class, read.get(0));
+ assertSame(ProviderType.class, read.get(1));
+ }
+ }
+
+ /**
+ * A {@code Class} held inside a container reaches the same deserializer
as one read on its own, because a list
+ * reads each element as an {@code Object} and so resolves the type the
element itself carries. The unregistered
+ * name is the second element, so the refusal covers every element rather
than only the first. V2 writes a list as a
+ * bare JSON array while V3 wraps it in {@code g:List}, so the two reads
are not handed the same document.
+ */
+ @Test
+ public void shouldRejectListElementThatIsNotRegistered() throws Exception {
+ final String refusal = "Class not recognized - " +
File.class.getName();
+ final String elements = classValue(SubgraphStrategy.class.getName()) +
"," +
+ classValue(File.class.getName());
+
+ assertThat(refusalOfJson(mapperV2, "[" + elements + "]").getMessage(),
containsString(refusal));
+ assertThat(refusalOfJson(mapperV3,
"{\"@type\":\"g:List\",\"@value\":[" + elements + "]}").getMessage(),
+ containsString(refusal));
+ }
+
+ @Test
+ public void shouldNotLoadClassNamedByUnregisteredStrategy() throws
Exception {
+ assertFalse("the canary was already initialised before the read, so
this fixture no longer proves anything",
+ CanaryFlag.initialised);
+
+ final String fqcn = ClassJacksonDeserializerTest.class.getName() +
"$CanaryStrategy";
+ assertThat(refusalOf(mapperV2, fqcn).getMessage(),
containsString(fqcn));
+ assertThat(refusalOf(mapperV3, fqcn).getMessage(),
containsString(fqcn));
+
+ assertFalse("reading the name of an unregistered strategy initialised
the class it named",
+ CanaryFlag.initialised);
+
+ Class.forName(fqcn);
+ assertTrue("the canary cannot report a load, so the assertions above
cannot fail", CanaryFlag.initialised);
+ }
+
+ /**
+ * Reads a {@code Class} value naming {@code fqcn} into an {@code Object},
as a bytecode argument arrives, and
+ * returns the refusal. The deserializer's {@code IOException} surfaces as
a {@link MismatchedInputException}.
+ */
+ private MismatchedInputException refusalOf(final ObjectMapper mapper,
final String fqcn) throws Exception {
+ return refusalOfJson(mapper, classValue(fqcn));
+ }
+
+ /**
+ * Reads {@code json} into an {@code Object} and hands back the refusal it
produced, failing the calling test if it
+ * deserialized instead.
+ */
+ private MismatchedInputException refusalOfJson(final ObjectMapper mapper,
final String json) throws Exception {
+ try {
+ mapper.readValue(json, Object.class);
+ } catch (MismatchedInputException ex) {
+ return ex;
+ }
+
+ fail("A class that is not a registered strategy must not deserialize -
" + json);
+ return null;
+ }
+
+ /**
+ * Reads {@code json} into an {@code Object}, as a bytecode argument
arrives, and hands back the list it produced.
+ */
+ private List<?> readList(final ObjectMapper mapper, final String json)
throws Exception {
+ return (List<?>) mapper.readValue(json, Object.class);
+ }
+
+ /**
+ * The {@code Class} value a client sends, as it appears both on its own
and as an element of a list.
+ */
+ private static String classValue(final String fqcn) {
+ return "{\"@type\":\"g:Class\",\"@value\":\"" + fqcn + "\"}";
+ }
+
+ /**
+ * The flag lives here rather than on {@code CanaryStrategy} because
reading a static field initialises the class
+ * declaring it, so a flag on {@code CanaryStrategy} could not be read
without loading what it reports on.
+ */
+ static final class CanaryFlag {
+ static boolean initialised;
+ }
+
+ /**
+ * A {@link TraversalStrategy} that is never registered, so refusing it
tests registry membership rather than the
+ * class not being a strategy. Never name it as a class literal and never
register it.
+ */
+ static final class CanaryStrategy
+ extends
AbstractTraversalStrategy<TraversalStrategy.DecorationStrategy>
+ implements TraversalStrategy.DecorationStrategy {
+
+ static {
+ CanaryFlag.initialised = true;
+ }
+
+ @Override
+ public void apply(final Traversal.Admin<?, ?> traversal) {
+ // do nothing
+ }
+ }
+
+ /**
+ * Stands in for a class a provider ships, registered by each test that
reads it and unregistered again in
+ * {@code unregisterProviderType}. It is deliberately not a {@link
TraversalStrategy}, which
+ * the registry refuses, and it is registered rather than refused, so it
is not a refusal fixture:
+ * {@code CanaryStrategy} is the class that is never registered.
+ */
+ private static final class ProviderType {
+ }
+}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/ClassSerializerTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/ClassSerializerTest.java
new file mode 100644
index 0000000000..98f1acf806
--- /dev/null
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/ClassSerializerTest.java
@@ -0,0 +1,250 @@
+/*
+ * 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.gryo;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy;
+import org.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
+import
org.apache.tinkerpop.gremlin.structure.io.gryo.kryoshim.shaded.ShadedInputAdapter;
+import org.apache.tinkerpop.shaded.kryo.Kryo;
+import org.apache.tinkerpop.shaded.kryo.io.Input;
+import org.apache.tinkerpop.shaded.kryo.io.Output;
+import org.junit.After;
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Covers both serializers that read a class name, {@code ClassSerializer} for
a single value and
+ * {@code ClassArraySerializer} for the {@code withoutStrategies()} argument.
Each test reads through both Gryo
+ * versions, which share them. The file is not parameterized by version
because the canary can report only one load, so
+ * a second run would see a flag the first had already set.
+ */
+public class ClassSerializerTest {
+
+ private static final GryoVersion[] VERSIONS = new
GryoVersion[]{GryoVersion.V1_0, GryoVersion.V3_0};
+
+ @After
+ public void unregisterProviderType() {
+ ClassRegistry.unregister(ProviderType.class);
+ }
+
+ /**
+ * A provider makes a class of its own nameable by calling {@link
ClassRegistry#register(Class)}, so that call is
+ * pinned here through the serializer rather than only through the
registry's fall-back to {@code GlobalCache}.
+ * {@code ProviderType} is registered rather than refused, so do not write
a refusal test against it.
+ */
+ @Test
+ public void shouldReadClassRegisteredThroughClassRegistry() throws
Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ for (final GryoVersion version : VERSIONS) {
+ assertEquals(ProviderType.class, roundTrip(version,
ProviderType.class, Class.class));
+ }
+ }
+
+ /**
+ * {@code SubgraphStrategy} is registered with {@code
TraversalStrategies.GlobalCache}, which the registry falls
+ * back to, so only {@code ProviderType} has to be registered here. Two
elements are written so that the read
+ * covers the loop rather than one name.
+ */
+ @Test
+ public void shouldReadClassArrayRegisteredThroughClassRegistry() throws
Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ final Class[] classes = new Class[]{SubgraphStrategy.class,
ProviderType.class};
+ for (final GryoVersion version : VERSIONS) {
+ assertArrayEquals(classes, roundTrip(version, classes,
Class[].class));
+ }
+ }
+
+ @Test
+ public void shouldRejectClassThatIsNotRegistered() throws Exception {
+ for (final GryoVersion version : VERSIONS) {
+ assertEquals("Class not recognized - " + File.class.getName(),
+ refusalOf(version, File.class, Class.class).getMessage());
+ }
+ }
+
+ /**
+ * The unregistered name is the second element, so the refusal covers
every element rather than only the first.
+ * Refusing part way through leaves the stream partly consumed, which
costs nothing because the whole read fails.
+ */
+ @Test
+ public void shouldRejectClassArrayThatIsNotRegistered() throws Exception {
+ final Class[] classes = new Class[]{SubgraphStrategy.class,
File.class};
+ for (final GryoVersion version : VERSIONS) {
+ assertEquals("Class not recognized - " + File.class.getName(),
+ refusalOf(version, classes, Class[].class).getMessage());
+ }
+ }
+
+ /**
+ * The counterpart to the refusal below, without which a blanket refusal
of every list holding a {@code Class} would
+ * pass that test. Both names resolve, one through {@code GlobalCache} and
one through {@link ClassRegistry}, so both
+ * resolution paths are covered inside a container, and both elements are
pinned by identity and in order so a
+ * partly read or empty list cannot pass.
+ */
+ @Test
+ public void shouldReadListElementsThatAreRegistered() throws Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ final ArrayList<Class> classes = new
ArrayList<>(Arrays.asList(SubgraphStrategy.class, ProviderType.class));
+ for (final GryoVersion version : VERSIONS) {
+ final ArrayList<?> read = roundTrip(version, classes,
ArrayList.class);
+
+ assertEquals(2, read.size());
+ assertSame(SubgraphStrategy.class, read.get(0));
+ assertSame(ProviderType.class, read.get(1));
+ }
+ }
+
+ /**
+ * A {@code Class} held inside a container reaches the same serializer as
one read on its own, because Kryo reads
+ * each element of a collection that carries no element type with the
serializer registered for the class the
+ * element itself names. The unregistered name is the second element, so
the refusal covers every element rather
+ * than only the first.
+ */
+ @Test
+ public void shouldRejectListElementThatIsNotRegistered() throws Exception {
+ final ArrayList<Class> classes = new
ArrayList<>(Arrays.asList(SubgraphStrategy.class, File.class));
+ for (final GryoVersion version : VERSIONS) {
+ assertEquals("Class not recognized - " + File.class.getName(),
+ refusalOf(version, classes, ArrayList.class).getMessage());
+ }
+ }
+
+ /**
+ * The refusal test above would pass against an implementation that
resolved the name through a class loader before
+ * refusing, so this is the only test that distinguishes a refusal from
never having loaded the class. It detects a
+ * return to the initialising {@code Class.forName(name)}. The
three-argument form loads without initialising and
+ * would not be detected.
+ */
+ @Test
+ public void shouldNotLoadClassNamedByUnregisteredStrategy() throws
Exception {
+ assertFalse("the canary was already initialised before the read, so
this fixture no longer proves anything",
+ CanaryFlag.initialised);
+
+ final String fqcn = ClassSerializerTest.class.getName() +
"$CanaryStrategy";
+ try {
+ readName(fqcn);
+ fail("A class that is not a registered strategy must not
deserialize - " + fqcn);
+ } catch (RuntimeException ex) {
+ assertEquals("Class not recognized - " + fqcn, ex.getMessage());
+ }
+
+ assertFalse("reading the name of an unregistered strategy initialised
the class it named",
+ CanaryFlag.initialised);
+
+ Class.forName(fqcn);
+ assertTrue("the canary cannot report a load, so the assertions above
cannot fail", CanaryFlag.initialised);
+ }
+
+ private <T> T roundTrip(final GryoVersion version, final Object value,
final Class<T> clazz) throws Exception {
+ final Kryo kryo =
GryoMapper.build().version(version).create().createMapper();
+ try (final ByteArrayOutputStream stream = new ByteArrayOutputStream())
{
+ final Output output = new Output(stream);
+ kryo.writeObject(output, value);
+ output.flush();
+
+ try (final InputStream inputStream = new
ByteArrayInputStream(stream.toByteArray())) {
+ return kryo.readObject(new Input(inputStream), clazz);
+ }
+ }
+ }
+
+ /**
+ * Round trips {@code value} and hands back the refusal the read produced,
failing the calling test if it
+ * deserialized instead.
+ */
+ private RuntimeException refusalOf(final GryoVersion version, final Object
value,
+ final Class<?> clazz) throws Exception {
+ try {
+ roundTrip(version, value, clazz);
+ } catch (RuntimeException ex) {
+ return ex;
+ }
+
+ fail("A class that is not a registered strategy must not deserialize -
" + value);
+ return null;
+ }
+
+ /**
+ * Hands a name to {@code ClassSerializer} written the way its own {@code
write} writes one. The writer takes a
+ * loaded {@code Class}, so a name that must stay unloaded cannot be
produced by round tripping a class literal.
+ */
+ private Class readName(final String fqcn) {
+ final Output output = new Output(64, -1);
+ output.writeString(fqcn);
+ output.flush();
+
+ return new UtilSerializers.ClassSerializer().read(null,
+ new ShadedInputAdapter(new Input(output.toBytes())),
Class.class);
+ }
+
+ /**
+ * Records that the static initialiser of {@code CanaryStrategy} ran. It
is held here, on a class of its own,
+ * rather than on {@code CanaryStrategy} itself, because reading a static
field initialises the class that declares
+ * it: a flag on {@code CanaryStrategy} could not be read without loading
the very class it reports on.
+ */
+ static final class CanaryFlag {
+ static boolean initialised;
+ }
+
+ /**
+ * A {@link TraversalStrategy} on the test classpath, never registered, so
refusing it tests registry membership
+ * rather than the class not being a strategy. Never name it as a class
literal and never register it.
+ */
+ static final class CanaryStrategy
+ extends
AbstractTraversalStrategy<TraversalStrategy.DecorationStrategy>
+ implements TraversalStrategy.DecorationStrategy {
+
+ static {
+ CanaryFlag.initialised = true;
+ }
+
+ @Override
+ public void apply(final Traversal.Admin<?, ?> traversal) {
+ // do nothing
+ }
+ }
+
+ /**
+ * Stands in for a class a provider ships, registered by each test that
reads it and unregistered again in
+ * {@code unregisterProviderType}. It is deliberately not a {@link
TraversalStrategy}, which the registry refuses,
+ * and it is registered rather than refused, so it is not a refusal
fixture: {@code CanaryStrategy} is the class
+ * that is never registered.
+ */
+ private static final class ProviderType {
+ }
+}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoMapperTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoMapperTest.java
index bd997f29c8..aabe7d34b0 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoMapperTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoMapperTest.java
@@ -23,6 +23,7 @@ import
org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
import org.apache.tinkerpop.gremlin.process.traversal.Merge;
import org.apache.tinkerpop.gremlin.process.traversal.TextP;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy;
import
org.apache.tinkerpop.gremlin.process.traversal.util.TraversalExplanation;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.io.Io;
@@ -757,7 +758,7 @@ public class GryoMapperTest {
@Test
public void shouldHandleClass() throws Exception {
- final Class<?> clazz = java.io.File.class;
+ final Class<?> clazz = SubgraphStrategy.class;
assertEquals(clazz, serializeDeserialize(clazz, Class.class));
}
diff --git a/gremlin-go/driver/connection_test.go
b/gremlin-go/driver/connection_test.go
index a04cddcbfc..5412515bb6 100644
--- a/gremlin-go/driver/connection_test.go
+++ b/gremlin-go/driver/connection_test.go
@@ -874,7 +874,7 @@ func TestConnection(t *testing.T) {
g := initializeGraph(t, testNoAuthUrl, testNoAuthAuthInfo,
testNoAuthTlsConfig)
defer g.remoteConnection.Close()
- prop := &GremlinType{"java.lang.Object"}
+ prop :=
&GremlinType{"org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy"}
i := g.AddV("type_test").Property("data", prop).Iterate()
err := <-i
assert.Nil(t, err)
diff --git
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/Model.java
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/Model.java
index d4788bd5aa..b18d953051 100644
---
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/Model.java
+++
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/Model.java
@@ -31,6 +31,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Scope;
import org.apache.tinkerpop.gremlin.process.traversal.TextP;
import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy;
import
org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalMetrics;
import org.apache.tinkerpop.gremlin.process.traversal.util.MutableMetrics;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics;
@@ -44,7 +45,6 @@ import
org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
import org.apache.tinkerpop.gremlin.util.message.RequestMessage;
import org.apache.tinkerpop.gremlin.util.message.ResponseMessage;
-import java.io.File;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.net.InetAddress;
@@ -98,7 +98,7 @@ public class Model {
// IMPORTANT - the "title" or name of the Entry needs to be unique
- addCoreEntry(File.class, "Class", "");
+ addCoreEntry(SubgraphStrategy.class, "Class", "");
addCoreEntry(new Date(1481750076295L), "Date");
addCoreEntry(100.00d, "Double");
addCoreEntry(100.00f, "Float", "");
diff --git
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphbinary/GraphBinaryCompatibilityTest.java
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphbinary/GraphBinaryCompatibilityTest.java
index b12b3f4b5f..9c73f9c9c3 100644
---
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphbinary/GraphBinaryCompatibilityTest.java
+++
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphbinary/GraphBinaryCompatibilityTest.java
@@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.structure.io.Buffer;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryReader;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter;
import org.apache.tinkerpop.gremlin.util.ser.NettyBufferFactory;
+import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -35,6 +36,10 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.util.Arrays;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.fail;
+
/**
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
@@ -112,4 +117,21 @@ public class GraphBinaryCompatibilityTest extends
AbstractTypedCompatibilityTest
public void shouldReadWriteAuthenticationChallenge() throws Exception {
super.shouldReadWriteAuthenticationChallenge();
}
+
+ /**
+ * The {@code class-unregistered} fixture is the {@code class} fixture,
when a {@code Class} value naming any
+ * class on the class path decoded. It names {@code java.io.File}, which
is not a registered
+ * {@link
org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy}, so it no
longer decodes.
+ * It is kept to record exactly which values stopped being readable.
+ */
+ @Test
+ public void shouldNotReadClassThatIsNotRegistered() throws Exception {
+ final byte[] bytes = readFromResource("class-unregistered");
+ try {
+ read(bytes, Class.class);
+ fail("A Class value naming a class that is not a registered
strategy must not deserialize");
+ } catch (IOException ex) {
+ assertThat(ex.getMessage(), containsString("Class not recognized -
java.io.File"));
+ }
+ }
}
diff --git
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java
index 4ba4cede70..259dacf0ff 100644
---
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java
+++
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java
@@ -203,7 +203,7 @@ public abstract class AbstractRoundTripTest {
new Object[] {"Bytecode", bytecode, null},
new Object[] {"Binding", new Bytecode.Binding<>("x", 123),
null},
new Object[] {"Traverser", new
DefaultRemoteTraverser<>("marko", 100), null},
- new Object[] {"Class", Bytecode.class, null},
+ new Object[] {"Class", SubgraphStrategy.class, null},
new Object[] {"ByteBuffer", ByteBuffer.wrap(new byte[]{ 1, 2,
3 }), null},
new Object[] {"InetAddressV4",
InetAddress.getByName("127.0.0.1"), null},
new Object[] {"InetAddressV6", InetAddress.getByName("::1"),
null},
diff --git
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/ClassSerializerTest.java
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/ClassSerializerTest.java
new file mode 100644
index 0000000000..169899ea6f
--- /dev/null
+++
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/ClassSerializerTest.java
@@ -0,0 +1,361 @@
+/*
+ * 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.util.ser.binary.types;
+
+import io.netty.buffer.ByteBufAllocator;
+import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy;
+import org.apache.tinkerpop.gremlin.structure.io.Buffer;
+import org.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
+import org.apache.tinkerpop.gremlin.structure.io.binary.DataType;
+import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryReader;
+import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter;
+import org.apache.tinkerpop.gremlin.structure.io.binary.TypeSerializerRegistry;
+import
org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.apache.tinkerpop.gremlin.util.ser.NettyBufferFactory;
+import org.junit.After;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * A {@code Class} value carries a class name off the wire, and that name is
resolved through {@link ClassRegistry}
+ * rather than through a class loader, so only a class that registry holds, or
a {@link TraversalStrategy} registered
+ * with {@code TraversalStrategies.GlobalCache}, can be named. The write path
is deliberately left ungated because the
+ * writer is handed a class the server already holds.
+ */
+public class ClassSerializerTest {
+
+ private static final NettyBufferFactory bufferFactory = new
NettyBufferFactory();
+
+ private final ByteBufAllocator allocator = ByteBufAllocator.DEFAULT;
+
+ @After
+ public void unregisterFixtures() {
+
TraversalStrategies.GlobalCache.unregisterStrategy(NestedStrategy.class);
+ ClassRegistry.unregister(ProviderType.class);
+ }
+
+ @Test
+ public void shouldReadClassRegisteredAsABuiltIn() throws Exception {
+ assertEquals(SubgraphStrategy.class,
readClass(SubgraphStrategy.class.getName()));
+ }
+
+ /**
+ * The generic {@code Class} value exists for {@code withoutStrategies()},
so the decode is also pinned against the
+ * bytecode a client produces rather than only against a name written on
its own.
+ */
+ @Test
+ public void shouldDecodeRegisteredStrategyFromWithoutStrategiesBytecode()
throws Exception {
+ final Bytecode bytecode = EmptyGraph.instance().traversal().
+
withoutStrategies(SubgraphStrategy.class).V().asAdmin().getBytecode();
+
+ final Buffer buffer = bufferFactory.create(allocator.buffer());
+ new GraphBinaryWriter().write(bytecode, buffer);
+
+ assertEquals(bytecode, new GraphBinaryReader().read(buffer));
+ }
+
+ @Test
+ public void shouldRejectClassThatIsNotRegistered() throws Exception {
+ try {
+ readClass(File.class.getName());
+ fail("A class that is not a registered strategy must not
deserialize");
+ } catch (IOException ex) {
+ assertThat(ex.getMessage(), containsString("Class not recognized -
java.io.File"));
+ }
+ }
+
+ /**
+ * A refusal must not reveal whether the named class is on the classpath.
Present-but-unregistered and absent
+ * entirely get the same checked {@link IOException}, same message apart
from the name, no cause - that
+ * indistinguishability is what closing the oracle means. It holds by
construction: the registry lookup never
+ * consults a class loader. Pinned because a change that resolved the name
before refusing would still pass every
+ * other refusal test here.
+ */
+ @Test
+ public void shouldRefusePresentAndAbsentClassesIndistinguishably() throws
Exception {
+ final String present = File.class.getName();
+ final String absent =
"org.apache.tinkerpop.gremlin.util.ser.binary.types.NotOnTheClasspath";
+
+ // the test only means something while one name resolves and the other
does not, so both are pinned first
+ Class.forName(present);
+ try {
+ Class.forName(absent);
+ fail("the absent name must not be on the classpath for this test
to mean anything");
+ } catch (ClassNotFoundException expected) {
+ // the fixture holds
+ }
+
+ final IOException presentRefusal = refusalOf(present);
+ final IOException absentRefusal = refusalOf(absent);
+
+ assertEquals(presentRefusal.getClass(), absentRefusal.getClass());
+ assertEquals("Class not recognized - " + present,
presentRefusal.getMessage());
+ assertEquals("Class not recognized - " + absent,
absentRefusal.getMessage());
+
+ // the messages are the same once the name each one echoes is taken
out of it
+ assertEquals(presentRefusal.getMessage().replace(present, "?"),
+ absentRefusal.getMessage().replace(absent, "?"));
+
+ // nor does a cause answer what the message does not, such as a
ClassNotFoundException on one side only
+ assertNull(presentRefusal.getCause());
+ assertNull(absentRefusal.getCause());
+ }
+
+ @Test
+ public void shouldRejectEmptyName() throws Exception {
+ try {
+ readClass("");
+ fail("An empty class name must not deserialize");
+ } catch (IOException ex) {
+ assertEquals("Class not recognized - ", ex.getMessage());
+ }
+ }
+
+ /**
+ * A {@code String} value is not nullable at this position, so a null name
cannot arrive from a well-formed
+ * message. It is still worth pinning that a null reaches the same refusal
rather than a {@code NullPointerException}
+ * that would escape the request handlers and leave a client with no
response at all.
+ */
+ @Test
+ public void shouldRejectNullName() throws Exception {
+ final GraphBinaryReader reader = new
GraphBinaryReader(TypeSerializerRegistry.build().
+ add(String.class, new NullStringSerializer()).create());
+ try {
+ reader.readValue(bufferFactory.create(allocator.buffer()),
Class.class, false);
+ fail("A null class name must not deserialize");
+ } catch (IOException ex) {
+ assertEquals("Class not recognized - null", ex.getMessage());
+ }
+ }
+
+ @Test
+ public void shouldRejectArrayDescriptor() throws Exception {
+ try {
+ readClass("[Ljava.lang.String;");
+ fail("An array descriptor must not deserialize");
+ } catch (IOException ex) {
+ assertEquals("Class not recognized - [Ljava.lang.String;",
ex.getMessage());
+ }
+ }
+
+ @Test
+ public void shouldReadRegisteredNestedClass() throws Exception {
+ TraversalStrategies.GlobalCache.registerStrategy(NestedStrategy.class);
+
+ assertEquals(NestedStrategy.class,
readClass(NestedStrategy.class.getName()));
+ }
+
+ /**
+ * A provider makes a class of its own nameable by calling {@link
ClassRegistry#register(Class)}, so that call is
+ * pinned here through the deserializer rather than only through the
registry's fall-back to {@code GlobalCache}.
+ * {@code ProviderType} is registered rather than refused, so do not write
a refusal test against it.
+ */
+ @Test
+ public void shouldReadClassRegisteredThroughClassRegistry() throws
Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ assertEquals(ProviderType.class,
readClass(ProviderType.class.getName()));
+ }
+
+ /**
+ * Every other refusal test here would pass against an implementation that
resolved the name through a class loader
+ * before refusing, so this is the only test that distinguishes a refusal
from never having loaded the class. It
+ * detects a return to the initialising {@code Class.forName(name)}; the
three-argument form loads without
+ * initialising and would not be detected.
+ */
+ @Test
+ public void shouldNotLoadClassNamedByUnregisteredStrategy() throws
Exception {
+ assertFalse("the canary was already initialised before the read, so
this fixture no longer proves anything",
+ CanaryFlag.initialised);
+
+ final String fqcn = ClassSerializerTest.class.getName() +
"$CanaryStrategy";
+ final IOException refusal = refusalOf(fqcn);
+ assertEquals("Class not recognized - " + fqcn, refusal.getMessage());
+
+ assertFalse("reading the name of an unregistered strategy initialised
the class it named",
+ CanaryFlag.initialised);
+
+ Class.forName(fqcn);
+ assertTrue("the canary cannot report a load, so the assertions above
cannot fail", CanaryFlag.initialised);
+ }
+
+ /**
+ * The counterpart to the refusal below, without which a blanket refusal
of every list holding a {@code Class} would
+ * pass that test. Both names resolve, one through {@code GlobalCache} and
one through {@link ClassRegistry}, so both
+ * resolution paths are covered inside a container, and both elements are
pinned by identity and in order so a
+ * partly read or empty list cannot pass.
+ */
+ @Test
+ public void shouldReadListElementsThatAreRegistered() throws Exception {
+ ClassRegistry.register(ProviderType.class);
+
+ final Buffer buffer = bufferFactory.create(allocator.buffer());
+ new GraphBinaryWriter().write(Arrays.asList(SubgraphStrategy.class,
ProviderType.class), buffer);
+
+ final List<?> read = (List<?>) new GraphBinaryReader().read(buffer);
+
+ assertEquals(2, read.size());
+ assertSame(SubgraphStrategy.class, read.get(0));
+ assertSame(ProviderType.class, read.get(1));
+ }
+
+ /**
+ * A {@code Class} held inside a container is read by the same serializer
as one read on its own, because a
+ * collection reads each element as a fully qualified value and so
dispatches on the element's own type code. The
+ * unregistered name is the second element, so the refusal covers every
element rather than only the first.
+ */
+ @Test
+ public void shouldRejectListElementThatIsNotRegistered() throws Exception {
+ final Buffer buffer = bufferFactory.create(allocator.buffer());
+ new GraphBinaryWriter().write(Arrays.asList(SubgraphStrategy.class,
File.class), buffer);
+
+ try {
+ new GraphBinaryReader().read(buffer);
+ fail("A list holding a class that is not a registered strategy
must not deserialize");
+ } catch (IOException ex) {
+ assertThat(ex.getMessage(), containsString("Class not recognized -
" + File.class.getName()));
+ }
+ }
+
+ /**
+ * The writer is handed a class the server itself holds rather than a name
off the wire, so it is not gated. The
+ * consequence, recorded here, is that a server can write a {@code Class}
value that it would refuse to read.
+ */
+ @Test
+ public void shouldWriteClassThatIsNotRegistered() throws Exception {
+ final Buffer buffer = bufferFactory.create(allocator.buffer());
+ new GraphBinaryWriter().writeValue(File.class, buffer, false);
+
+ assertEquals(File.class.getName(), new
GraphBinaryReader().readValue(buffer, String.class, false));
+ }
+
+ /**
+ * The value of a {@code Class} is the class name written as a
non-nullable {@code String} value, so writing the
+ * name that way produces exactly what a client sends for a {@code Class}.
+ */
+ private Class readClass(final String fqcn) throws IOException {
+ final Buffer buffer = bufferFactory.create(allocator.buffer());
+ new GraphBinaryWriter().writeValue(fqcn, buffer, false);
+
+ return new GraphBinaryReader().readValue(buffer, Class.class, false);
+ }
+
+ /**
+ * Reads a {@code Class} value naming {@code fqcn} and hands back the
refusal it produced, failing the calling test
+ * if the name deserialized instead.
+ */
+ private IOException refusalOf(final String fqcn) throws IOException {
+ try {
+ readClass(fqcn);
+ } catch (IOException ex) {
+ return ex;
+ }
+
+ fail("A class that is not a registered strategy must not deserialize -
" + fqcn);
+ return null;
+ }
+
+ private static final class NestedStrategy
+ extends
AbstractTraversalStrategy<TraversalStrategy.DecorationStrategy>
+ implements TraversalStrategy.DecorationStrategy {
+
+ @Override
+ public void apply(final Traversal.Admin<?, ?> traversal) {
+ // do nothing
+ }
+ }
+
+ /**
+ * Stands in for a class a provider ships, registered by each test that
reads it and unregistered again in
+ * {@code unregisterFixtures}. It is deliberately not a {@link
TraversalStrategy}, which the
+ * registry refuses, and it is registered rather than refused, so it is
not a refusal fixture:
+ * {@code CanaryStrategy} is the class that is never registered.
+ */
+ private static final class ProviderType {
+ }
+
+ /**
+ * Records that the static initialiser of {@code CanaryStrategy} ran. It
is held here, on a class of its own,
+ * rather than on {@code CanaryStrategy} itself, because reading a static
field initialises the class that declares
+ * it: a flag on {@code CanaryStrategy} could not be read without loading
the very class it reports on.
+ */
+ static final class CanaryFlag {
+ static boolean initialised;
+ }
+
+ /**
+ * A {@link TraversalStrategy} on the test classpath, never registered, so
refusing it tests registry membership
+ * rather than the class not being a strategy. Never name it as a class
literal and never register it -
+ * {@code shouldNotLoadClassNamedByUnregisteredStrategy} names it as a
string and watches its static initialiser.
+ */
+ static final class CanaryStrategy
+ extends
AbstractTraversalStrategy<TraversalStrategy.DecorationStrategy>
+ implements TraversalStrategy.DecorationStrategy {
+
+ static {
+ CanaryFlag.initialised = true;
+ }
+
+ @Override
+ public void apply(final Traversal.Admin<?, ?> traversal) {
+ // do nothing
+ }
+ }
+
+ /**
+ * Stands in for the {@code String} serializer to show that a null name is
refused the same way, which a
+ * well-formed message cannot produce at a non-nullable position.
+ */
+ private static class NullStringSerializer extends
SimpleTypeSerializer<String> {
+
+ NullStringSerializer() {
+ super(DataType.STRING);
+ }
+
+ @Override
+ protected String readValue(final Buffer buffer, final
GraphBinaryReader context) throws IOException {
+ return null;
+ }
+
+ @Override
+ protected void writeValue(final String value, final Buffer buffer,
+ final GraphBinaryWriter context) throws
IOException {
+ throw new IOException("the String serializer must not be asked to
write");
+ }
+ }
+}
diff --git
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/TraversalStrategySerializerTest.java
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/TraversalStrategySerializerTest.java
index 1c70494000..3ee763f06c 100644
---
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/TraversalStrategySerializerTest.java
+++
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/types/TraversalStrategySerializerTest.java
@@ -27,6 +27,7 @@ import
org.apache.tinkerpop.gremlin.process.traversal.strategy.TraversalStrategy
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReadOnlyStrategy;
import org.apache.tinkerpop.gremlin.structure.io.Buffer;
+import org.apache.tinkerpop.gremlin.structure.io.ClassRegistry;
import org.apache.tinkerpop.gremlin.structure.io.binary.DataType;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryReader;
import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter;
@@ -67,6 +68,27 @@ public class TraversalStrategySerializerTest {
}
}
+ /**
+ * The other half of the one-way containment between the two registries,
seen at the wire. A name
+ * {@link ClassRegistry} holds decodes as a {@code Class} value, as
+ * {@code
ClassSerializerTest.shouldReadClassRegisteredThroughClassRegistry} shows, and
must still be refused here,
+ * because this position reflectively constructs what it resolves and only
{@code GlobalCache} admits a class to it.
+ */
+ @Test
+ public void shouldRejectStrategyNamedByAClassRegistryEntry() throws
Exception {
+ final String fqcn = ProviderType.class.getName();
+ ClassRegistry.register(ProviderType.class);
+
+ try {
+ readStrategy(reader(), fqcn);
+ fail("A class that is only nameable must not deserialize at the
strategy selector");
+ } catch (IOException ex) {
+ assertThat(ex.getMessage(), containsString("TraversalStrategy not
recognized - " + fqcn));
+ } finally {
+ ClassRegistry.unregister(ProviderType.class);
+ }
+ }
+
@Test
public void shouldRejectStrategyThatIsNotRegisteredWithoutInitializingIt()
throws Exception {
// a class literal does not initialize the class, so naming it this
way keeps the assertion below meaningful
@@ -178,6 +200,13 @@ public class TraversalStrategySerializerTest {
}
}
+ /**
+ * Stands in for a class a provider registers with {@link ClassRegistry},
which refuses a {@link TraversalStrategy},
+ * so a name that registry holds is never one the strategy selector may
construct.
+ */
+ private static final class ProviderType {
+ }
+
/**
* Stands in for the {@code ClassSerializer} to show that nothing consults
it while a strategy is read.
*/
diff --git
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-v1.gbin
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-unregistered-v1.gbin
similarity index 100%
copy from
gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-v1.gbin
copy to
gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-unregistered-v1.gbin
diff --git
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-v1.gbin
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-v1.gbin
index 6be272dc42..37e651f7f2 100644
Binary files
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-v1.gbin
and
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphbinary/class-v1.gbin
differ
diff --git
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v2.json
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v2.json
index 80f15a26ed..033a6efe65 100644
---
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v2.json
+++
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v2.json
@@ -1,4 +1,4 @@
{
"@type" : "g:Class",
- "@value" : "java.io.File"
+ "@value" :
"org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy"
}
\ No newline at end of file
diff --git
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v3.json
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v3.json
index 80f15a26ed..033a6efe65 100644
---
a/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v3.json
+++
b/gremlin-util/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/graphson/class-v3.json
@@ -1,4 +1,4 @@
{
"@type" : "g:Class",
- "@value" : "java.io.File"
+ "@value" :
"org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy"
}
\ No newline at end of file
