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.

Sign up for GitHub

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

Jump to bottom

With XML configuration, setter selection can be random in case of overloaded setter methods (e.g. on SimpleClientHttpRequestFactory in 6.1) #31872

Closed
takahashihrzg opened this issue Dec 20, 2023 · 5 comments
Closed

With XML configuration, setter selection can be random in case of overloaded setter methods (e.g. on SimpleClientHttpRequestFactory in 6.1) #31872

takahashihrzg opened this issue Dec 20, 2023 · 5 comments
Assignees
@jhoeller
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone
6.1.3

Comments

@takahashihrzg
Copy link

Affects: 6.1.0-M2 or later

Summary

In XMLConfig, if a bean-defined class has an overloaded setter, an exception may be thrown at runtime.

I checked the logs and found that the wrong setter was used.
Specifically, I found that the setter was changing each time the application was launched.

This behavior only occurs in XMLConfig and not in JavaConfig.
I think this behavior is a bug in Spring Framework, what do you think?

Detail

I found this behavior in the bean definition of SimpleClientHttpRequestFactory.
Here is an example of a Baen definition.

  <bean id="clientHttpRequestFactory" class="org.springframework.http.client.SimpleClientHttpRequestFactory">
    <property name="connectTimeout" >
      <bean class="java.time.Duration" factory-method="ofMillis">
        <constructor-arg type="long" value="2000" />
      </bean>
    </property>
    <property name="readTimeout" >
      <bean class="java.time.Duration" factory-method="ofMillis">
        <constructor-arg type="long" value="2000" />
      </bean>
    </property>
  </bean>

  <bean id="timeoutRestTemplate" class="org.springframework.web.client.RestTemplate">
    <constructor-arg ref="clientHttpRequestFactory" />
  </bean>

spring-web-6.1.0-M2 or later,
SimplyClientHttpRequestFactory have overload setter of Duration and int such as setConnectTimeout method, setReadTimeout method.
https://github.com/spring-projects/spring-framework/blob/v6.1.0-M2/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java#L99C1-L135C3

In the XMLConfig example above, my expected behavior was that a Duration setter would be used.
However, in actuality, the int setter was sometimes used.

The log of that case is shown below.

log.txt
	org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'timeoutRestClientServiceImpl': Unsatisfied dependency expressed through field 'timeoutRestTemplate': Error creating bean with name 'timeoutRestTemplate' defined in file [C:\WORK\dev\apache-tomcat-10.1.8\webapps\spring-functionaltest-web\WEB-INF\classes\META-INF\spring\rscl-app\applicationContext-rscl.xml]: Cannot resolve reference to bean 'clientHttpRequestFactory' while setting constructor argument
		at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.resolveFieldValue(AutowiredAnnotationBeanPostProcessor.java:767)
		at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:747)
		at org.springframework.beans.factory.annotation.InjectionMetadata.inject(InjectionMetadata.java:145)
		at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessProperties(AutowiredAnnotationBeanPostProcessor.java:492)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1420)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:600)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:523)
		at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:325)
		at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234)
		at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:323)
		at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
		at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:973)
		at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:939)
		at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:608)
		at org.springframework.web.context.ContextLoader.configureAndRefreshWebApplicationContext(ContextLoader.java:394)
		at org.springframework.web.context.ContextLoader.initWebApplicationContext(ContextLoader.java:274)
		at org.springframework.web.context.ContextLoaderListener.contextInitialized(ContextLoaderListener.java:102)
		at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:4454)
		at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:4892)
		at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
		at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:683)
		at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:658)
		at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:713)
		at org.apache.catalina.startup.HostConfig.deployWAR(HostConfig.java:975)
		at org.apache.catalina.startup.HostConfig$DeployWar.run(HostConfig.java:1949)
		at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
		at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
		at org.apache.tomcat.util.threads.InlineExecutorService.execute(InlineExecutorService.java:75)
		at java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:123)
		at org.apache.catalina.startup.HostConfig.deployWARs(HostConfig.java:776)
		at org.apache.catalina.startup.HostConfig.deployApps(HostConfig.java:426)
		at org.apache.catalina.startup.HostConfig.start(HostConfig.java:1656)
		at org.apache.catalina.startup.HostConfig.lifecycleEvent(HostConfig.java:309)
		at org.apache.catalina.util.LifecycleBase.fireLifecycleEvent(LifecycleBase.java:123)
		at org.apache.catalina.util.LifecycleBase.setStateInternal(LifecycleBase.java:423)
		at org.apache.catalina.util.LifecycleBase.setState(LifecycleBase.java:366)
		at org.apache.catalina.core.ContainerBase.startInternal(ContainerBase.java:898)
		at org.apache.catalina.core.StandardHost.startInternal(StandardHost.java:846)
		at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
		at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1332)
		at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1322)
		at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
		at org.apache.tomcat.util.threads.InlineExecutorService.execute(InlineExecutorService.java:75)
		at java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:145)
		at org.apache.catalina.core.ContainerBase.startInternal(ContainerBase.java:871)
		at org.apache.catalina.core.StandardEngine.startInternal(StandardEngine.java:241)
		at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
		at org.apache.catalina.core.StandardService.startInternal(StandardService.java:428)
		at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
		at org.apache.catalina.core.StandardServer.startInternal(StandardServer.java:913)
		at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
		at org.apache.catalina.startup.Catalina.start(Catalina.java:795)
		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
		at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
		at java.base/java.lang.reflect.Method.invoke(Method.java:568)
		at org.apache.catalina.startup.Bootstrap.start(Bootstrap.java:347)
		at org.apache.catalina.startup.Bootstrap.main(Bootstrap.java:478)
	Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'timeoutRestTemplate' defined in file [C:\WORK\dev\apache-tomcat-10.1.8\webapps\spring-functionaltest-web\WEB-INF\classes\META-INF\spring\rscl-app\applicationContext-rscl.xml]: Cannot resolve reference to bean 'clientHttpRequestFactory' while setting constructor argument
		at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:377)
		at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveValueIfNecessary(BeanDefinitionValueResolver.java:135)
		at org.springframework.beans.factory.support.ConstructorResolver.resolveConstructorArguments(ConstructorResolver.java:702)
		at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:206)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1356)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1193)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:563)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:523)
		at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:325)
		at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234)
		at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:323)
		at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
		at org.springframework.beans.factory.config.DependencyDescriptor.resolveCandidate(DependencyDescriptor.java:254)
		at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1441)
		at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1348)
		at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.resolveFieldValue(AutowiredAnnotationBeanPostProcessor.java:764)
		... 57 more
	Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'clientHttpRequestFactory' defined in file [C:\WORK\dev\apache-tomcat-10.1.8\webapps\spring-functionaltest-web\WEB-INF\classes\META-INF\spring\rscl-app\applicationContext-rscl.xml]: Failed to convert property value of type 'java.time.Duration' to required type 'int' for property 'readTimeout'; Cannot convert value of type 'java.time.Duration' to required type 'int' for property 'readTimeout': PropertyEditor [org.springframework.beans.propertyeditors.CustomNumberEditor] returned inappropriate value of type 'java.time.Duration'
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:608)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:523)
		at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:325)
		at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234)
		at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:323)
		at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
		at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:365)
		... 72 more
	Caused by: org.springframework.beans.TypeMismatchException: Failed to convert property value of type 'java.time.Duration' to required type 'int' for property 'readTimeout'; Cannot convert value of type 'java.time.Duration' to required type 'int' for property 'readTimeout': PropertyEditor [org.springframework.beans.propertyeditors.CustomNumberEditor] returned inappropriate value of type 'java.time.Duration'
		at org.springframework.beans.AbstractNestablePropertyAccessor.convertIfNecessary(AbstractNestablePropertyAccessor.java:598)
		at org.springframework.beans.AbstractNestablePropertyAccessor.convertForProperty(AbstractNestablePropertyAccessor.java:607)
		at org.springframework.beans.BeanWrapperImpl.convertForProperty(BeanWrapperImpl.java:190)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.convertForProperty(AbstractAutowireCapableBeanFactory.java:1734)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyPropertyValues(AbstractAutowireCapableBeanFactory.java:1691)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1435)
		at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:600)
		... 78 more
	Caused by: java.lang.IllegalArgumentException: Cannot convert value of type 'java.time.Duration' to required type 'int' for property 'readTimeout': PropertyEditor [org.springframework.beans.propertyeditors.CustomNumberEditor] returned inappropriate value of type 'java.time.Duration'
		at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:268)
		at org.springframework.beans.AbstractNestablePropertyAccessor.convertIfNecessary(AbstractNestablePropertyAccessor.java:588)
		... 84 more
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 20, 2023
@bclozel
Copy link
Member

bclozel commented Dec 20, 2023

Could you share a minimal sample project that demonstrates the problem?

Can you replicate the issue with 6.1.0-M1 as well? This could be linked to #29559 and this note about parameter name retention in the upgrade guide. It doesn't look like httpcomponents is compiling their code with the -parameters option, so that could explain it.

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Dec 20, 2023
@jhoeller
Copy link
Contributor

In contrast to the sibling issue about constructor arguments, this one is not actually related to parameter name retention as far as I can tell. This is rather a consequence of us having added overloaded setter methods to SimpleClientHttpRequestFactory in 6.1 which is not properly supported by the JavaBeans Introspector, sometimes choosing an arbitrary setter method then (depending on the JVM's runtime reflection order which is not predictable).

While we arguably should have introduced those Duration-based setters with different method names instead, as traditional JavaBeans wants it to be if the argument types differ, there is also a long-standing case that we should handle this in a more lenient but predictable fashion. I'll see what I can do about this for 6.1.3.

@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 21, 2023
@jhoeller jhoeller self-assigned this Dec 21, 2023
@jhoeller jhoeller added this to the 6.1.3 milestone Dec 21, 2023
@jhoeller jhoeller changed the title In XMLConfig, there is a case that the setter selection is random With XML configuration, setter selection can be random in case of overloaded setter methods Dec 21, 2023
@jhoeller jhoeller added type: bug A general bug and removed type: enhancement A general enhancement status: waiting-for-feedback We need additional information before we can continue labels Dec 21, 2023
@jhoeller jhoeller changed the title With XML configuration, setter selection can be random in case of overloaded setter methods With XML configuration, setter selection can be random in case of overloaded setter methods (e.g. on SimpleClientHttpRequestFactory in 6.1) Dec 21, 2023
@dima-sarin
Copy link

This is still reproducible for me exactly with the same XML Bean definition on 6.1.3 version. According to stack trace the exception (TypeMismatchException) is happening here

spring-framework/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

Line 1689 in 2e9d6a1

convertedValue = convertForProperty(resolvedValue, propertyName, bw, converter);

As far as I understand the fix by @jhoeller will help if convertIfNecessary method is called later @

spring-framework/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

Line 1717 in 2e9d6a1

bw.setPropertyValues(new MutablePropertyValues(deepCopy));

Stack trace:

Caused by: org.springframework.beans.TypeMismatchException: Failed to convert property value of type 'java.time.Duration' to required type 'int' for property 'connectTimeout'; Cannot convert value of type 'java.time.Duration' to required type 'int' for property 'connectTimeout': PropertyEditor [org.springframework.beans.propertyeditors.CustomNumberEditor] returned inappropriate value of type 'java.time.Duration'
	at org.springframework.beans.AbstractNestablePropertyAccessor.convertIfNecessary(AbstractNestablePropertyAccessor.java:600)
	at org.springframework.beans.AbstractNestablePropertyAccessor.convertForProperty(AbstractNestablePropertyAccessor.java:609)
	at org.springframework.beans.BeanWrapperImpl.convertForProperty(BeanWrapperImpl.java:189)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.convertForProperty(AbstractAutowireCapableBeanFactory.java:1732)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyPropertyValues(AbstractAutowireCapableBeanFactory.java:1689)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1433)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:598)

Any suggestions?

@jhoeller
Copy link
Contributor

@dima-sarin good catch, we should be equally defensive when pre-converting a value there, sorry for missing this the first time around. Could you please create a follow-up GitHub issue linking to this one? I'll revisit this for 6.1.4 then.

@dima-sarin dima-sarin mentioned this issue Jan 30, 2024
Closed
@dima-sarin
Copy link

@jhoeller , I really appreciate such a quick turnaround on this one.
I've created a new issue (#32159).

Thanks again!

jhoeller added a commit that referenced this issue Jan 30, 2024
@jhoeller
Avoid pre-conversion attempt in case of overloaded write methods
af5acb6 
Closes gh-32159
See gh-31872
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jhoeller jhoeller

Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Milestone
6.1.3
Development

No branches or pull requests
5 participants
@bclozel @jhoeller @spring-projects-issues @takahashihrzg @dima-sarin