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

No longer serialize static fields; use toString as fallback #2309

Merged
merged 10 commits into from Oct 20, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
- Ensure potential callback exceptions are caught #2123 ([#2291](https://github.com/getsentry/sentry-java/pull/2291))
- Remove verbose FrameMetricsAggregator failure logging ([#2293](https://github.com/getsentry/sentry-java/pull/2293))
- Ignore broken regex for tracePropagationTarget ([#2288](https://github.com/getsentry/sentry-java/pull/2288))
- No longer serialize static fields; use toString as fallback ([#2309](https://github.com/getsentry/sentry-java/pull/2309))

### Features

Expand Down
Expand Up @@ -2,13 +2,21 @@

import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.net.InetAddress;
import java.net.URI;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collection;
import java.util.Currency;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicIntegerArray;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -39,6 +47,22 @@ public final class JsonReflectionObjectSerializer {
return object;
} else if (object instanceof String) {
return object;
} else if (object instanceof Locale) {
return object.toString();
} else if (object instanceof AtomicIntegerArray) {
return atomicIntegerArrayToList((AtomicIntegerArray) object);
} else if (object instanceof AtomicBoolean) {
return ((AtomicBoolean) object).get();
} else if (object instanceof URI) {
return object.toString();
} else if (object instanceof InetAddress) {
return object.toString();
} else if (object instanceof UUID) {
return object.toString();
} else if (object instanceof Currency) {
return object.toString();
} else if (object instanceof Calendar) {
return calendarToMap((Calendar) object);
} else if (object.getClass().isEnum()) {
return object.toString();
} else {
Expand All @@ -63,7 +87,12 @@ public final class JsonReflectionObjectSerializer {
} else if (object instanceof Map) {
serializedObject = map((Map<?, ?>) object, logger);
} else {
serializedObject = serializeObject(object, logger);
@NotNull Map<String, Object> objectAsMap = serializeObject(object, logger);
if (objectAsMap.isEmpty()) {
serializedObject = object.toString();
} else {
serializedObject = objectAsMap;
}
}
} catch (Exception exception) {
logger.log(SentryLevel.INFO, "Not serializing object due to throwing sub-path.", exception);
Expand All @@ -84,6 +113,9 @@ public final class JsonReflectionObjectSerializer {
if (Modifier.isTransient(field.getModifiers())) {
continue;
}
if (Modifier.isStatic(field.getModifiers())) {
continue;
}
String fieldName = field.getName();
try {
field.setAccessible(true);
Expand All @@ -94,6 +126,7 @@ public final class JsonReflectionObjectSerializer {
logger.log(SentryLevel.INFO, "Cannot access field " + fieldName + ".");
}
}

return map;
}

Expand All @@ -108,6 +141,15 @@ public final class JsonReflectionObjectSerializer {
return list;
}

private @NotNull List<Object> atomicIntegerArrayToList(@NotNull AtomicIntegerArray array)
throws Exception {
List<Object> list = new ArrayList<>();
for (int i = 0; i < array.length(); i++) {
list.add(array.get(i));
}
return list;
}

private @NotNull List<Object> list(@NotNull Collection<?> collection, @NotNull ILogger logger)
throws Exception {
List<Object> list = new ArrayList<>();
Expand All @@ -131,4 +173,17 @@ public final class JsonReflectionObjectSerializer {
}
return hashMap;
}

private @NotNull Map<String, Object> calendarToMap(@NotNull Calendar calendar) {
Map<String, Object> map = new HashMap<>();

map.put("year", (long) calendar.get(1));
adinauer marked this conversation as resolved.
Show resolved Hide resolved
map.put("month", (long) calendar.get(2));
map.put("dayOfMonth", (long) calendar.get(5));
map.put("hourOfDay", (long) calendar.get(11));
map.put("minute", (long) calendar.get(12));
map.put("second", (long) calendar.get(13));

return map;
}
}
51 changes: 48 additions & 3 deletions sentry/src/test/java/io/sentry/JsonObjectSerializerTest.kt
@@ -1,8 +1,11 @@
package io.sentry

import com.nhaarman.mockitokotlin2.inOrder
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
import org.junit.Test
import java.util.Locale
import java.util.TimeZone

internal class JsonObjectSerializerTest {
Expand Down Expand Up @@ -114,9 +117,9 @@ internal class JsonObjectSerializerTest {

@Test
fun `serialize unknown object without data`() {
fixture.getSUT().serialize(fixture.writer, fixture.logger, object {})
verify(fixture.writer).beginObject()
verify(fixture.writer).endObject()
val value = object {}
fixture.getSUT().serialize(fixture.writer, fixture.logger, value)
verify(fixture.writer).value(value.toString())
}

@Test
Expand Down Expand Up @@ -166,10 +169,52 @@ internal class JsonObjectSerializerTest {
verify(fixture.writer).endObject()
}

@Test
fun `serialize locale`() {
val inOrder = inOrder(fixture.writer)
fixture.getSUT().serialize(fixture.writer, fixture.logger, Locale.US)

inOrder.verify(fixture.writer).value("en_US")
}

@Test
fun `serialize locale in map`() {
val map = mapOf<String, Locale>("one" to Locale.US)
val inOrder = inOrder(fixture.writer)
fixture.getSUT().serialize(fixture.writer, fixture.logger, map)
inOrder.verify(fixture.writer).beginObject()
inOrder.verify(fixture.writer).name("one")
inOrder.verify(fixture.writer).value("en_US")
inOrder.verify(fixture.writer).endObject()
}

@Test
fun `serialize locale in list`() {
val list = listOf<Locale>(Locale.US, Locale.GERMAN)
val inOrder = inOrder(fixture.writer)
fixture.getSUT().serialize(fixture.writer, fixture.logger, list)
inOrder.verify(fixture.writer).beginArray()
inOrder.verify(fixture.writer).value("en_US")
inOrder.verify(fixture.writer).value("de")
inOrder.verify(fixture.writer).endArray()
}

@Test
fun `serialize locale in object`() {
val obj = ClassWithLocaleProperty(Locale.US)
val inOrder = inOrder(fixture.writer)
fixture.getSUT().serialize(fixture.writer, fixture.logger, obj)
inOrder.verify(fixture.writer).beginObject()
inOrder.verify(fixture.writer).name("localeProperty")
inOrder.verify(fixture.writer).value("en_US")
inOrder.verify(fixture.writer).endObject()
}

class UnknownClassWithData(
private val integer: Int,
private val string: String
)
}

data class ClassWithEnumProperty(val enumProperty: DataCategory)
data class ClassWithLocaleProperty(val localeProperty: Locale)
Expand Up @@ -3,6 +3,13 @@ package io.sentry
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import org.junit.Test
import java.net.URI
import java.util.Calendar
import java.util.Currency
import java.util.Locale
import java.util.UUID
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicIntegerArray
import kotlin.test.assertEquals

class JsonReflectionObjectSerializerTest {
Expand Down Expand Up @@ -87,11 +94,13 @@ class JsonReflectionObjectSerializerTest {
}

@Test
fun `serialize object without fields`() {
fun `serialize object without fields using toString`() {
val objectWithoutFields = ClassWithoutFields()
val expected = mapOf<String, Any>()
val expected = mapOf<String, Any>(
"toString" to ""
)
val actual = fixture.getSut().serialize(objectWithoutFields, fixture.logger)
assertEquals(expected, actual)
assertEquals(objectWithoutFields.toString(), actual)
}

@Test
Expand Down Expand Up @@ -263,6 +272,78 @@ class JsonReflectionObjectSerializerTest {
assertEquals("Error", actual)
}

@Test
fun `locale`() {
val actual = fixture.getSut().serialize(Locale.US, fixture.logger)
assertEquals("en_US", actual)
}

@Test
fun `AtomicIntegerArray is serialized`() {
val actual = fixture.getSut().serialize(AtomicIntegerArray(arrayOf(1, 2, 3).toIntArray()), fixture.logger)
assertEquals(listOf(1, 2, 3), actual)
}

@Test
fun `AtomicBoolean is serialized`() {
val actual = fixture.getSut().serialize(AtomicBoolean(true), fixture.logger)
assertEquals(true, actual)
}

@Test
fun `StringBuffer is serialized`() {
val sb = StringBuffer()
sb.append("hello")
sb.append(" ")
sb.append("world")
val actual = fixture.getSut().serialize(sb, fixture.logger)
assertEquals("hello world", actual)
}

@Test
fun `StringBuilder is serialized`() {
val sb = StringBuilder()
sb.append("hello")
sb.append(" ")
sb.append("world")
val actual = fixture.getSut().serialize(sb, fixture.logger)
assertEquals("hello world", actual)
}

@Test
fun `URI is serialized`() {
val actual = fixture.getSut().serialize(URI("http://localhost:8081/api/product?id=99"), fixture.logger)
assertEquals("http://localhost:8081/api/product?id=99", actual)
}

@Test
fun `UUID is serialized`() {
val actual = fixture.getSut().serialize(UUID.fromString("828900a5-15dc-413f-8c17-6ef04d74e074"), fixture.logger)
assertEquals("828900a5-15dc-413f-8c17-6ef04d74e074", actual)
}

@Test
fun `Currency is serialized`() {
val actual = fixture.getSut().serialize(Currency.getInstance("USD"), fixture.logger)
assertEquals("USD", actual)
}

@Test
fun `Calendar is serialized`() {
val calendar = Calendar.getInstance()
calendar.set(2022, 0, 1, 11, 59, 58)
val actual = fixture.getSut().serialize(calendar, fixture.logger)
val expected = mapOf<String, Any?>(
"month" to 0L,
"year" to 2022L,
"dayOfMonth" to 1L,
"hourOfDay" to 11L,
"minute" to 59L,
"second" to 58L
)
assertEquals(expected, actual)
}

// Helper

class ClassWithPrimitiveFields(
Expand Down
Expand Up @@ -27,6 +27,7 @@
import java.io.StringWriter;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.concurrent.atomic.AtomicInteger;

@SuppressWarnings({"resource", "NullAway"}) // Ignore warnings to preserve original code.
public final class JsonWriterTest extends TestCase {
Expand Down Expand Up @@ -313,6 +314,13 @@ public void testBooleans() throws IOException {
assertEquals("[true,false]", stringWriter.toString());
}

public void testAtomicInteger() throws IOException {
StringWriter stringWriter = new StringWriter();
JsonWriter jsonWriter = new JsonWriter(stringWriter);
jsonWriter.value(new AtomicInteger(42));
assertEquals("42", stringWriter.toString());
}

public void testBoxedBooleans() throws IOException {
StringWriter stringWriter = new StringWriter();
JsonWriter jsonWriter = new JsonWriter(stringWriter);
Expand Down