False positive with java:S6856 when using a @ModelAttribute method with @PathVariable

After upgrading to SonarQube 10.4, I have a false positive on java:S6856 when using a @ModelAttribute to store a @PathVariable resolved item for all @RequestMapping methods in the controller.

Here is a reproducer :

  @ModelAttribute("viewCfg")
  public ViewConfig getViewConfiguration(@PathVariable("view") final String viewName)
  {
    return viewConfigService.resolveByName(viewName);
  }

  @GetMapping("/{view}") // FP here : "Bind path variable "view" to a method parameter"
  public String list(@ModelAttribute("viewCfg") final ViewConfig viewConfig)
  {
    return "";
  }

Hey @alec,
Thanks again for pushing the new rule in its corner :joy:
Unfortunately for this case, I am unable to reproduce the issue with the snippet you shared. In addition, from reading the implementation, we explicitly avoid raising on path variables that rely on the annotation @ModelAttribute for the value to be injected.

But it happens that when covering different annotation set we failed to collect some. A few questions that might help us sort this out:

  1. Which version of Spring are you using?
  2. Is @org.springframework.web.bind.annotation.ModelAttribute the annotation you are referring to?

Best,

Dorian

Sorry, I simplified it a bit too much and forgot there is inheritance involved.

class A {
  @ModelAttribute("viewCfg")
  public ViewConfig getViewConfiguration(@PathVariable("view") final String viewName) {
    return viewConfigService.resolveByName(viewName);
  }
}

class B extends A {
  @GetMapping("/{view}") // FP here : "Bind path variable "view" to a method parameter"
  public String list(@ModelAttribute("viewCfg") final ViewConfig viewConfig) {
    return "";
  }
}

I suppose the rule is only checking for ModelAttributes in the current class and not in the class hierarchy.

  1. Spring framework/mvc version 6.1.3
  2. I’m effectively using this annotation.

I also suppose the rule can have the same problem with a ModelAttribute populated through a ControllerAdvice but this case seems harder than the inheritance one and a rare use case.

Hi @alec,
Thank you for coming back to this topic with an updated example. I can now reproduce the issue with the inheritance.
I created a ticket to track the issue. Let me know if you see missing information there that could be helpful when we start working on a fix.

Cheers,

Dorian

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.