#432 Injector - prevent static field injection, add more tests

This commit is contained in:
ljacqu 2016-04-30 10:44:32 +02:00
parent 2c491803d3
commit 908399e271
14 changed files with 211 additions and 45 deletions

View File

@ -253,8 +253,8 @@ public class AuthMe extends JavaPlugin {
initializer.provide(DataFolder.class, getDataFolder());
// Register elements we instantiate manually
initializer.register(newSettings);
initializer.register(messages);
initializer.register(NewSetting.class, newSettings);
initializer.register(Messages.class, messages);
initializer.register(DataSource.class, database);
initializer.provide(BaseCommands.class, CommandInitializer.buildCommands(initializer));

View File

@ -9,7 +9,6 @@ import java.lang.annotation.Annotation;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Type;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@ -51,20 +50,6 @@ public class AuthMeServiceInitializer {
return get(clazz, new HashSet<Class<?>>());
}
/**
* Registers an instantiation by its type.
*
* @param object the object to register
* @throws IllegalStateException if an object of the same type has already been registered
*/
public void register(Object object) {
if (object instanceof Type) {
throw new IllegalStateException("You tried to register a Type object: '" + object
+ "'. This likely indicates an error. Please use register(Class<T>, T) if really desired.");
}
storeObject(object);
}
/**
* Register an object with a custom class (supertype). Use this for example to specify a
* concrete implementation of an interface or an abstract class.

View File

@ -8,7 +8,7 @@ import java.lang.annotation.Target;
/**
* Annotation to denote the collection of AuthMe commands.
*/
@Target(ElementType.PARAMETER)
@Target({ElementType.PARAMETER, ElementType.FIELD})
@Retention(RetentionPolicy.RUNTIME)
public @interface BaseCommands {
}

View File

@ -1,5 +1,7 @@
package fr.xephi.authme.initialization;
import com.google.common.base.Preconditions;
import javax.inject.Inject;
import javax.inject.Provider;
import java.lang.annotation.Annotation;
@ -36,6 +38,7 @@ class ConstructorInjection<T> implements Injection<T> {
@Override
public T instantiateWith(Object... values) {
validateNoNullValues(values);
try {
return constructor.newInstance(values);
} catch (InstantiationException | IllegalAccessException | InvocationTargetException e) {
@ -60,7 +63,7 @@ class ConstructorInjection<T> implements Injection<T> {
*
* @param clazz the class to process
* @param <T> the class' type
* @return injection constructor for the class
* @return injection constructor for the class, null if not applicable
*/
@SuppressWarnings("unchecked")
private static <T> Constructor<T> getInjectionConstructor(Class<T> clazz) {
@ -74,4 +77,10 @@ class ConstructorInjection<T> implements Injection<T> {
return null;
}
private static void validateNoNullValues(Object[] array) {
for (Object entry : array) {
Preconditions.checkNotNull(entry);
}
}
}

View File

@ -8,7 +8,7 @@ import java.lang.annotation.Target;
/**
* Annotation for specifying the plugin's data folder.
*/
@Target(ElementType.PARAMETER)
@Target({ElementType.PARAMETER, ElementType.FIELD})
@Retention(RetentionPolicy.RUNTIME)
public @interface DataFolder {
}

View File

@ -8,6 +8,7 @@ import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@ -57,6 +58,7 @@ class FieldInjection<T> implements Injection<T> {
for (int i = 0; i < fields.length; ++i) {
try {
Preconditions.checkNotNull(values[i]);
fields[i].set(instance, values[i]);
} catch (IllegalAccessException e) {
throw new UnsupportedOperationException(e);
@ -65,15 +67,15 @@ class FieldInjection<T> implements Injection<T> {
return instance;
}
private static Class<?> getFirstNonInjectAnnotation(Field field) {
for (Annotation annotation : field.getAnnotations()) {
if (annotation.annotationType() != Inject.class) {
return annotation.annotationType();
}
}
return null;
}
/**
* Returns a provider for a {@code FieldInjection<T>} instance, i.e. a provides an object
* with which field injection can be performed on the given class if applicable. The provided
* value is {@code null} if field injection cannot be applied to the class.
*
* @param clazz the class to provide field injection for
* @param <T> the class' type
* @return field injection provider for the given class
*/
public static <T> Provider<FieldInjection<T>> provide(final Class<T> clazz) {
return new Provider<FieldInjection<T>>() {
@Override
@ -92,6 +94,10 @@ class FieldInjection<T> implements Injection<T> {
List<Field> fields = new ArrayList<>();
for (Field field : clazz.getDeclaredFields()) {
if (field.isAnnotationPresent(Inject.class)) {
if (Modifier.isStatic(field.getModifiers())) {
throw new IllegalStateException(String.format("Field '%s' in class '%s' is static but "
+ "annotated with @Inject", field.getName(), clazz.getSimpleName()));
}
field.setAccessible(true);
fields.add(field);
}
@ -99,6 +105,15 @@ class FieldInjection<T> implements Injection<T> {
return fields;
}
private static Class<?> getFirstNonInjectAnnotation(Field field) {
for (Annotation annotation : field.getAnnotations()) {
if (annotation.annotationType() != Inject.class) {
return annotation.annotationType();
}
}
return null;
}
private static <T> Constructor<T> getDefaultConstructor(Class<T> clazz) {
try {
Constructor<?> defaultConstructor = clazz.getDeclaredConstructor();

View File

@ -10,6 +10,7 @@ import fr.xephi.authme.initialization.samples.Duration;
import fr.xephi.authme.initialization.samples.FieldInjectionWithAnnotations;
import fr.xephi.authme.initialization.samples.InvalidClass;
import fr.xephi.authme.initialization.samples.InvalidPostConstruct;
import fr.xephi.authme.initialization.samples.InvalidStaticFieldInjection;
import fr.xephi.authme.initialization.samples.PostConstructTestClass;
import fr.xephi.authme.initialization.samples.ProvidedClass;
import fr.xephi.authme.initialization.samples.Size;
@ -34,7 +35,7 @@ public class AuthMeServiceInitializerTest {
@Before
public void setInitializer() {
initializer = new AuthMeServiceInitializer(ALLOWED_PACKAGE);
initializer.register(new ProvidedClass(""));
initializer.register(ProvidedClass.class, new ProvidedClass(""));
}
@Test
@ -128,7 +129,7 @@ public class AuthMeServiceInitializerTest {
@Test(expected = RuntimeException.class)
public void shouldThrowForSecondRegistration() {
// given / when / then
initializer.register(new ProvidedClass(""));
initializer.register(ProvidedClass.class, new ProvidedClass(""));
}
@Test(expected = RuntimeException.class)
@ -152,15 +153,6 @@ public class AuthMeServiceInitializerTest {
initializer.register(String.class, null);
}
@Test(expected = RuntimeException.class)
public void shouldThrowForRegisterOfType() {
// given / when / then
// this most likely means that the second argument was forgotten, so throw an error and force
// the API user to use the explicit register(Class.class, String.class) if really, really desired
// (Though for such generic types, an annotation would be a lot better)
initializer.register(String.class);
}
@Test
public void shouldExecutePostConstructMethod() {
// given
@ -215,7 +207,7 @@ public class AuthMeServiceInitializerTest {
@Test(expected = RuntimeException.class)
public void shouldThrowForAlreadyRegisteredClass() {
// given
initializer.register(new BetaManager());
initializer.register(BetaManager.class, new BetaManager());
// when / then
initializer.register(BetaManager.class, new BetaManager());
@ -233,4 +225,10 @@ public class AuthMeServiceInitializerTest {
assertThat(singletonScoped, not(sameInstance(requestScoped)));
}
@Test(expected = RuntimeException.class)
public void shouldThrowForStaticFieldInjection() {
// given / when / then
initializer.newInstance(InvalidStaticFieldInjection.class);
}
}

View File

@ -0,0 +1,81 @@
package fr.xephi.authme.initialization;
import fr.xephi.authme.initialization.samples.AlphaService;
import fr.xephi.authme.initialization.samples.ClassWithAnnotations;
import fr.xephi.authme.initialization.samples.Duration;
import fr.xephi.authme.initialization.samples.GammaService;
import fr.xephi.authme.initialization.samples.InvalidClass;
import fr.xephi.authme.initialization.samples.ProvidedClass;
import fr.xephi.authme.initialization.samples.Size;
import org.junit.Test;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
/**
* Test for {@link ConstructorInjection}.
*/
public class ConstructorInjectionTest {
@Test
public void shouldReturnDependencies() {
// given
Injection<ClassWithAnnotations> injection = ConstructorInjection.provide(ClassWithAnnotations.class).get();
// when
Class<?>[] dependencies = injection.getDependencies();
Class<?>[] annotations = injection.getDependencyAnnotations();
// then
assertThat(dependencies, arrayContaining(int.class, GammaService.class, long.class));
assertThat(annotations, arrayContaining((Class<?>) Size.class, null, Duration.class));
}
@Test
public void shouldInstantiate() {
// given
GammaService gammaService = new GammaService(
AlphaService.newInstance(new ProvidedClass("")));
Injection<ClassWithAnnotations> injection = ConstructorInjection.provide(ClassWithAnnotations.class).get();
// when
ClassWithAnnotations instance = injection.instantiateWith(-112, gammaService, 19L);
// then
assertThat(instance, not(nullValue()));
assertThat(instance.getSize(), equalTo(-112));
assertThat(instance.getGammaService(), equalTo(gammaService));
assertThat(instance.getDuration(), equalTo(19L));
}
@Test(expected = NullPointerException.class)
public void shouldThrowForNullValue() {
// given
Injection<ClassWithAnnotations> injection = ConstructorInjection.provide(ClassWithAnnotations.class).get();
// when / then
injection.instantiateWith(-112, null, 12L);
}
@Test(expected = RuntimeException.class)
public void shouldThrowUponInstantiationError() {
// given
AlphaService alphaService = AlphaService.newInstance(new ProvidedClass(""));
Injection<InvalidClass> injection = ConstructorInjection.provide(InvalidClass.class).get();
// when
injection.instantiateWith(alphaService, 5);
}
@Test
public void shouldReturnNullForNoConstructorInjection() {
// given / when
Injection<FieldInjection> injection = ConstructorInjection.provide(FieldInjection.class).get();
// then
assertThat(injection, nullValue());
}
}

View File

@ -1,15 +1,19 @@
package fr.xephi.authme.initialization;
import fr.xephi.authme.initialization.samples.AlphaService;
import fr.xephi.authme.initialization.samples.BadFieldInjection;
import fr.xephi.authme.initialization.samples.BetaManager;
import fr.xephi.authme.initialization.samples.ClassWithAnnotations;
import fr.xephi.authme.initialization.samples.Duration;
import fr.xephi.authme.initialization.samples.FieldInjectionWithAnnotations;
import fr.xephi.authme.initialization.samples.GammaService;
import fr.xephi.authme.initialization.samples.InvalidStaticFieldInjection;
import fr.xephi.authme.initialization.samples.ProvidedClass;
import fr.xephi.authme.initialization.samples.Size;
import org.junit.Test;
import javax.inject.Inject;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
@ -51,4 +55,61 @@ public class FieldInjectionTest {
assertThat(betaManager.getDependencies(), arrayContaining(providedClass, gammaService, alphaService));
}
@Test
public void shouldProvideNullForImpossibleFieldInjection() {
// given / when
FieldInjection<BadFieldInjection> injection = FieldInjection.provide(BadFieldInjection.class).get();
// then
assertThat(injection, nullValue());
}
@Test(expected = RuntimeException.class)
public void shouldForwardExceptionDuringInstantiation() {
// given
FieldInjection<ThrowingConstructor> injection = FieldInjection.provide(ThrowingConstructor.class).get();
// when / when
injection.instantiateWith(new ProvidedClass(""));
}
@Test(expected = RuntimeException.class)
public void shouldThrowForInvalidFieldValue() {
// given
ProvidedClass providedClass = new ProvidedClass("");
AlphaService alphaService = AlphaService.newInstance(providedClass);
GammaService gammaService = new GammaService(alphaService);
FieldInjection<BetaManager> injection = FieldInjection.provide(BetaManager.class).get();
// when / then
// Correct order is provided, gamma, alpha
injection.instantiateWith(providedClass, alphaService, gammaService);
}
@Test(expected = NullPointerException.class)
public void shouldThrowForNullValue() {
// given
ProvidedClass providedClass = new ProvidedClass("");
AlphaService alphaService = AlphaService.newInstance(providedClass);
FieldInjection<BetaManager> injection = FieldInjection.provide(BetaManager.class).get();
// when / then
// Correct order is provided, gamma, alpha
injection.instantiateWith(providedClass, null, alphaService);
}
@Test(expected = RuntimeException.class)
public void shouldThrowForStaticFieldInjection() {
// given / when / then
FieldInjection.provide(InvalidStaticFieldInjection.class).get();
}
private static class ThrowingConstructor {
@Inject
private ProvidedClass providedClass;
public ThrowingConstructor() {
throw new UnsupportedOperationException("Exception in constructor");
}
}
}

View File

@ -5,7 +5,7 @@ import javax.inject.Inject;
/**
* Classes with circular dependencies.
*/
public class CircularClasses {
public abstract class CircularClasses {
public static final class Circular1 {
@Inject

View File

@ -3,7 +3,7 @@ package fr.xephi.authme.initialization.samples;
import javax.inject.Inject;
/**
* Sample - class dependent on alpha and provided.
* Sample - class dependent on alpha service.
*/
public class GammaService {

View File

@ -6,7 +6,7 @@ import javax.inject.Inject;
/**
* Class with invalid @PostConstruct method.
*/
public class InvalidPostConstruct {
public abstract class InvalidPostConstruct {
public static final class WithParams {
@Inject

View File

@ -0,0 +1,17 @@
package fr.xephi.authme.initialization.samples;
import javax.inject.Inject;
/**
* Sample class - attempted field injection on a static member.
*/
public class InvalidStaticFieldInjection {
@Inject
private ProvidedClass providedClass;
@Inject
protected static AlphaService alphaService;
InvalidStaticFieldInjection() { }
}

View File

@ -4,7 +4,7 @@ import javax.annotation.PostConstruct;
import javax.inject.Inject;
/**
* Sample class for testing the execution of the @PostConstruct method.
* Sample class for testing the execution of @PostConstruct methods.
*/
public class PostConstructTestClass {