Skip to content

Commit 53b604b

Browse files
jpbempeljpbempel
authored
Avoid exception when capturing fields in jdk16+ (#7774)
Since jdk16, calling setAccessible(true) on fields that are on another java module that is not open/exported will result with InaccessibleObjectException thrown each time. To avoid this exception we can call `trySetAccessible` that will return true if we can call setAccessible without exception. The method `trySetAccessible` is only available since jdk9 so we need to use MethodHandle on it to be able to call it only when it is available. NB: setAccessible for jdk8 will always work anyway. InaccessibleObjectException provides a useful message for the cause of this error and we have added back a similar message by getting the module where resides the field that is not accessible to explain to the user why the field is not captured what it can do to resolve the problem. Fix also the report of evaluation errors for log template for inaccessible fields inside an object (marked UNDEFINED)
1 parent 00856e0 commit 53b604b

File tree

10 files changed

+193
-123
lines changed

10 files changed

+193
-123
lines changed

‎dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/Fields.java‎

Lines changed: 0 additions & 78 deletions
This file was deleted.

‎dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/el/ReflectiveFieldValueResolver.java‎

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,70 @@
11
package datadog.trace.bootstrap.debugger.el;
22

3+
import static java.lang.invoke.MethodType.methodType;
4+
35
import datadog.trace.bootstrap.debugger.CapturedContext;
6+
import java.lang.invoke.MethodHandle;
7+
import java.lang.invoke.MethodHandles;
48
import java.lang.reflect.Field;
59
import java.lang.reflect.Modifier;
10+
import org.slf4j.Logger;
11+
import org.slf4j.LoggerFactory;
612

713
/** A helper class to resolve a reference path using reflection. */
814
public class ReflectiveFieldValueResolver {
15+
private static final Logger LOGGER = LoggerFactory.getLogger(ReflectiveFieldValueResolver.class);
16+
// This is a workaround for the fact that Field.trySetAccessible is not available in Java 8
17+
private static final MethodHandle TRY_SET_ACCESSIBLE;
18+
19+
static {
20+
MethodHandle methodHandle = null;
21+
try {
22+
MethodHandles.Lookup lookup = MethodHandles.lookup();
23+
methodHandle = lookup.findVirtual(Field.class, "trySetAccessible", methodType(boolean.class));
24+
} catch (Exception e) {
25+
LOGGER.debug("Looking up trySetAccessible failed: ", e);
26+
}
27+
TRY_SET_ACCESSIBLE = methodHandle;
28+
}
29+
// We cannot create a Field instance from scratch to be used as special constant,
30+
// so need to reflectively access itself
31+
private static final Field INACCESSIBLE_FIELD;
32+
33+
static {
34+
Field field = null;
35+
try {
36+
field = ReflectiveFieldValueResolver.class.getDeclaredField("INACCESSIBLE_FIELD");
37+
} catch (Exception e) {
38+
LOGGER.debug("INACCESSIBLE_FIELD failed: ", e);
39+
}
40+
INACCESSIBLE_FIELD = field;
41+
}
42+
43+
private static final Class<?> MODULE_CLASS;
44+
private static final MethodHandle GET_MODULE;
45+
46+
static {
47+
MethodHandle methodHandle = null;
48+
Class<?> moduleClass = null;
49+
try {
50+
moduleClass = Class.forName("java.lang.Module");
51+
MethodHandles.Lookup lookup = MethodHandles.lookup();
52+
methodHandle = lookup.findVirtual(Class.class, "getModule", methodType(moduleClass));
53+
} catch (Exception e) {
54+
LOGGER.debug("Looking up getModule failed: ", e);
55+
}
56+
GET_MODULE = methodHandle;
57+
MODULE_CLASS = moduleClass;
58+
}
59+
960
public static Object resolve(Object target, Class<?> targetType, String fldName) {
1061
Field fld = safeGetField(targetType, fldName);
1162
if (fld == null) {
1263
return Values.UNDEFINED_OBJECT;
1364
}
65+
if (fld == INACCESSIBLE_FIELD) {
66+
return Values.UNDEFINED_OBJECT;
67+
}
1468
try {
1569
return Modifier.isStatic(fld.getModifiers()) ? fld.get(null) : fld.get(target);
1670
} catch (IllegalAccessException | IllegalArgumentException ignored) {
@@ -35,10 +89,11 @@ public static CapturedContext.CapturedValue getFieldAsCapturedValue(
3589
Class<?> clazz, Object target, String fieldName) {
3690
Field field;
3791
try {
38-
field = getField(clazz, fieldName);
92+
FieldResult fieldResult = getField(clazz, fieldName);
93+
field = fieldResult.field;
3994
if (field == null) {
4095
return CapturedContext.CapturedValue.notCapturedReason(
41-
fieldName, Object.class.getTypeName(), "Field not found");
96+
fieldName, Object.class.getTypeName(), fieldResult.msg);
4297
}
4398
} catch (Exception ex) {
4499
return CapturedContext.CapturedValue.notCapturedReason(
@@ -153,7 +208,7 @@ private static Field getField(Object target, String name) throws NoSuchFieldExce
153208

154209
private static Field safeGetField(Class<?> container, String name) {
155210
try {
156-
return getField(container, name);
211+
return getField(container, name).field;
157212
} catch (SecurityException ignored) {
158213
return null;
159214
} catch (Exception e) {
@@ -163,18 +218,58 @@ private static Field safeGetField(Class<?> container, String name) {
163218
}
164219
}
165220

166-
private static Field getField(Class<?> container, String name) {
221+
private static class FieldResult {
222+
final Field field;
223+
final String msg;
224+
225+
public FieldResult(Field field, String msg) {
226+
this.field = field;
227+
this.msg = msg;
228+
}
229+
}
230+
231+
private static FieldResult getField(Class<?> container, String name) {
167232
while (container != null) {
168233
Field[] declaredFields = container.getDeclaredFields();
169234
for (int i = 0; i < declaredFields.length; i++) {
170235
Field declaredField = declaredFields[i];
171236
if (declaredField.getName().equals(name)) {
172-
declaredField.setAccessible(true);
173-
return declaredField;
237+
if (trySetAccessible(declaredField)) {
238+
declaredField.setAccessible(true);
239+
} else {
240+
return new FieldResult(null, buildInaccessibleMsg(declaredField));
241+
}
242+
return new FieldResult(declaredField, null);
174243
}
175244
}
176245
container = container.getSuperclass();
177246
}
178-
return null;
247+
return new FieldResult(null, "Field not found");
248+
}
249+
250+
public static boolean trySetAccessible(Field field) {
251+
if (TRY_SET_ACCESSIBLE == null) {
252+
return true;
253+
}
254+
try {
255+
return (boolean) TRY_SET_ACCESSIBLE.invokeExact(field);
256+
} catch (Throwable e) {
257+
LOGGER.debug("trySetAccessible call failed: ", e);
258+
return true;
259+
}
260+
}
261+
262+
public static String buildInaccessibleMsg(Field field) {
263+
if (MODULE_CLASS != null && GET_MODULE != null) {
264+
try {
265+
Object module = GET_MODULE.invoke(field.getDeclaringClass());
266+
return "Field is not accessible: "
267+
+ module
268+
+ " does not opens/exports to the current module";
269+
} catch (Throwable ex) {
270+
LOGGER.debug("buildInaccessibleMsg failed: ", ex);
271+
}
272+
}
273+
return "Field is not accessible";
179274
}
180275
}

‎dd-java-agent/agent-debugger/debugger-bootstrap/src/test/java/datadog/trace/bootstrap/debugger/el/ReflectiveFieldValueResolverTest.java‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,52 @@
11
package datadog.trace.bootstrap.debugger.el;
22

3+
import static datadog.trace.bootstrap.debugger.el.ReflectiveFieldValueResolver.getFieldAsCapturedValue;
4+
import static datadog.trace.bootstrap.debugger.el.ReflectiveFieldValueResolver.resolve;
35
import static org.junit.jupiter.api.Assertions.*;
46

7+
import datadog.trace.bootstrap.debugger.CapturedContext;
8+
import java.util.ArrayList;
59
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.api.condition.EnabledForJreRange;
11+
import org.junit.jupiter.api.condition.JRE;
612

713
class ReflectiveFieldValueResolverTest {
814

915
@Test
10-
void getFieldAsCapturedValue() {
11-
assertNull(ReflectiveFieldValueResolver.getFieldAsCapturedValue(null, "fieldName").getValue());
16+
void testGetFieldAsCapturedValue() {
17+
assertNull(getFieldAsCapturedValue(null, "fieldName").getValue());
1218
C c = new C();
13-
assertEquals(3, ReflectiveFieldValueResolver.getFieldAsCapturedValue(c, "c1").getValue());
14-
assertEquals(1, ReflectiveFieldValueResolver.getFieldAsCapturedValue(c, "a1").getValue());
15-
assertEquals("2", ReflectiveFieldValueResolver.getFieldAsCapturedValue(c, "a2").getValue());
16-
assertNull(ReflectiveFieldValueResolver.getFieldAsCapturedValue(c, "notExist").getValue());
19+
assertEquals(3, getFieldAsCapturedValue(c, "c1").getValue());
20+
assertEquals(1, getFieldAsCapturedValue(c, "a1").getValue());
21+
assertEquals("2", getFieldAsCapturedValue(c, "a2").getValue());
22+
assertNull(getFieldAsCapturedValue(c, "notExist").getValue());
23+
}
24+
25+
@Test
26+
@EnabledForJreRange(min = JRE.JAVA_17)
27+
void testGetFieldAsCapturedValueInaccessible() {
28+
CapturedContext.CapturedValue elementData =
29+
getFieldAsCapturedValue(new ArrayList<>(), "elementData");
30+
assertEquals(
31+
"Field is not accessible: module java.base does not opens/exports to the current module",
32+
elementData.getNotCapturedReason());
33+
}
34+
35+
@Test
36+
void testResolve() {
37+
assertEquals(1, resolve(new A(), A.class, "a1"));
38+
assertEquals("2", resolve(new A(), A.class, "a2"));
39+
assertEquals(3, resolve(new C(), C.class, "c1"));
40+
assertEquals(1, resolve(new C(), C.class, "a1"));
41+
assertEquals(4, resolve(new C(), C.class, "c2"));
42+
assertEquals(Values.UNDEFINED_OBJECT, resolve(new C(), C.class, "unknown"));
43+
}
44+
45+
@Test
46+
@EnabledForJreRange(min = JRE.JAVA_17)
47+
void testResolveInaccessible() {
48+
assertEquals(
49+
Values.UNDEFINED_OBJECT, resolve(new ArrayList<>(), ArrayList.class, "elementData"));
1750
}
1851

1952
static class A {
@@ -25,5 +58,6 @@ static class B extends A {}
2558

2659
static class C extends A {
2760
private int c1 = 3;
61+
private static int c2 = 4;
2862
}
2963
}

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/StringTemplateBuilder.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ public String evaluate(CapturedContext context, LogProbe.LogStatus status) {
5454
}
5555
} catch (EvaluationException ex) {
5656
status.addError(new EvaluationError(ex.getExpr(), ex.getMessage()));
57-
status.setLogTemplateErrors(true);
5857
String msg =
5958
ex instanceof RedactedException ? Redaction.REDACTED_VALUE : ex.getMessage();
6059
sb.append("{").append(msg).append("}");
6160
}
61+
if (!status.getErrors().isEmpty()) {
62+
status.setLogTemplateErrors(true);
63+
}
6264
}
6365
}
6466
}

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/MoshiSnapshotHelper.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,14 +446,19 @@ public void handleFieldException(Exception ex, Field field) {
446446
field.getName(),
447447
ExceptionHelper.foldExceptionStackTrace(ex));
448448
}
449+
fieldNotCaptured(ex.toString(), field);
450+
}
451+
452+
@Override
453+
public void fieldNotCaptured(String reason, Field field) {
449454
String fieldName = field.getName();
450455
try {
451456
jsonWriter.name(fieldName);
452457
jsonWriter.beginObject();
453458
jsonWriter.name(TYPE);
454459
jsonWriter.value(field.getType().getTypeName());
455460
jsonWriter.name(NOT_CAPTURED_REASON);
456-
jsonWriter.value(ex.toString());
461+
jsonWriter.value(reason);
457462
jsonWriter.endObject();
458463
} catch (IOException e) {
459464
LOGGER.debug("Serialization error: failed to extract field", e);

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import datadog.trace.bootstrap.debugger.CapturedContext;
66
import datadog.trace.bootstrap.debugger.Limits;
7+
import datadog.trace.bootstrap.debugger.el.ReflectiveFieldValueResolver;
78
import datadog.trace.bootstrap.debugger.util.Redaction;
89
import datadog.trace.bootstrap.debugger.util.TimeoutChecker;
910
import datadog.trace.bootstrap.debugger.util.WellKnownClasses;
@@ -102,6 +103,8 @@ default boolean objectFilterInField(Field field) throws Exception {
102103

103104
void handleFieldException(Exception ex, Field field);
104105

106+
void fieldNotCaptured(String reason, Field field);
107+
105108
void objectEpilogue(Object value) throws Exception;
106109

107110
void notCaptured(NotCapturedReason reason) throws Exception;
@@ -111,6 +114,7 @@ default boolean objectFilterInField(Field field) throws Exception {
111114

112115
private final TokenWriter tokenWriter;
113116
private final TimeoutChecker timeoutChecker;
117+
private RuntimeException exception;
114118

115119
public SerializerWithLimits(TokenWriter tokenWriter, TimeoutChecker timeoutChecker) {
116120
this.tokenWriter = tokenWriter;
@@ -298,9 +302,14 @@ private void serializeObjectValue(Object value, Limits limits) throws Exception
298302
}
299303

300304
private void onField(Field field, Object obj, Limits limits) throws Exception {
301-
field.setAccessible(true);
302-
Object fieldValue = field.get(obj);
303-
internalOnField(field.getName(), field.getType().getTypeName(), fieldValue, limits);
305+
if (ReflectiveFieldValueResolver.trySetAccessible(field)) {
306+
field.setAccessible(true);
307+
Object fieldValue = field.get(obj);
308+
internalOnField(field.getName(), field.getType().getTypeName(), fieldValue, limits);
309+
} else {
310+
String msg = ReflectiveFieldValueResolver.buildInaccessibleMsg(field);
311+
tokenWriter.fieldNotCaptured(msg, field);
312+
}
304313
}
305314

306315
private void onSpecialField(

0 commit comments

Comments
 (0)