StackOverflowError, ClassCastException and other issues with ConversionService [SPR-7488]

[spring-projects-issues]

Oleg Zhurakousky opened SPR-7488 and commented

We have the following scenario in Spring Integration. We are trying to extract MessageHeaders from the Message and map MessageHeaders as input parameter of the method that takes Properties type. SpEL uses ConversionService to convert MessageHeaders to Properties obviously trying to convert every element to a String. When it gets to $history header which is initially an empty ArrayList it attempts to convert it to a String.
When ArrayList is empty we get StackOverflowError. When ArrayList contains at least one element everything is fine.

First ConversionService finds the right Converter which is ObjectToCollectionConverter.
ObjectToCollectionConverter gets TypeDescriptor for target type and this is where things get really interesting.
TypeDescriptor attempts to resolve common type of the Collection via call to CollectionUtils.findCommonElementType(collection) which is based on looking at the first element and comparing it to every other element after that and if they all match that is the common type otherwise it will return 'null'. That explains why everything works when ArrayList contained at least one element. However in our case its empty so TypeDescriptor is making another attempt to extract the collection type by using GenericCollectionTypeResolver.getCollectionType() and that is where it gets fishy because the first line of code there is:

if (clazz.getName().startsWith("java.util.")) {
	return null;
}

... which obviously results in null for our case and eventually results in elementType of this TypeDescriptor to be set to TypeDescriptor.NULL, which is then takes the actual type of the collection and makes it a target type via TypeDescriptor(object) where 'object' is the actual collection (instead of the its type or common type) thus resulting in the cycle causing StackOverflowError.

Questions:

1. If target type is Properties should MapToMapConverter simply call toString() on every value? Although I personally disagree that toString() method should be used as default for conversion it is my understanding that it is currently the assumption of the ConversionService.
2. Why GenericCollectionTypeResolver.getCollectionType() ignores java.util package?

I am attaching the test cases which reproduce all these plus few more minor issues in isolation.

---

Affects: 3.0.4

Attachments:

• StackOverflowExceptionTest.java (5.31 kB)

Issue Links:

• ConversionService.canConvert(..) is inconsistent with the behavior of ConversionService.convert(..) method. [SPR-7548] #12205 ConversionService.canConvert(..) is inconsistent with the behavior of ConversionService.convert(..) method.

Referenced from: commits 0a17e41

[spring-projects-issues]

Juergen Hoeller commented

Researching those failing tests at the moment...

A couple of quick initial comments:

• That "java.util." exclusion code in GenericCollectionTypeResolver is an optimization: The base collection classes in that package are not typed for specific key or value types, so they are not worth checking.
• java.util.Properties is commonly used with String keys and String values; however, it is technically a Map<Object, Object> for backwards compatibility reasons (since it used to extend a plain Hashtable which did accept other key/value types).

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Another note: The simpleClassCastException test performs an invalid call to the ConversionService... It passes in a List as a source value but then claims a source type descriptor for a String and a target type descriptor for the list! Switching the type descriptors to the correct order makes the test pass. So the best we can do there is to catch such invalid arguments upfront and to throw an IllegalArgumentException in that case.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

In the failWithEmptyListOnSimpleConversion test, do you really mean to pass in a plain Object TypeDescriptor and a list as a target type here, with the source value being a list as well?

conversionService.convert(list, TypeDescriptor.forObject(new Object()), TypeDescriptor.forObject(list));

In general, the source type descriptor should be as specific as possible a match with the given source value. Is this possibly an accident as well, with the TypeDescriptor arguments to be switched?

Juergen

[spring-projects-issues]

Juergen Hoeller commented

I've fixed this as per my analysis above: We're catching invalid arguments early now, and we're avoiding a stack overflow in the object-to-collection case even if the passed-in source object happens to be a collection as well (along with a source TypeDescriptor for Object).

If there is anything I've missed, let me know.

Juergen

[spring-projects-issues]

Oleg Zhurakousky commented

Thanks Juergen

Just a few quick once:
The main test that caused the issue is of course the first one, the other once were mainly to support the first one as well as show other possible scenarios encountered during the research, however wrong/right they might have been.
The simpleClassCastException test was just something I accidentally noticed while digging through the original problem, so i figured i'll let you know for pure awareness.

As for failWithEmptyListOnSimpleConversion test; Of course i didn't mean to pass the Object. This test was meant to spare you from some digging as it is a by-product of the first (main) test (look at the comments in the first test) where having empty ArrayList resulted in CS treating it as an Object type.

Anyway, thanks for acting on it quickly.

[spring-projects-issues]

Juergen Hoeller commented

Alright, I eventually figured that this might be the end result of your experiments - in a mission to break the ConversionService :-)

I originally thought that some of those tests where derived from actual use cases. I guess they were, but not in the shape that they ended up with here.

Anyway, I think this should be all sorted now. Feel free to give the latest 3.0.5 snapshot a try...

Juergen