We've got an application that uses a lot of XML wiring - since it dates back to before Java config was a thing.
We've recently discovered that Spring is autowiring in beans when we didn't expect it to - we'd got a typo in a constructor argument name and instead of an error things were still running.
I've since reproduced this in a trivial application - which is attached. autowire-demo.zip
In this, we've got:
* A ServiceB
class
* A ServiceA
class that requires an instance of ServiceB
* A controller class that requires an instance of ServiceA
. (This was simply to be able to test the wiring)
* A spring.xml
context file for constructing ServiceA
and ServiceB
.
However, you'll notice that in spring.xml
we've got this:
<bean id="serviceB" class="com.example.autowire_demo.ServiceB" />
<bean id="serviceA" class="com.example.autowire_demo.ServiceA" autowire="no">
</bean>
So the ServiceA
instance has explicitly got autowire="no"
on it, but is missing the constructor-arg
that it needs.
If you run this then you will get a working service. Spring is autowiring serviceB
into serviceA
even though it shouldn't.
In a similar manner - which I assume is related but I don't know for certain - we've noticed that if the constructor argument is a List
(or probably other collection types) and it's missing from the XML then Spring autowires in an empty collection instead of erroring. This is actually worse since at least with the above it's probably the case that there's only one candidate and so the right thing will happen by luck. With the list case, you're definitely getting the wrong behaviour.
Unfortunately this now means that we might have some missing/broken wiring and Spring isn't alerting us to the issue.
Cheers
Comment From: bclozel
Hi @grahamcox-oclc and thanks for the complete issue and repro project.
I have traced this behavior back to #16883. As of Spring Framework 4.3, the @Autowired
annotation is not required anymore on a bean if it has a single constructor. This is called out in the reference documentation.
If we remove the single constructor from ServiceA
like so:
public class ServiceA {
private ServiceB serviceB;
public List<String> getStrings() {
return serviceB.getStrings();
}
}
The endpoint fails with a NPE, because the ServiceB
instance has not been injected in ServiceA
:
➜ autowire-demo http :8080/
HTTP/1.1 500
Connection: close
Content-Type: application/json
Date: Fri, 07 Feb 2025 10:53:15 GMT
Transfer-Encoding: chunked
{
"error": "Internal Server Error",
"path": "/",
"status": 500,
"timestamp": "2025-02-07T10:53:15.425+00:00"
}
If we add another constructor to that class:
public class ServiceA {
private ServiceB serviceB;
ServiceA(ServiceB serviceB) {
this.serviceB = serviceB;
}
ServiceA(String name, ServiceB serviceB) {
this.serviceB = serviceB;
}
public List<String> getStrings() {
return serviceB.getStrings();
}
}
The startup fails because autowiring was not expressed for this bean:
Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.legacy.ServiceA]: No default constructor found
I can think of two ways to change this situation.
- we could try to enforce a hard check at the XML definition parsing level, failing or warning if the "autowire=no" mode is set, no constructor args are provided, and we have a single constructor that is not annotated
@Autowired
. Unfortunately this would mean copying a great deal of logic in the XML parsing infrastructure and perform reflection operations way too early. - we could revert #16883 but this has been a well established behavior since 4.3
In both cases, there are real risks of regressions in lots of applications and this would probably require you to change many cases in your XML configuration namespace that you didn't notice before.
Without that, I think the best course of action here is to apply best practices on the beans defined by the application: * if a single constructor accepts a list of instances and requires it not to be empty, the constructor should assert so * to avoid any confusion, bean definitions and constructor arguments should use the most specific type possible to avoid unwanted injections
I'm closing this issue for now. Thanks!