Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fb jpms - fixes for illegal access to private information in java.* packages. #1328

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4fd4d01
adding Netbeans artifacts
struberg Dec 2, 2024
3d64ffb
LANG-1685 fix upToClass test
struberg Dec 2, 2024
68a8c9c
LANG-1685 improve ToStringBuilder for Java9++
struberg Dec 3, 2024
9fd537a
LANG-1758 improve CompareToBuilder
struberg Dec 4, 2024
edb8ba8
LANG-1265 LANG-1667 fix unallowed private access
struberg Dec 4, 2024
6733551
LANG-1265 fix checkstyle issues
struberg Dec 4, 2024
0a41e5c
LANG-1265 pull the logic into central helper methods
struberg Dec 4, 2024
0bf6ceb
LANG-1265 unify to TemporalAccessor
struberg Dec 4, 2024
bb7b412
LANG-1265 Character is also a java native type
struberg Dec 5, 2024
e353c3e
LANG-1265 add EqualsBuilder support for Collections and Maps.
struberg Dec 5, 2024
d3c8c9f
LANG-1265 improve Map handling
struberg Dec 5, 2024
60fd6cb
remove nb-configuration.xml from rat excludes
struberg Dec 5, 2024
f20639a
LANG-1265 move to concrete types
struberg Dec 5, 2024
c64d702
Merge branch 'master' into fb_jpms
struberg Dec 5, 2024
ec4150b
LANG-1265 add Duration as additional non-reflection type
struberg Dec 5, 2024
08d913e
LANG-1265 adding Period as non reflexible class
struberg Dec 6, 2024
a775d16
Merge remote-tracking branch 'origin/master' into fb_jpms
struberg Dec 6, 2024
d951d0e
LANG-1265 trying to solve inaccessibility in a generic way
struberg Dec 6, 2024
0d628cd
LANG-1265 more work on inaccessible classes
struberg Dec 7, 2024
0e916b8
LANG-1265 rename to isNonIntrospectibleClass
struberg Dec 7, 2024
499a0a9
Merge branch 'master' into fb_jpms
struberg Dec 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ site-content

# jenv's version file
.java-version

# Netbeans
nb-configuration.xml
5 changes: 2 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@
<exclude>src/site/resources/download_lang.cgi</exclude>
<exclude>src/site/resources/release-notes/RELEASE-NOTES-*.txt</exclude>
<exclude>src/test/resources/lang-708-input.txt</exclude>
<exclude>nb-configuration.xml</exclude>
struberg marked this conversation as resolved.
Show resolved Hide resolved
</excludes>
</configuration>
</plugin>
Expand Down Expand Up @@ -458,9 +459,7 @@
<jdk>[9,)</jdk>
</activation>
<properties>
<!-- LANG-1265: allow tests to access private fields/methods of java.base classes via reflection -->
<!-- LANG-1667: allow tests to access private fields/methods of java.base/java.util such as ArrayList via reflection -->
<argLine>-Xmx512m --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED</argLine>
<argLine>-Xmx512m</argLine>
</properties>
</profile>
<profile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ private static void reflectionAppend(
final boolean useTransients,
final String[] excludeFields) {

if (Reflection.isJavaInternalClass(lhs) && lhs instanceof Comparable) {
builder.append(lhs, rhs);
return;
}

struberg marked this conversation as resolved.
Show resolved Hide resolved
final Field[] fields = clazz.getDeclaredFields();
AccessibleObject.setAccessible(fields, true);
for (int i = 0; i < fields.length && builder.comparison == 0; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ public EqualsBuilder append(final Object lhs, final Object rhs) {
// to be inlined
appendArray(lhs, rhs);
} else // The simple case, not an array, just test the element
if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass)) {
if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass) && !Reflection.isJavaInternalClass(lhsClass)) {
reflectionAppend(lhs, rhs);
} else {
isEquals = lhs.equals(rhs);
Expand Down Expand Up @@ -956,7 +956,7 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) {
}

try {
if (testClass.isArray()) {
if (testClass.isArray() || Reflection.isJavaInternalClass(testClass)) {
append(lhs, rhs);
} else //If either class is being excluded, call normal object equals method on lhsClass.
if (bypassReflectionClasses != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ private static void reflectionAppend(final Object object, final Class<?> clazz,
if (isRegistered(object)) {
return;
}
if (Reflection.isJavaInternalClass(object)) {
builder.append(object);
return;
}
try {
register(object);
// The elements in the returned array are not sorted and are not in any particular order.
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/org/apache/commons/lang3/builder/Reflection.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.commons.lang3.builder;

import java.lang.reflect.Field;
import java.time.temporal.Temporal;
import java.util.Objects;

/**
Expand All @@ -41,4 +42,24 @@ static Object getUnchecked(final Field field, final Object obj) {
}
}

/**
* Check whether the given object is of a type which is an internal java Class, like String, Integer, Boolean, LocalDateTime, etc
* This is often needed as we cannot look into them via reflection since Java9.
*
* @return {@code true} if the given object is a Java internal class
*/
static boolean isJavaInternalClass(Object o) {
return o != null && isJavaInternalClass(o.getClass());
}

/**
* Check whether the given class is an internal java Class, like String, Integer, Boolean, LocalDateTime, etc
* This is often needed as we cannot look into them via reflection since Java9.
*
* @return {@code true} if the given class is a Java internal class
*/
static boolean isJavaInternalClass(Class<?> clazz) {
return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Temporal.class.isAssignableFrom(clazz);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.time.temporal.TemporalAccessor;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Map;
import java.util.Objects;

import org.apache.commons.lang3.ArraySorter;
Expand Down Expand Up @@ -848,8 +850,13 @@ public String toString() {

validate();

if (handleNativeClasses()) {
return super.toString();
}

Class<?> clazz = getObject().getClass();
appendFieldsIn(clazz);

while (clazz.getSuperclass() != null && clazz != getUpToClass()) {
clazz = clazz.getSuperclass();
appendFieldsIn(clazz);
Expand All @@ -867,4 +874,21 @@ private void validate() {
}
}

private boolean handleNativeClasses() {
Object value = getObject();
if (value instanceof Number || value instanceof Boolean || value instanceof Character
|| value instanceof TemporalAccessor) {
struberg marked this conversation as resolved.
Show resolved Hide resolved
getStringBuffer().append("value=").append(getObject().toString());
return true;
}

if (value.getClass().isArray() || value instanceof Collection || value instanceof Map) {
struberg marked this conversation as resolved.
Show resolved Hide resolved
ToStringStyle.unregister(value);
getStyle().append(getStringBuffer(), null, value, true);
return true;
}

return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;

Expand Down Expand Up @@ -229,21 +230,33 @@ public void assertReflectionArray(final String expected, final Object actual) {
*/
@Test
public void test_setUpToClass_invalid() {
final Integer val = Integer.valueOf(5);
final HirAFixture val = new HirAFixture();
final ReflectionToStringBuilder test = new ReflectionToStringBuilder(val);
assertThrows(IllegalArgumentException.class, () -> test.setUpToClass(String.class));
test.toString();
}

private static class HirAFixture extends HirBFixture {
int x = 1;
}

private static class HirBFixture extends HirCFixture {
int y = 2;
}

private static class HirCFixture {
struberg marked this conversation as resolved.
Show resolved Hide resolved
int z = 3;
}

/**
* Tests ReflectionToStringBuilder setUpToClass().
*/
@Test
public void test_setUpToClass_valid() {
final Integer val = Integer.valueOf(5);
final HirAFixture val = new HirAFixture();
final ReflectionToStringBuilder test = new ReflectionToStringBuilder(val);
test.setUpToClass(Number.class);
struberg marked this conversation as resolved.
Show resolved Hide resolved
test.toString();
test.setUpToClass(HirBFixture.class);
assertEquals(val.toString() + "[x=1,y=2]", test.toString());
}

@Test
Expand Down Expand Up @@ -752,6 +765,12 @@ public void testObjectArray() {
assertEquals(baseStr + "[<null>]", new ToStringBuilder(base).append((Object) array).toString());
}

@Test
public void testArrayList() {
List<Integer> list = Arrays.asList(23, 12, 39);
assertEquals(baseStr + "[[23, 12, 39]]", new ToStringBuilder(base).append(list).toString());
}

@Test
public void testObjectBuild() {
final Integer i3 = Integer.valueOf(3);
Expand Down Expand Up @@ -972,17 +991,14 @@ public void testReflectionHierarchyArrayList() {
// LANG-1337 without this, the generated string can differ depending on the JVM version/vendor
final List<Object> list = new ArrayList<>(ARRAYLIST_INITIAL_CAPACITY);
final String baseString = toBaseString(list);
final String expectedWithTransients = baseString
+ "[elementData={<null>,<null>,<null>,<null>,<null>,<null>,<null>,<null>,<null>,<null>},size=0,modCount=0]";
final String expected = baseString
+ "[[]]";
final String toStringWithTransients = ToStringBuilder.reflectionToString(list, null, true);
if (!expectedWithTransients.equals(toStringWithTransients)) {
assertEquals(expectedWithTransients, toStringWithTransients);
}
final String expectedWithoutTransients = baseString + "[size=0]";
assertEquals(expected, toStringWithTransients);

// no difference as Collections and Maps are not handled via reflection anymore
final String toStringWithoutTransients = ToStringBuilder.reflectionToString(list, null, false);
if (!expectedWithoutTransients.equals(toStringWithoutTransients)) {
assertEquals(expectedWithoutTransients, toStringWithoutTransients);
}
assertEquals(expected, toStringWithoutTransients);
}

@Test
Expand Down
Loading