From 8059401ba60327a863b4c4d0106e968e6bd5efa3 Mon Sep 17 00:00:00 2001 From: jansupol Date: Tue, 4 Aug 2020 16:42:14 +0200 Subject: [PATCH] Make Kryo use setRegistrationRequired(true) by default Signed-off-by: jansupol --- .../glassfish/jersey/kryo/KryoFeature.java | 39 +++++++++++++- .../internal/KryoMessageBodyProvider.java | 41 +++++++++----- ...trationNotRequiredKryoContextResolver.java | 44 +++++++++++++++ ...lication.java => KryoContextResolver.java} | 27 ++++------ .../jersey/kryo/PersonResourceBaseTest.java | 54 +++++++++++++++++++ ...onResourceRegistrationNotRequiredTest.java | 37 +++++++++++++ .../jersey/kryo/PersonResourceTest.java | 34 ++---------- .../mbw/kryo/JaxRsApplication.java | 9 +++- .../mbw/kryo/PersonResourceTest.java | 4 +- 9 files changed, 226 insertions(+), 63 deletions(-) create mode 100644 incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/RegistrationNotRequiredKryoContextResolver.java rename incubator/kryo/src/test/java/org/glassfish/jersey/kryo/{JaxRsApplication.java => KryoContextResolver.java} (59%) create mode 100644 incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceBaseTest.java create mode 100644 incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceRegistrationNotRequiredTest.java diff --git a/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/KryoFeature.java b/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/KryoFeature.java index 963c26334d..f2af34c5bb 100644 --- a/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/KryoFeature.java +++ b/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/KryoFeature.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -22,21 +22,58 @@ import org.glassfish.jersey.Beta; import org.glassfish.jersey.kryo.internal.KryoMessageBodyProvider; +import org.glassfish.jersey.kryo.internal.RegistrationNotRequiredKryoContextResolver; /** + *

* Feature used to register Kryo providers. + *

+ *

+ * For the security reasons, Kryo#setRegistrationRequired(true) should be specified. + * Unless {@code KryoFeature#registrationRequired(false)} is registered, a {@code ContextResolver} should be registered. + * There the user is expected to create new {@code Kryo} instance with the registrations: + *

+ * public Kryo getContext(Class type) {
+ *      ...
+ *      Kryo kryo = new Kryo();
+ *      kryo.setRegistrationRequired(true);
+ *      kryo.register(The_class_for_which_the_KryoMessageBodyProvider_should_be_allowed);
+ *      ...
+ *      return kryo;
+ * }
+ * 
+ * Note that {@code ContextResolver#getContext} is invoked just once when creating {@code KryoPool} and the {@code type} argument + * is {@code null}. + *

* * @author Libor Kramolis */ @Beta public class KryoFeature implements Feature { + private final boolean registrationRequired; + + public KryoFeature() { + registrationRequired = true; + } + + public static KryoFeature registrationRequired(boolean registrationRequired) { + return new KryoFeature(registrationRequired); + } + + private KryoFeature(boolean registrationRequired) { + this.registrationRequired = registrationRequired; + } + @Override public boolean configure(final FeatureContext context) { final Configuration config = context.getConfiguration(); if (!config.isRegistered(KryoMessageBodyProvider.class)) { context.register(KryoMessageBodyProvider.class); + if (!registrationRequired) { + context.register(RegistrationNotRequiredKryoContextResolver.class); + } } return true; diff --git a/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/KryoMessageBodyProvider.java b/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/KryoMessageBodyProvider.java index ccc3f06e92..096c0a8ded 100644 --- a/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/KryoMessageBodyProvider.java +++ b/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/KryoMessageBodyProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -21,15 +21,20 @@ import java.io.OutputStream; import java.lang.annotation.Annotation; import java.lang.reflect.Type; +import java.util.Optional; import javax.ws.rs.Consumes; import javax.ws.rs.Produces; import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Configuration; +import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.ext.ContextResolver; import javax.ws.rs.ext.MessageBodyReader; import javax.ws.rs.ext.MessageBodyWriter; import javax.ws.rs.ext.Provider; +import javax.ws.rs.ext.Providers; import com.esotericsoftware.kryo.Kryo; import com.esotericsoftware.kryo.io.Input; @@ -39,6 +44,7 @@ import com.esotericsoftware.kryo.pool.KryoPool; /** + * The KryoMessageBodyProvider expects a {@code ContextResolver} registered. * @author Libor Kramolis */ @Provider @@ -46,18 +52,27 @@ @Produces("application/x-kryo") public class KryoMessageBodyProvider implements MessageBodyWriter, MessageBodyReader { - private final KryoPool kryoPool; + private final ContextResolver contextResolver; + private final Optional kryoPool; - public KryoMessageBodyProvider() { + public KryoMessageBodyProvider(@Context Providers providers) { + final MediaType mediaType = new MediaType("application", "x-kryo"); + contextResolver = providers.getContextResolver(Kryo.class, mediaType); + kryoPool = getKryoPool(); + } + + private Kryo getKryo() { + return contextResolver != null ? contextResolver.getContext(null) : null; + } + + private Optional getKryoPool() { + final Kryo kryo = getKryo(); final KryoFactory kryoFactory = new KryoFactory() { public Kryo create() { - final Kryo kryo = new Kryo(); - //TODO: configure kryo instance, customize settings - //TODO: e.g. looking for Kryo via ContextResolver (like Jackson) return kryo; } }; - kryoPool = new KryoPool.Builder(kryoFactory).softReferences().build(); + return kryo == null ? Optional.empty() : Optional.of(new KryoPool.Builder(kryoFactory).softReferences().build()); } // @@ -73,7 +88,7 @@ public long getSize(final Object object, final Class type, final Type generic @Override public boolean isWriteable(final Class type, final Type genericType, final Annotation[] annotations, final MediaType mediaType) { - return true; + return kryoPool != null; } @Override @@ -82,12 +97,12 @@ public void writeTo(final Object object, final Class type, final Type generic final MultivaluedMap httpHeaders, final OutputStream entityStream) throws IOException, WebApplicationException { final Output output = new Output(entityStream); - kryoPool.run(new KryoCallback() { + kryoPool.ifPresent(a -> a.run(new KryoCallback() { public Object execute(Kryo kryo) { kryo.writeObject(output, object); return null; } - }); + })); output.flush(); } @@ -98,7 +113,7 @@ public Object execute(Kryo kryo) { @Override public boolean isReadable(final Class type, final Type genericType, final Annotation[] annotations, final MediaType mediaType) { - return true; + return kryoPool != null; } @Override @@ -108,11 +123,11 @@ public Object readFrom(final Class type, final Type genericType, final InputStream entityStream) throws IOException, WebApplicationException { final Input input = new Input(entityStream); - return kryoPool.run(new KryoCallback() { + return kryoPool.map(pool -> pool.run(new KryoCallback() { public Object execute(Kryo kryo) { return kryo.readObject(input, type); } - }); + })).orElse(null); } } diff --git a/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/RegistrationNotRequiredKryoContextResolver.java b/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/RegistrationNotRequiredKryoContextResolver.java new file mode 100644 index 0000000000..786c2fb85e --- /dev/null +++ b/incubator/kryo/src/main/java/org/glassfish/jersey/kryo/internal/RegistrationNotRequiredKryoContextResolver.java @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package org.glassfish.jersey.kryo.internal; + +import com.esotericsoftware.kryo.Kryo; + +import javax.ws.rs.ext.ContextResolver; + +/** + * Backwards compatibility ContextResolver. + * It should only be used when the user specifically agrees on a vulnerability provided when the + * KryoFeature#registrationRequired(false) is used. + * The default behaviour is demanded to require {@code ContextResolver} with + *
+ * public Kryo getContext(Class type) {
+ *      ...
+ *      Kryo kryo = new Kryo();
+ *      kryo.setRegistrationRequired(true);
+ *      kryo.register(The_class_for_which_the_KryoMessageBodyProvider_should_be_allowed);
+ *      ...
+ *      return kryo;
+ * }
+ * 
+ */ +public class RegistrationNotRequiredKryoContextResolver implements ContextResolver { + @Override + public Kryo getContext(Class type) { + return new Kryo(); + } +} diff --git a/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/JaxRsApplication.java b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/KryoContextResolver.java similarity index 59% rename from incubator/kryo/src/test/java/org/glassfish/jersey/kryo/JaxRsApplication.java rename to incubator/kryo/src/test/java/org/glassfish/jersey/kryo/KryoContextResolver.java index cf0f35f5f5..789b968f72 100644 --- a/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/JaxRsApplication.java +++ b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/KryoContextResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -16,25 +16,16 @@ package org.glassfish.jersey.kryo; -import java.util.HashSet; -import java.util.Set; +import com.esotericsoftware.kryo.Kryo; -import javax.ws.rs.core.Application; - -/** - * Test case JAX-RS application. - * - * @author Libor Kramolis - */ -public class JaxRsApplication extends Application { - - static final Set> APP_CLASSES = new HashSet>() {{ - add(PersonResource.class); - }}; +import javax.ws.rs.ext.ContextResolver; +public class KryoContextResolver implements ContextResolver { @Override - public Set> getClasses() { - return APP_CLASSES; + public Kryo getContext(Class type) { + Kryo kryo = new Kryo(); + kryo.setRegistrationRequired(true); + kryo.register(Person.class); + return kryo; } - } diff --git a/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceBaseTest.java b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceBaseTest.java new file mode 100644 index 0000000000..2bbedf80d8 --- /dev/null +++ b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceBaseTest.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package org.glassfish.jersey.kryo; + +import org.glassfish.jersey.test.JerseyTest; +import org.junit.Test; + +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.Response; + +import static org.junit.Assert.assertEquals; + +public abstract class PersonResourceBaseTest extends JerseyTest { + + @Test + public void testGet() { + final Person getResponse = target().request().get(Person.class); + assertEquals("Wolfgang", getResponse.name); + assertEquals(21, getResponse.age); + assertEquals("Salzburg", getResponse.address); + } + + @Test + public void testPost() { + final Person[] testData = new Person[] {new Person("Joseph", 23, "Nazareth"), new Person("Mary", 18, "Nazareth")}; + for (Person original : testData) { + final Person postResponse = target().request() + .post(Entity.entity(original, "application/x-kryo"), Person.class); + assertEquals(original, postResponse); + } + } + + @Test + public void testPut() { + final Response putResponse = target().request().put(Entity.entity(new Person("Jules", 12, "Paris"), + "application/x-kryo")); + assertEquals(204, putResponse.getStatus()); + } + +} diff --git a/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceRegistrationNotRequiredTest.java b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceRegistrationNotRequiredTest.java new file mode 100644 index 0000000000..31ef246b99 --- /dev/null +++ b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceRegistrationNotRequiredTest.java @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package org.glassfish.jersey.kryo; + +import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.test.TestProperties; + +import javax.ws.rs.core.Application; + +public class PersonResourceRegistrationNotRequiredTest extends PersonResourceBaseTest { + @Override + protected Application configure() { + enable(TestProperties.LOG_TRAFFIC); + enable(TestProperties.DUMP_ENTITY); + return new ResourceConfig().register(PersonResource.class).register(KryoFeature.registrationRequired(false)); + } + + @Override + protected void configureClient(final ClientConfig config) { + config.register(KryoFeature.registrationRequired(false)); + } +} diff --git a/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceTest.java b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceTest.java index ed28bd3508..dbbcbd64b8 100644 --- a/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceTest.java +++ b/incubator/kryo/src/test/java/org/glassfish/jersey/kryo/PersonResourceTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -21,6 +21,7 @@ import javax.ws.rs.core.Response; import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; import org.glassfish.jersey.test.TestProperties; @@ -32,42 +33,17 @@ * * @author Libor Kramolis */ -public class PersonResourceTest extends JerseyTest { +public class PersonResourceTest extends PersonResourceBaseTest { @Override protected Application configure() { enable(TestProperties.LOG_TRAFFIC); enable(TestProperties.DUMP_ENTITY); - return new JaxRsApplication(); + return new ResourceConfig().register(PersonResource.class).register(KryoContextResolver.class); } @Override protected void configureClient(final ClientConfig config) { + config.register(KryoContextResolver.class); } - - @Test - public void testGet() { - final Person getResponse = target().request().get(Person.class); - assertEquals("Wolfgang", getResponse.name); - assertEquals(21, getResponse.age); - assertEquals("Salzburg", getResponse.address); - } - - @Test - public void testPost() { - final Person[] testData = new Person[] {new Person("Joseph", 23, "Nazareth"), new Person("Mary", 18, "Nazareth")}; - for (Person original : testData) { - final Person postResponse = target().request() - .post(Entity.entity(original, "application/x-kryo"), Person.class); - assertEquals(original, postResponse); - } - } - - @Test - public void testPut() { - final Response putResponse = target().request().put(Entity.entity(new Person("Jules", 12, "Paris"), - "application/x-kryo")); - assertEquals(204, putResponse.getStatus()); - } - } diff --git a/tests/performance/test-cases/mbw-kryo/src/main/java/org/glassfish/jersey/tests/performance/mbw/kryo/JaxRsApplication.java b/tests/performance/test-cases/mbw-kryo/src/main/java/org/glassfish/jersey/tests/performance/mbw/kryo/JaxRsApplication.java index 5435a1c3e4..1c3f1c346f 100644 --- a/tests/performance/test-cases/mbw-kryo/src/main/java/org/glassfish/jersey/tests/performance/mbw/kryo/JaxRsApplication.java +++ b/tests/performance/test-cases/mbw-kryo/src/main/java/org/glassfish/jersey/tests/performance/mbw/kryo/JaxRsApplication.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -16,6 +16,9 @@ package org.glassfish.jersey.tests.performance.mbw.kryo; +import org.glassfish.jersey.kryo.KryoFeature; + +import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -37,4 +40,8 @@ public Set> getClasses() { return APP_CLASSES; } + @Override + public Set getSingletons() { + return Collections.singleton(KryoFeature.registrationRequired(false)); + } } diff --git a/tests/performance/test-cases/mbw-kryo/src/test/java/org/glassfish/jersey/tests/performance/mbw/kryo/PersonResourceTest.java b/tests/performance/test-cases/mbw-kryo/src/test/java/org/glassfish/jersey/tests/performance/mbw/kryo/PersonResourceTest.java index 919258078d..7d1d1e51b8 100644 --- a/tests/performance/test-cases/mbw-kryo/src/test/java/org/glassfish/jersey/tests/performance/mbw/kryo/PersonResourceTest.java +++ b/tests/performance/test-cases/mbw-kryo/src/test/java/org/glassfish/jersey/tests/performance/mbw/kryo/PersonResourceTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -21,6 +21,7 @@ import javax.ws.rs.core.Response; import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.kryo.KryoFeature; import org.glassfish.jersey.test.JerseyTest; import org.junit.Test; @@ -40,6 +41,7 @@ protected Application configure() { @Override protected void configureClient(final ClientConfig config) { + config.register(KryoFeature.registrationRequired(false)); } @Test