Affects: 5.3.31
If I need a StringValueResolver
to be injected by the bean container, I can use:
@Component
public MyService implements EmbeddedValueResolverAware {
private StringValueResolver stringValueResolver = null;
@Override
public void setEmbeddedValueResolver(StringValueResolver resolver) {
this.stringValueResolver = resolver;
}
}
However, this does not work:
@Component
public MyService implements {
@Autowired
private StringValueResolver stringValueResolver;
}
because:
org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'org.springframework.util.StringValueResolver' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {@org.springframework.beans.factory.annotation.Autowired(required=true)}
This is a bit unexpected, because in similar situations it works (for example: you can autowire an ApplicationContext
instead of using ApplicationContextAware
interface). Is there a reason? This is not only less convenient, but it also makes your code subject to flagging by tools like SonarQube as potentially subject to security vulnerabilities since you're apparently holding state in a singleton bean.
Comment From: snicoll
@Component
public MyService implements EmbeddedValueResolverAware {
@Autowired
private StringValueResolver stringValueResolver;
}
Just to make sure I understand the report, that implements EmbeddedValueResolverAware
is noise and should not be present, right?
This is a bit unexpected, because in similar situations it works (for example: you can autowire an ApplicationContext instead of using ApplicationContextAware interface).
I am not sure about it is unexpected. StringValueResolver
isn't a bean that we make available in the context, is all. I am not sure that expecting it because another type is is legit.
it also makes your code subject to flagging by tools like SonarQube as potentially subject to security vulnerabilities since you're apparently holding state in a singleton bean.
I didn't get that, can you clarify?
Comment From: mauromol
Just to make sure I understand the report, that
implements EmbeddedValueResolverAware
is noise and should not be present, right?
Yes, sorry, I've fixed it.
This is a bit unexpected, because in similar situations it works (for example: you can autowire an ApplicationContext instead of using ApplicationContextAware interface).
I am not sure about it is unexpected.
StringValueResolver
isn't a bean that we make available in the context, is all. I am not sure that expecting it because another type is is legit.
But neither ApplicationContext is such a bean, but you can get it autowired. Probably just like other Beans you can get autowired instead of using analogous *Aware interfaces (sorry, don't have code at hand, but I'm pretty sure there are others).
it also makes your code subject to flagging by tools like SonarQube as potentially subject to security vulnerabilities since you're apparently holding state in a singleton bean.
I didn't get that, can you clarify?
SonarQube flags a security warning on singletons (like @Component
s) that have instance methods not annotated with @Autowired
or @Value
or alike, because they appear as code smell, because they may be used to create stateful singletons that may leak information among different clients. Which is reasonable, unless you have very specific needs.
So, since there's no special treatment for StringValueResolver, having that non-annotated field in there triggers the mentioned flag.
Comment From: sbrannen
But neither ApplicationContext is such a bean, but you can get it autowired.
For the record, the ApplicationContext
is not a bean. Rather, it's registered as a "resolvable dependency".
https://github.com/spring-projects/spring-framework/blob/f726e806cd712297df379c7bcd1ddf83b1e398b6/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java#L738-L743
Probably just like other Beans you can get autowired instead of using analogous *Aware interfaces (sorry, don't have code at hand, but I'm pretty sure there are others).
ApplicationContext
is a special case. It implements MessageSource
, ApplicationEventPublisher
, and ResourcePatternResolver
(and therefore ResourceLoader
). Consequently, since it is a "resolvable dependency" it can be injected as any of those types directly -- for example, via @Autowired MessageSource ...
, etc.
Note, however, that ApplicationContext
does not implement StringValueResolver
.
Comment From: snicoll
Also, all the types that are mentioned here are interfaces and EmbeddedValueResolver
is not. StringValueResolver
itself is not unique enough as a type. It's simply not considered a first-class reference, which explains why you can't inject it. If you insist on injecting, you can do so with a ConfigurableBeanFactory
and then create the EmbeddedValueResolver
from it.
SonarQube flags a security warning on singletons (like @Components) that have instance methods not annotated with @Autowired or @Value or alike, because they appear as code smell, because they may be used to create stateful singletons that may leak information among different clients. Which is reasonable, unless you have very specific needs. So, since there's no special treatment for StringValueResolver, having that non-annotated field in there triggers the mentioned flag.
I am not sure I got that but if there are special treatments for ApplicationContext
, BeanFactory
, and the like, this is obviously something to do with SonarQube, and not a valid point to request this change.
Thanks for the suggestion, but we believe that the current state is fine as it is.
Comment From: mauromol
Also, all the types that are mentioned here are interfaces and
EmbeddedValueResolver
is not.StringValueResolver
itself is not unique enough as a type. It's simply not considered a first-class reference, which explains why you can't inject it. If you insist on injecting, you can do so with aConfigurableBeanFactory
and then create theEmbeddedValueResolver
from it.
You say it's not "special enough", but you have EmbeddedValueResolverAware
, which promotes it as a special enough feature, IMHO. Especially because in this way you can resolve properties with placeholders like you get with @Value
in a programmatic way: I'm not aware of any alternative which does not involve using an "old style" interface to get injected such component, is there?
I was also suggested to use it in the past (see https://github.com/spring-projects/spring-framework/issues/26795#issuecomment-828420153): indeed, in my own experience there are cases in which there's no real functional alternative.
Another strange thing is that it seems like it was possible to get this autowired in the past (see https://stackoverflow.com/a/2887618/1465635, but I also find other StackOverflow answers where the StringValueResolver
seems to be autowirable, although it doesn't work for me).
Just above the code fragment from AbstractApplicationContext
that has been referenced by @sbrannen, you can see a list of *Aware
interfaces, but it looks like this is the only one that is not resolvable.
ApplicationContext
does not implement StringValueResolver
, but ApplicationContextAwareProcessor
does process EmbeddedValueResolverAware
instances and inject an EmbeddedValueResolver
on them. This injected instance is built on top of a bean factory and provides a feature that is strictly bound to a bean factory and its bean expression resolver. So, I find it really really strange that such a component cannot be autowired and the fact that ApplicationContext
itself does not implement this interface is a weak argument IMHO.
I am not sure I got that but if there are special treatments for
ApplicationContext
,BeanFactory
, and the like, this is obviously something to do with SonarQube, and not a valid point to request this change.
There's no special treatment for ApplicationContext
and alike in SonarQube, they can be autowired. Currently, the only "special treatment" seems to be the one that Spring Framework is reserving to StringValueResolver
(or EmbeddedValueResolver
if you prefer).
If you want to read more, the complete description of the issue can be found here: https://rules.sonarsource.com/java/?search=Members%20of%20Spring%20components%20should%20be%20injected
Comment From: snicoll
but you have EmbeddedValueResolverAware, which promotes it as a special enough feature, IMHO.
I don't see why you would conclude that. The bottom line is that, while there is one ApplicationContext
, one Environment
, etc, it isn't the case for EmbeddedValueResolver
. It's simply not an important enough scenario for user components. And for infrastructure components, EmbeddedValueResolverAware is fine anyway. I understand that you disagree, I am just pointing out the fact that having an Aware
interface doesn't change anything.
If you want to read more, the complete description of the issue can be found here: https://rules.sonarsource.com/java/?search=Members%20of%20Spring%20components%20should%20be%20injected
This rule looks completely bogus to me.
Comment From: mauromol
I don't see why you would conclude that. The bottom line is that, while there is one
ApplicationContext
, oneEnvironment
, etc, it isn't the case forEmbeddedValueResolver
. It's simply not an important enough scenario for user components. And for infrastructure components, EmbeddedValueResolverAware is fine anyway. I understand that you disagree, I am just pointing out the fact that having anAware
interface doesn't change anything.
I understand your point of view: you don't like the fact that "string value resolver", as a name, may mean everything, as well as EmbeddedValueResolver
. I am not responsible of the names historically chosen by Spring Framework for them, or of the lack of a more specific interface that explains the feature we're talking about in a unambiguous and unique way. But it's a fact that through this interface and the EmbeddedValueResolverAware
injection technique you get a no doubt unique feature strictly bound to the bean factory, that is the ability to resolve configuration properties possibly containing placeholders (as references to other configuration properties). And there's one such resolver per application context, not multiple ones.
Currently, there's no way to autowire such a component from the Spring Framework. The only way to get this is to let your bean implement EmbeddedValueResolverAware
interface. And, as any other *Aware
interface, it would be natural to expect that this can also be autowired, even if its name is (unfortunately) not so special. This is the whole point of this issue.
I also think that the change would be so minimal that it wouldn't hurt the framework... but anyway, it's your decision.
The SonarQube thing is a plus. The rule might sound bogus to you, but if you look at the "how can I fix it?" section, you'll find a non-compliant example which is really dumb, but it explains the potential issue. After all, Spring Documentation itself tells you that you should avoid state in singleton beans. If you have an instance variable that is not annotated, it means that it can (potentially) keep state that is not part of the bean initialization. Of course, it depends on how you use it. But, as I said, preventing your code quality tool from flagging your code as potentially insecure (just because it does not know about how EmbeddedValueResolverAware
is handled) is just a side effect of the main issue here.