Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Kryo use setRegistrationRequired(true) by default #4541

Merged
merged 1 commit into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -22,21 +22,58 @@

import org.glassfish.jersey.Beta;
import org.glassfish.jersey.kryo.internal.KryoMessageBodyProvider;
import org.glassfish.jersey.kryo.internal.RegistrationNotRequiredKryoContextResolver;
jansupol marked this conversation as resolved.
Show resolved Hide resolved

/**
* <p>
* Feature used to register Kryo providers.
* </p>
* <p>
* For the security reasons, Kryo#setRegistrationRequired(true) should be specified.
* Unless {@code KryoFeature#registrationRequired(false)} is registered, a {@code ContextResolver<Kryo>} should be registered.
* There the user is expected to create new {@code Kryo} instance with the registrations:
* <pre>
* 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;
* }
* </pre>
* Note that {@code ContextResolver#getContext} is invoked just once when creating {@code KryoPool} and the {@code type} argument
* is {@code null}.
* </p>
*
* @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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -39,25 +44,35 @@
import com.esotericsoftware.kryo.pool.KryoPool;

/**
* The KryoMessageBodyProvider expects a {@code ContextResolver<Kryo>} registered.
* @author Libor Kramolis
*/
@Provider
@Consumes("application/x-kryo")
@Produces("application/x-kryo")
public class KryoMessageBodyProvider implements MessageBodyWriter<Object>, MessageBodyReader<Object> {

private final KryoPool kryoPool;
private final ContextResolver<Kryo> contextResolver;
private final Optional<KryoPool> 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<KryoPool> 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());
}

//
Expand All @@ -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
Expand All @@ -82,12 +97,12 @@ public void writeTo(final Object object, final Class<?> type, final Type generic
final MultivaluedMap<String, Object> 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();
}

Expand All @@ -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
Expand All @@ -108,11 +123,11 @@ public Object readFrom(final Class<Object> 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);
}

}
Original file line number Diff line number Diff line change
@@ -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
* <code>KryoFeature#registrationRequired(false)</code> is used.
* The default behaviour is demanded to require {@code ContextResolver} with
* <pre>
* 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;
* }
* </pre>
*/
public class RegistrationNotRequiredKryoContextResolver implements ContextResolver<Kryo> {
@Override
public Kryo getContext(Class<?> type) {
return new Kryo();
}
}
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be 2015, 2020?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is a new file. Git thinks the file was renamed and changed, but the previous file was erased and the new file created. The files are completely different, with different purpose

*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -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<Class<?>> APP_CLASSES = new HashSet<Class<?>>() {{
add(PersonResource.class);
}};
import javax.ws.rs.ext.ContextResolver;

public class KryoContextResolver implements ContextResolver<Kryo> {
@Override
public Set<Class<?>> getClasses() {
return APP_CLASSES;
public Kryo getContext(Class<?> type) {
Kryo kryo = new Kryo();
kryo.setRegistrationRequired(true);
kryo.register(Person.class);
return kryo;
}

}
Original file line number Diff line number Diff line change
@@ -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());
}

}
Original file line number Diff line number Diff line change
@@ -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));
}
}
Loading