kusalk commented on code in PR #780:
URL: https://github.com/apache/struts/pull/780#discussion_r1382538745
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
Review Comment:
Wanted to be a bit more deliberate about how we handle the arguments in case
OGNL behaviour changes
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
Review Comment:
Throw an exception here because if this ceases to be the case, it means the
following logic may need changes
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
Review Comment:
Extract this into its own protected method for consistency
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]",
target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!",
member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!",
targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null ||
targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!",
targetClass);
- return false;
- }
- if (memberClass.getPackage() == null ||
memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!",
memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
Review Comment:
Extracted all the exclusion list checks into this method
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
Review Comment:
Tried to standardise around just passing the object and/or member to these
protected methods
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
Review Comment:
This is essentially an exemption to `checkStaticMethodAccess` so I moved it
within that method - we shouldn't be relying on method call order here
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
Review Comment:
Removed redundant argument
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]",
target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!",
member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!",
targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null ||
targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!",
targetClass);
- return false;
- }
- if (memberClass.getPackage() == null ||
memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!",
memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
+ Class<?> memberClass = member.getDeclaringClass();
+ if (isClassExcluded(memberClass)) {
+ LOG.warn("Declaring class of member type [{}] is excluded!",
memberClass);
+ return false;
+ }
+ if (isPackageExcluded(memberClass)) {
LOG.warn("Package [{}] of target class [{}] of target [{}] is
excluded!",
- targetClass.getPackage(),
- targetClass,
+ memberClass.getPackage(),
+ memberClass,
target);
return false;
}
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (isClassExcluded(targetClass)) {
+ LOG.warn("Target class [{}] of target [{}] is excluded!",
targetClass, target);
+ return false;
+ }
+ if (isPackageExcluded(targetClass)) {
+ LOG.warn("Package [{}] of member [{}] are excluded!",
targetClass.getPackage(), member);
+ return false;
+ }
+ return true;
+ }
- if (targetClass != memberClass && isPackageExcluded(memberClass)) {
- LOG.warn("Package [{}] of member [{}] are excluded!",
memberClass.getPackage(), member);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkDefaultPackageAccess(Object target, Member member) {
+ if (!disallowDefaultPackageAccess) {
+ return true;
+ }
+ Class<?> memberClass = member.getDeclaringClass();
+ if (memberClass.getPackage() == null ||
memberClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!",
memberClass);
+ return false;
+ }
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (targetClass.getPackage() == null ||
targetClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!",
targetClass);
return false;
}
+ return true;
+ }
- return isAcceptableProperty(propertyName);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkProxyMemberAccess(Object target, Member member) {
+ return !(disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target));
}
/**
* Check access for static method (via modifiers).
- *
+ * <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- *
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticMethodAccess(Member member, int
memberModifiers) {
- return !Modifier.isStatic(memberModifiers) || member instanceof Field;
+ protected boolean checkStaticMethodAccess(Member member) {
+ if (checkEnumAccess(member)) {
+ LOG.trace("Exempting Enum#values from static method check: class
[{}]", member.getDeclaringClass());
+ return true;
+ }
+ return member instanceof Field || !isStatic(member);
+ }
+
+ private static boolean isStatic(Member member) {
+ return Modifier.isStatic(member.getModifiers());
}
/**
* Check access for static field (via modifiers).
* <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticFieldAccess(Member member, int
memberModifiers) {
- if (Modifier.isStatic(memberModifiers) && member instanceof Field) {
- return allowStaticFieldAccess;
- } else {
+ protected boolean checkStaticFieldAccess(Member member) {
+ if (allowStaticFieldAccess) {
Review Comment:
Bypass computation if static field access is permitted
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
Review Comment:
Set to null as there is no more useful information to extract here, and it
simplifies the checks/logic to follow
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]",
target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!",
member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!",
targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
Review Comment:
Extracted this into its own method
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member
member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member,
String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property:
{}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL
3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ?
memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match
member!");
+ if (target instanceof Class) { // Target may be of type Class for
static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not
match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null &&
!member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match
member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target
[{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target
[{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]",
target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!",
member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!",
targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null ||
targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!",
targetClass);
- return false;
- }
- if (memberClass.getPackage() == null ||
memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!",
memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
+ Class<?> memberClass = member.getDeclaringClass();
+ if (isClassExcluded(memberClass)) {
+ LOG.warn("Declaring class of member type [{}] is excluded!",
memberClass);
+ return false;
+ }
+ if (isPackageExcluded(memberClass)) {
LOG.warn("Package [{}] of target class [{}] of target [{}] is
excluded!",
- targetClass.getPackage(),
- targetClass,
+ memberClass.getPackage(),
+ memberClass,
target);
return false;
}
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (isClassExcluded(targetClass)) {
+ LOG.warn("Target class [{}] of target [{}] is excluded!",
targetClass, target);
+ return false;
+ }
+ if (isPackageExcluded(targetClass)) {
+ LOG.warn("Package [{}] of member [{}] are excluded!",
targetClass.getPackage(), member);
+ return false;
+ }
+ return true;
+ }
- if (targetClass != memberClass && isPackageExcluded(memberClass)) {
- LOG.warn("Package [{}] of member [{}] are excluded!",
memberClass.getPackage(), member);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkDefaultPackageAccess(Object target, Member member) {
+ if (!disallowDefaultPackageAccess) {
+ return true;
+ }
+ Class<?> memberClass = member.getDeclaringClass();
+ if (memberClass.getPackage() == null ||
memberClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!",
memberClass);
+ return false;
+ }
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (targetClass.getPackage() == null ||
targetClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!",
targetClass);
return false;
}
+ return true;
+ }
- return isAcceptableProperty(propertyName);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkProxyMemberAccess(Object target, Member member) {
+ return !(disallowProxyMemberAccess && ProxyUtil.isProxyMember(member,
target));
}
/**
* Check access for static method (via modifiers).
- *
+ * <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- *
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticMethodAccess(Member member, int
memberModifiers) {
- return !Modifier.isStatic(memberModifiers) || member instanceof Field;
+ protected boolean checkStaticMethodAccess(Member member) {
+ if (checkEnumAccess(member)) {
+ LOG.trace("Exempting Enum#values from static method check: class
[{}]", member.getDeclaringClass());
+ return true;
+ }
+ return member instanceof Field || !isStatic(member);
+ }
+
+ private static boolean isStatic(Member member) {
+ return Modifier.isStatic(member.getModifiers());
}
/**
* Check access for static field (via modifiers).
* <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticFieldAccess(Member member, int
memberModifiers) {
- if (Modifier.isStatic(memberModifiers) && member instanceof Field) {
- return allowStaticFieldAccess;
- } else {
+ protected boolean checkStaticFieldAccess(Member member) {
+ if (allowStaticFieldAccess) {
return true;
}
+ return !(member instanceof Field) || !isStatic(member);
}
/**
* Check access for public members (via modifiers)
- * <p>
- * Returns true if-and-only-if the member is public.
*
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkPublicMemberAccess(int memberModifiers) {
- return Modifier.isPublic(memberModifiers);
+ protected boolean checkPublicMemberAccess(Member member) {
+ return Modifier.isPublic(member.getModifiers());
}
- protected boolean checkEnumAccess(Object target, Member member) {
- if (target instanceof Class) {
- final Class<?> clazz = (Class<?>) target;
- return Enum.class.isAssignableFrom(clazz) &&
member.getName().equals("values");
- }
- return false;
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkEnumAccess(Member member) {
+ return member.getDeclaringClass().isEnum()
+ && isStatic(member)
+ && member instanceof Method
+ && member.getName().equals("values")
+ && ((Method) member).getParameterCount() == 0;
Review Comment:
Closed a minor hole where another static `values` method could also be
exempted
--
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: issues-unsubscr...@struts.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
