Nicolas FABRE opened SPR-11489 and commented

In the class org.springframework.core.convert.support.StringToCollectionConverter which converts String to List\, if a single String element is given in input, it trims (remove blanks) the element. Why ?

It is not a desired behavior in our case.

Line 68 : target.add(field.trim());

Could this trim() be removed ?

Thanks

Affects: 4.0.2

Attachments: - test.zip (4.84 MB)

Comment From: spring-projects-issues

Stéphane Nicoll commented

We could maybe add an option to enable that trim on every elements found in the input String which would be true by default.

That being said, I don't really understand why you make a difference between one element and a set of elements. Are you basically saying that you are ok with

"__myString_,_anotherString_" -> ["myString","anotherString"]

and not with

"__myString__" -> ["myString"]

(where _ means space)

Comment From: spring-projects-issues

Nicolas FABRE commented

Stéphane,

After further investigations, here is the more detailed problem we face. I join a small Spring MVC project which illustrate the problem.

We have a select multiple input in a JSP. It is implemented with spring form taglib and bind to a ModelAttribute attribute List\. If we select just one \

Then, without any conversion service registered, the value is set to the ModelAttribute by using the CustomCollectionEditor.setValue(..) at the end (after being passed through ServletRequestDataBinder.bind(..), BeanWrapperImpl.setPropertyValue(..), TypeConverterDelegate.doConvert(..), ...). In this case, the value is correctly set without being trimed.

However, if we register a conversion service org.springframework.format.support.FormattingConversionServiceFactoryBean, then the TypeConverterDelegate finds it and uses it instead of using CustomCollectionEditor. Finally the single String value is converted by the StringToCollectionConverter which is pre-registered in FormattingConversionServiceFactoryBean and the white spaces are trimed.

I think the problem here is that StringToCollectionConverter and CustomCollectionEditor behavior are not consistent in this use case. However it's fair that StringToCollectionConverter in itself tries to trim. It's the same for CustomCollectionEditor's behavior. Seeing, what it is supposed to do, it does it well.

From my point of view, the real problem is initially that the HTTP param value is seen as a single String whereas it is a value of a HTML select multiple (WebUtils.getParametersStartingWith(..) behavior). Instead, I think that it would be more logical to keep the array of String with just one element in the case of a select multiple. The conversion which would occur then, would be Array to List and there would no problem anymore.

Sorry for the lack of analysis before having created this current issue. I think an issue on Spring MVC's ServletRequestDataBinder would make more sense. What's your point of view ?

Comment From: spring-projects-issues

Nicolas FABRE commented

Here is the small project which illustrates the issue. Build it with Gradle.

Comment From: spring-projects-issues

Nicolas FABRE commented

By the way, to confirm the analysis above, removing StringToCollectionConverter (despite is initially automatically added by FormattingConversionServiceFactoryBean) solves the issue.

public class MyFormatterRegistrar implements FormatterRegistrar {
    @Override
    public void registerFormatters(FormatterRegistry registry) {
        registry.removeConvertible(String.class, Collection.class);
    }
}

Comment From: spring-projects-issues

Stéphane Nicoll commented

I have added your sample project to the spring-framework-issues repo: https://github.com/spring-projects/spring-framework-issues/tree/master/SPR-11489

Comment From: spring-projects-issues

Stéphane Nicoll commented

You are exposed to a corner-case situation mainly because the IDs of your select are affected by a trim. See the documentation for more details on how you can use a separate identifier than the display value.

Spring MVC is trying to do its best to accommodate with as many use cases as possible with the default settings. It might happen that these are not enough and you should then explicitly register a property editor.

Adding this to the controller will honour the spacings in your IDs:

@InitBinder
public void registerCodesPropertyEditor(WebDataBinder binder) {
    binder.registerCustomEditor(List.class, "codes", new CustomCollectionEditor(List.class));
}

The real problem now is that the behaviour of the generic conversion service is not strictly identical to the one of the default property editor (in this case the CustomCollectionEditor). Changing either one of those by default would definitely have side effects on existing applications.

At this point, I'd strongly suggest you not to rely to outer spaces in your IDs. If you cannot afford that, please register a custom editor explicitly as explained above.

Let me know if I am missing anything. Thanks!

Comment From: spring-projects-issues

Nicolas FABRE commented

Stéphane,

You are not missing anything. I agree with you to not do anything for this issue. It would be too cubersome to make Spring know what is the type of HTML field associated to a given HTTP param value and apply different conversion according to that. The suppleness brought by the registration of property editor is just fine.

Trust me, when I design IDs myself, I never put outer spaces ! :-) Here, I have not the choice as those IDs are given by another system.

Thanks a lot / Merci beaucoup en tout cas !

Comment From: rstoyanchev

This will be addressed in #23850.