From 5cbc384fb2514546e4fc1f55b4516b381d2ae194 Mon Sep 17 00:00:00 2001
From: Tatu Lund <tatu@vaadin.com>
Date: Wed, 27 Oct 2021 20:14:29 +0300
Subject: [PATCH] fix: avoid ConcurrentModificationException in Binder (#12458)

---
 .../src/main/java/com/vaadin/data/Binder.java | 21 +++++++++++------
 .../test/java/com/vaadin/data/BinderTest.java | 23 +++++++++++++++++++
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index 9726f54345..6a4afdb2f6 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -2001,7 +2001,7 @@ public class Binder<BEAN> implements Serializable {
      *         updated, {@code false} otherwise
      */
     public boolean writeBeanIfValid(BEAN bean) {
-        return doWriteIfValid(bean, new ArrayList<>(bindings)).isOk();
+        return doWriteIfValid(bean, bindings).isOk();
     }
 
     /**
@@ -2024,19 +2024,26 @@ public class Binder<BEAN> implements Serializable {
         Objects.requireNonNull(bean, "bean cannot be null");
         List<ValidationResult> binderResults = Collections.emptyList();
 
+        // make a copy of the incoming bindings to avoid their modifications
+        // during validation
+        Collection<Binding<BEAN, ?>> currentBindings = new ArrayList<>(
+                bindings);        
+
         // First run fields level validation, if no validation errors then
         // update bean
-        List<BindingValidationStatus<?>> bindingResults = bindings.stream()
-                .map(b -> b.validate(false)).collect(Collectors.toList());
+        List<BindingValidationStatus<?>> bindingResults = currentBindings
+                .stream().map(b -> b.validate(false))
+                .collect(Collectors.toList());
 
         if (bindingResults.stream()
                 .noneMatch(BindingValidationStatus::isError)) {
             // Store old bean values so we can restore them if validators fail
             Map<Binding<BEAN, ?>, Object> oldValues = getBeanState(bean,
-                    bindings);
+                    currentBindings);
 
-            bindings.forEach(binding -> ((BindingImpl<BEAN, ?, ?>) binding)
-                    .writeFieldValue(bean));
+            currentBindings
+                    .forEach(binding -> ((BindingImpl<BEAN, ?, ?>) binding)
+                            .writeFieldValue(bean));
             // Now run bean level validation against the updated bean
             binderResults = validateBean(bean);
             if (binderResults.stream().anyMatch(ValidationResult::isError)) {
@@ -2047,7 +2054,7 @@ public class Binder<BEAN> implements Serializable {
                  * Changes have been successfully saved. The set is only cleared
                  * when the changes are stored in the currently set bean.
                  */
-                bindings.clear();
+                changedBindings.clear();
             } else if (getBean() == null) {
                 /*
                  * When using readBean and writeBean there is no knowledge of
diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java
index 9f20f0b246..999142a440 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderTest.java
@@ -1542,6 +1542,29 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
     }
 
     @Test
+    public void invalidUsage_modifyFieldsInsideValidator_binderDoesNotThrow() {
+        TextField field = new TextField();
+
+        AtomicBoolean validatorIsExecuted = new AtomicBoolean();
+        binder.forField(field).asRequired().withValidator((val, context) -> {
+            nameField.setValue("foo");
+            ageField.setValue("bar");
+            validatorIsExecuted.set(true);
+            return ValidationResult.ok();
+        }).bind(Person::getEmail, Person::setEmail);
+
+        binder.forField(nameField).bind(Person::getFirstName,
+                Person::setFirstName);
+        binder.forField(ageField).bind(Person::getLastName,
+                Person::setLastName);
+
+        binder.setBean(new Person());
+
+        field.setValue("baz");
+        // mostly self control, the main check is: not exception is thrown
+        assertTrue(validatorIsExecuted.get());
+    }    
+
     public void setBean_readOnlyBinding_propertyBinding_valueIsNotUpdated() {
         Binder<ExampleBean> binder = new Binder<>(ExampleBean.class);
 
-- 
2.39.5