Introduce @FromModel as substitute to @ModelAttribute(binding=false) [SPR-15267]

[spring-projects-issues]

Jochen Pier opened SPR-15267 and commented

Dear Spring-Team,

we often use @ModelAttribute to fetch Model-Attributes from the model in controller actions.

Example:

@RequestMapping("/testAtrribute")
public @ResponseBody String action(@ModelAttribute(name = "attr", binding=false) String attr) {
     return "Received from Model:" + attr;
}

The problem: If we forget the "binding=false" attribute, we get a really heavy security risk. Additionally, I think that getting something from the existing Model (which was constructed from former processing) or binding data to request are very different things, that should get different names.

So I suggest to introduce a new name and deprecate the "binding=false" attribute.
Name-suggestion: @FromModel(name="xyz")

Example:

@RequestMapping("/testAtrribute")
public @ResponseBody String action(@FromModel(name = "attr") String attr) {
     return "Received from Model:" + attr;
}

And maybe another suggestion:
Rename the parameter-binding annotation too.
So

@RequestMapping("/testAtrribute")
public @ResponseBody String action(@FromRequest("Book") Book book) {
     return "Received from Request:" + book.getTitle();
}

Thank you for spring!

---

Issue Links:

• Prevent binding for @ModelAttribute [SPR-13402] #17982 Prevent binding for @ModelAttribute

[spring-projects-issues]

Rossen Stoyanchev commented

I agree model attribute access and data binding are separate concerns. The main purpose of @ModelAttribute however is to bind and validate to a command object. Its use for access to model attributes has always been secondary, supported but not really its main purpose.

For good or bad it will not be helpful to change this mental model at this stage. Especially for those for whom it isn't and hasn't been an issue since Spring Framework 2.5! It would be difficult to justify any such change no matter how logical. I would prefer to seek an alternative way to address the underlying reasons.

For example could we consider Model API enhancements to make it easier to access attributes with downcasting and support for Optional? Effectively trading a @ModelAttribute declaration for Model model + local variable initialization. I realize that mileage varies by use case but I don't think it's to find such short and sweet type names and the same follows for variable names, and the potential need for an explicit annotation name attribute. There could be more than one model attributes too.

Alternatively some additional annotation. We did add request and session access-related attributes. Perhaps an @Attribute that searches in order model, request, and session attribute.

[spring-projects-issues]

Jochen Pier commented

I personally like very much the possibility to get an object out of the model (or request-scope, or session...) directly as parameter in a controller action.
This makes the code very good testable.

But I understand that it would not help to erase an Annotation, that is present since 2.5!

So I still think it would be the best way to create new Annotations (in addition to the existing ones) for this purpose. You just introduced for example the @GetMethod annotation as a shortcut, so why not another shortcut annotation?
The name @Attribute is in my opinion too "general". And your suggestion that it searches in model->request->session is not nessessary (for at least me). In my projects, I always knew where my objects were stored (either model, request, session). (What can be the usecase for such a search-hierarchy?)
Maybe @AttributeFromModel or something?

I did not understand what you mean with "Model API enhancements" - but it sounds interesting :)

[spring-projects-issues]

Jochen Pier commented

I discovered this: HandlerMethodArgumentResolver
This is even more elegant for us - so if you want, you can close this. I'm sure, you have enough on your backlog.

[spring-projects-issues]

Arjen Poutsma commented

Resolving as requested.