Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Avoid leaking data via Exception message.
Do not include the result value in the exception message to avoid data exposure.
Improve data flow to avoid superfluous null checks.

See #2290.
Original Pull Request: #2291.
  • Loading branch information
christophstrobl committed Feb 9, 2021
1 parent cdeecd4 commit e3a8edf
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 29 deletions.
Expand Up @@ -27,7 +27,6 @@

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import org.springframework.core.CollectionFactory;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.util.ClassTypeInformation;
Expand All @@ -45,6 +44,7 @@
*
* @author Oliver Gierke
* @author Mark Paluch
* @author Christoph Strobl
* @since 1.10
*/
class ProjectingMethodInterceptor implements MethodInterceptor {
Expand Down Expand Up @@ -98,20 +98,22 @@ protected Object potentiallyConvertResult(TypeInformation<?> type, @Nullable Obj
return null;
}

if (type.isCollectionLike() && !ClassUtils.isPrimitiveArray(type.getType())) {
Class<?> targetType = type.getType();

if (type.isCollectionLike() && !ClassUtils.isPrimitiveArray(targetType)) {
return projectCollectionElements(asCollection(result), type);
} else if (type.isMap()) {
return projectMapValues((Map<?, ?>) result, type);
} else if (conversionRequiredAndPossible(result, type.getType())) {
return conversionService.convert(result, type.getType());
} else if (ClassUtils.isAssignable(type.getType(), result.getClass())) {
} else if (ClassUtils.isAssignable(targetType, result.getClass())) {
return result;
} else if (type.getType().isInterface()) {
return getProjection(result, type.getType());
} else if (conversionService.canConvert(result.getClass(), targetType)) {
return conversionService.convert(result, targetType);
} else if (targetType.isInterface()) {
return getProjection(result, targetType);
} else {
throw new UnsupportedOperationException(String.format(
"Cannot convert value '%s' of type '%s' to '%s' and cannot create a projection as the target type is not an interface",
result, ClassUtils.getDescriptiveType(result), ClassUtils.getQualifiedName(type.getType())));
throw new UnsupportedOperationException(
String.format("Cannot project %s to %s. Target type is not an interface and no matching Converter found!",
ClassUtils.getDescriptiveType(result), ClassUtils.getQualifiedName(targetType)));
}
}

Expand Down Expand Up @@ -166,23 +168,6 @@ private Object getProjection(@Nullable Object result, Class<?> returnType) {
: factory.createProjection(returnType, result);
}

/**
* Returns whether the source object needs to be converted to the given target type and whether we can convert it at
* all.
*
* @param source can be {@literal null}.
* @param targetType must not be {@literal null}.
* @return
*/
private boolean conversionRequiredAndPossible(Object source, Class<?> targetType) {

if (source == null || targetType.isInstance(source)) {
return false;
}

return conversionService.canConvert(source.getClass(), targetType);
}

/**
* Turns the given value into a {@link Collection}. Will turn an array into a collection an wrap all other values into
* a single-element collection.
Expand Down
Expand Up @@ -43,6 +43,7 @@
* @author Oliver Gierke
* @author Saulo Medeiros de Araujo
* @author Mark Paluch
* @author Christoph Strobl
*/
@ExtendWith(MockitoExtension.class)
class ProjectingMethodInterceptorUnitTests {
Expand Down Expand Up @@ -96,7 +97,7 @@ void considersPrimitivesAsWrappers() throws Throwable {
verify(factory, times(0)).createProjection((Class<?>) any(), any());
}

@Test // #2290
@Test // GH-2290
void failsForNonConvertibleTypes() throws Throwable {

MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService);
Expand All @@ -106,7 +107,6 @@ void failsForNonConvertibleTypes() throws Throwable {

assertThatThrownBy(() -> methodInterceptor.invoke(invocation)) //
.isInstanceOf(UnsupportedOperationException.class) //
.hasMessageContaining("'1'") //
.hasMessageContaining("BigInteger") //
.hasMessageContaining("boolean");
}
Expand Down

0 comments on commit e3a8edf

Please sign in to comment.