Copilot commented on code in PR #15950:
URL: https://github.com/apache/grails-core/pull/15950#discussion_r3555477245
##########
grails-web-databinding/src/main/groovy/grails/web/databinding/DataBindingUtils.java:
##########
@@ -300,6 +320,586 @@ public static BindingResult
bindObjectToDomainInstance(PersistentEntity entity,
return bindingResult;
}
+ private static void assignNullToMissingIncludedProperties(Object object,
DataBindingSource bindingSource, List include, List exclude, String filter) {
+ for (Object includedProperty : include) {
+ if (includedProperty instanceof CharSequence) {
+ String propertyName = includedProperty.toString();
+ if (propertyName.indexOf('*') == -1 &&
!isExcludedProperty(propertyName, exclude) && isBindingAllowed(object,
propertyName)) {
+ if (assignNullToMissingIndexedProperties(object,
bindingSource, propertyName, filter)) {
+ continue;
+ }
+ if (!bindingSourceContainsProperty(bindingSource,
propertyName, filter)) {
+ setPropertyToNull(object, propertyName);
+ }
+ }
+ }
+ }
+ }
+
+ private static boolean isExcludedProperty(String propertyName, List
exclude) {
+ if (exclude == null) {
+ return false;
+ }
+ for (Object excludedProperty : exclude) {
+ if (excludedProperty instanceof CharSequence) {
+ String excludedPropertyName = excludedProperty.toString();
+ if (propertyName.equals(excludedPropertyName) ||
propertyName.startsWith(excludedPropertyName + ".") ||
rootPropertyName(propertyName).equals(excludedPropertyName)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private static boolean isBindingAllowed(Object object, String
propertyName) {
+ if (object == null) {
+ return false;
+ }
+
+ if (!isPropertyAllowedByWhitelist(object, propertyName)) {
+ return false;
+ }
+
+ int separator = propertyPathSeparator(propertyName);
+ if (separator == -1) {
+ return true;
+ }
+
+ Object nestedObject = getPropertyValue(object,
propertyName.substring(0, separator));
+ String nestedPropertyName = propertyName.substring(separator + 1);
+ if (nestedObject instanceof Collection) {
+ for (Object item : (Collection) nestedObject) {
+ if (item != null && !isBindingAllowed(item,
nestedPropertyName)) {
+ return false;
+ }
+ }
+ return true;
+ }
+ if (nestedObject instanceof Map) {
+ for (Object value : ((Map) nestedObject).values()) {
+ if (value != null && !isBindingAllowed(value,
nestedPropertyName)) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return nestedObject == null || isBindingAllowed(nestedObject,
nestedPropertyName);
+ }
+
+ private static boolean isPropertyAllowedByWhitelist(Object object, String
propertyName) {
+ List bindingIncludeList = getBindingIncludeList(object);
+ if (bindingIncludeList == null || bindingIncludeList.isEmpty()) {
+ return true;
+ }
+ for (Object includedProperty : bindingIncludeList) {
+ if (includedProperty instanceof CharSequence) {
+ String includedPropertyName = includedProperty.toString();
+ if (propertyName.equals(includedPropertyName) ||
includedPropertyName.startsWith(propertyName + ".")) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
Review Comment:
`isPropertyAllowedByWhitelist` doesn't evaluate the
`$defaultDatabindingWhiteList` entries using the same wildcard semantics as the
binder. `DefaultASTDatabindingHelper` injects patterns like `address.*` and
`address_*` for non-simple properties, but the current check only does an exact
match (and a reverse prefix check), so nested paths like `address.country` will
often be treated as *not bindable* and skipped during `nullMissing` clearing.
It can also incorrectly allow a parent property when only a nested property is
whitelisted.
Consider matching the whitelist entries as simple patterns (same as
Spring/Grails data binding) so `nullMissing` respects bindable constraints
consistently.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
