spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.66k stars 38.14k forks source link

th:hasErrors() creates model attributes if not present (side-effect) [SPR-17567] #22099

Closed spring-projects-issues closed 1 year ago

spring-projects-issues commented 5 years ago

Nicolai Ehemann opened SPR-17567 and commented

I'm not 100% sure this is a problem of spring core or perhaps thymeleaf (due to my lack of knowledge of internals), but I suspect the problem is actually with spring:

Calling th:hasErrors() in a thymeleaf template can have the potentially unwanted side-effect of creating model attributes while evaluating the thymeleaf template.

One way I found to reproduce this is using uninitialized nested model attributes. If I have a model attribute that has another object as a field, and that field is not initialized per construction, it will be initialized upon calling th:hasErrors() on it.

For example, I have a model attribute person with a field address which in turn has a field street. Consider the following thymeleaf snippet:

<div th:object="${person}>
  <span th:text="*{address}"/><!-- adress is null -->
  <span th:text="${#fields.hasErrors('address.street')}"/>
  <span th:text="*{address}"/><!-- adress is now an object -->
</div>

I don't think hasErrors() should have such unexpected side effects.

I made a minimal modification to the spring example project gs-validating-form-input to demonstrate/reproduce the issue: https://github.com/McNetic/gs-validating-form-input/commit/55a265fa0b8a2df4f60af211699d43633319fd88  

(I know the examples are not very meaningful)


Affects: 5.1.2

Reference URL: https://github.com/McNetic/gs-validating-form-input/commit/55a265fa0b8a2df4f60af211699d43633319fd88

snicoll commented 1 year ago

@McNetic sorry for overlooking this. I've ran your sample and looking at the code I don't understand how a problem with th:hasErrors() is reported here. Each attempt at submitting the form creates a PersonForm so it makes sense that the address is changing. Can you clarify what you think is a Spring issue?

McNetic commented 1 year ago

I think I reported this because of the fact that an ostensibly ready-only method (namely hasErrors()) does in fact change state and create objects. This is not about submitting the form, but about template evaluation. If the template relies on the fact that the nested property is null in some cases and not null in others, it leads to hard to find errors.

I have to say after 5 years, I do not have any recollection of how this affected our production code, but I am quite sure that exactly that happend; otherwise, I would not have found about the issue...

snicoll commented 1 year ago

Thanks for following up.

I think I reported this because of the fact that an ostensibly ready-only method (namely hasErrors()) does in fact change state and create objects

We're back on understanding why you believe this would be a Spring Framework issue. That code executes a SpEL expression #fields.hasErrors('name') using org.thymeleaf.spring5.expression.ThymeleafEvaluationContext for the context. This ultimately calls org.thymeleaf.spring5.expression.Fields. Please report this issue against the Thymeleaf project.