Skip to content

Fixed NPE when MDC contains null values. #1364

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.

Sign up for GitHub

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

Merged
marandaneto merged 7 commits into from
Apr 1, 2021
Merged

Fixed NPE when MDC contains null values. #1364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a66a455
Fixed NPE when MDC contains null values.
donbeave Mar 30, 2021
5d22a48
Test ignore set tags with null values from MDC.
donbeave Mar 30, 2021
27e6277
Code format.
donbeave Mar 30, 2021
00a5613
Added changelog.
donbeave Mar 30, 2021
33325b0
Update CHANGELOG.md
donbeave Mar 31, 2021
e621444
Added changelog to Unreleased scope.
donbeave Mar 31, 2021
eef437a
Merge branch 'main' into main
marandaneto Mar 31, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Unreleased

* Feat: Add option to ignore exceptions by type (#1352)
* Fix: Fix NPE when MDC contains null values (#1364)
* Feat: Sentry closes Android NDK and ShutdownHook integrations (#1358)
* Fix: Accept only non null value maps (#1368)

Expand Down
6 changes: 4 additions & 2 deletions sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import io.sentry.SentryOptions;
import io.sentry.protocol.Message;
import io.sentry.protocol.SdkVersion;
import io.sentry.util.CollectionUtils;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -91,8 +90,11 @@ protected void append(@NotNull ILoggingEvent eventObject) {
event.setExtra("thread_name", loggingEvent.getThreadName());
}

// remove keys with null values, there is no sense to send these keys to Sentry
final Map<String, String> mdcProperties =
CollectionUtils.shallowCopy(loggingEvent.getMDCPropertyMap());
loggingEvent.getMDCPropertyMap().entrySet().stream()

@marandaneto marandaneto Apr 8, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind doing the same fix for log4j2 SentryAdapter? it's pretty much the same thing.

.filter(it -> it.getValue() != null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
if (!mdcProperties.isEmpty()) {
event.getContexts().put("MDC", mdcProperties);
}
Expand Down
15 changes: 15 additions & 0 deletions sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
import org.awaitility.kotlin.await
import org.slf4j.Logger
Expand Down Expand Up @@ -204,6 +205,20 @@ class SentryAppenderTest {
}
}

@Test
fun `ignore set tags with null values from MDC`() {
fixture = Fixture(minimumEventLevel = Level.WARN)
MDC.put("key1", null)
MDC.put("key2", null)
fixture.logger.warn("testing MDC tags")

await.untilAsserted {
verify(fixture.transport).send(checkEvent { event ->
assertNull(event.contexts["MDC"])
}, anyOrNull())
}
}

@Test
fun `does not create MDC context when no MDC tags are set`() {
fixture = Fixture(minimumEventLevel = Level.WARN)
Expand Down