Servlets should not have mutable instance fields false positive with Spring autowire (squid:S2226)

Question

I have a Servlet that have Spring auto-wiring capabilities using:

WebApplicationContextUtils.getRequiredWebApplicationContext(config.getServletContext())
                    .getAutowireCapableBeanFactory().autowireBean(this);

When I'm autowiring my beans SonarQube warns with `Servlets should not have mutable instance fields (squid:S2226)

@Autowired
MyBean myBean;

Is it SonarQube bug which ignores Spring autowiring? Can I add different annotation to prevent this warning? am I missing something?

By contract, a servlet container creates one instance of each servlet and then a dedicated thread is attached to each new incoming HTTP request to process the request. So all threads share the servlet instances and by extension their instance fields. To prevent any misunderstanding and unexpected behavior at runtime, all servlet fields should then be either static and/or final, or simply removed.

EDIT

I found similar issue that was fixed about Java's @Inject that should not raise this warning

In practice when a field is annotated with @Inject it won't be mutated. So fields annotated with this annotation should not raise issue RSPEC-2226.

EDIT 2

Open a potential false positive issue in SonarSource Community

https://docs.sonarqube.org/display/SONAR/Frequently+Asked+Questions#FrequentlyAskedQuestions-HowtoremoveFalse-Positiveissues? — JB Nizet, Sep 17 '18 at 11:38
It's not an issue per se. You asked Sonar to warn you when using mutable fields on servlets, and you're using mutable fields on servlets, so it warns you. Since you're smarter than Sonar, you can carefully analyze the situation and conclude that, in that precise case, it's OK to violate the rule. So you can tell Sonar to avoid bugging you again for these fields. — JB Nizet, Sep 17 '18 at 11:45
@JBNizet I found similar issue about `@Inject` that was fixed https://jira.sonarsource.com/browse/SONARJAVA-1550?jql=text%20~%20%22S2226%22 — Ori Marko, Sep 17 '18 at 11:49
I wonder if you add final to MyBean and use Autowire on the constructor instead of on the field will it still complain — Alexander Petrov, Sep 17 '18 at 13:07
@AlexandarPetrov adding final and Autowire on setter method still warns the same. How can I use constructor injection in Servlet? — Ori Marko, Sep 17 '18 at 13:10
@user7294900 not on setter, you need to add it on Constructor. You can't have a final field that is not initialized. You should not be able to compile it at all this way. — Alexander Petrov, Sep 17 '18 at 13:11
@AlexandarPetrov can you give an answer with example of your suggestion? is this what you mean https://stackoverflow.com/questions/18745770/spring-injection-into-servlet ? — Ori Marko, Sep 17 '18 at 13:15

Alexander Petrov · Answer 1 · 2018-09-17T13:34:13.120

1

In my opinion you should drop the AutowireCapableBeanFactory completely. Instead you shouold register your servlet like this

 @Bean
 public ServletRegistrationBean yourServlet(MyBean mybean) {
      // initialize your Servlet here  = new YourServlet(myBean);
      //register it to as ServletRegistrationBean
    return servletRegistration;
 }

This registration presumes you already have your MyBean in the spring application context.

edited Sep 17 '18 at 13:34

answered Sep 17 '18 at 13:27

Alexander Petrov

9,204
31
70

So Spring will manage my Servlet? Maybe I should change to full Spring control as `RestController` instead? what about SonarQube warning ? – Ori Marko Sep 17 '18 at 13:35
@user7294900 that I can not give you answer to. But I can assure you that this is one of the most standard ways to register a servlet in spring. – Alexander Petrov Sep 17 '18 at 13:36
Once you pass your MyBean through the constructor and mark it as final. I believe from the point of SonarQube it will look immutable. – Alexander Petrov Sep 17 '18 at 13:53

score 1 · Accepted Answer · answered Sep 18 '18 at 11:32

I believe you are right, this is false-positive, issue should not be raised on this field same way as it is not raised on fields annotated with @Inject, @Resource or @EJB.

I create an issue to fix this behavior https://jira.sonarsource.com/browse/SONARJAVA-2895

Servlets should not have mutable instance fields false positive with Spring autowire (squid:S2226)

2 Answers2