jamesfredley commented on code in PR #15947:
URL: https://github.com/apache/grails-core/pull/15947#discussion_r3599450186
##########
grails-web-databinding/src/main/groovy/grails/web/databinding/DataBindingUtils.java:
##########
@@ -119,13 +120,19 @@ public static BindingResult bindObjectToInstance(Object
object, Object source) {
}
protected static List getBindingIncludeList(final Object object) {
- List includeList = Collections.emptyList();
+ final boolean legacyBindableDefaultEnabled =
isLegacyBindableDefaultEnabled();
+ final String whiteListFieldName = legacyBindableDefaultEnabled ?
+ DefaultASTDatabindingHelper.LEGACY_DATABINDING_WHITELIST :
+ DefaultASTDatabindingHelper.DEFAULT_DATABINDING_WHITELIST;
+ final Map<Class, List> includeListCache = legacyBindableDefaultEnabled
?
+ CLASS_TO_LEGACY_BINDING_INCLUDE_LIST :
CLASS_TO_BINDING_INCLUDE_LIST;
+ List includeList = legacyBindableDefaultEnabled ?
Collections.emptyList() :
Collections.singletonList(DefaultASTDatabindingHelper.NO_BINDABLE_PROPERTIES);
try {
final Class<? extends Object> objectClass = object.getClass();
- if (CLASS_TO_BINDING_INCLUDE_LIST.containsKey(objectClass)) {
- includeList = CLASS_TO_BINDING_INCLUDE_LIST.get(objectClass);
+ if (includeListCache.containsKey(objectClass)) {
+ includeList = includeListCache.get(objectClass);
} else {
- final Field whiteListField =
objectClass.getDeclaredField(DefaultASTDatabindingHelper.DEFAULT_DATABINDING_WHITELIST);
+ final Field whiteListField =
objectClass.getDeclaredField(whiteListFieldName);
Review Comment:
Addressed. `getBindingIncludeList` no longer uses `getDeclaredField` on the
target class alone. It now resolves the injected whitelist fields by walking
the class hierarchy (a `getField`/`getPublicDeclaredField` helper), so an
inherited `public static` allowlist field is correctly found for
CGLIB/ByteBuddy/Hibernate proxy subclasses and ordinary subclasses (the
deny-by-default binding no longer collapses to bind-nothing for proxied
instances).
For security it also does more than a plain inherited lookup: it trusts a
`$defaultDatabindingWhiteList` value only when it is paired with a same-class
`$legacyDatabindingWhiteList`, so a mixed-generation hierarchy (an old
precompiled subclass declaring only the broad default field, extending a Grails
8 parent) cannot have its old permissive whitelist mistaken for a secure
generated allowlist. Both the proxy-subclass and mixed-generation paths are
covered by new regression tests.
--
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]
