Skip to content

Commit

Permalink
Send file name and path only if isSendDefaultPii is true (#3919)
Browse files Browse the repository at this point in the history 
* file span description is now masked if isSendDefaultPii is false
* file path on Android is sent only if isSendDefaultPii is true
  • Loading branch information
@stefanosiano
stefanosiano authored Nov 21, 2024
1 parent 159a367 commit e1b0b23
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 34 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
- Our `UncaughtExceptionHandlerIntegration` waited for the full flush timeout duration (default 15s) when rate limited.
- Do not replace `op` with auto generated content for OpenTelemetry spans with span kind `INTERNAL` ([#3906](https://github.com/getsentry/sentry-java/pull/3906))

### Behavioural Changes

- Send file name and path only if isSendDefaultPii is true ([#3919](https://github.com/getsentry/sentry-java/pull/3919))

## 8.0.0-beta.2

### Breaking Changes
Expand Down
20 changes: 18 additions & 2 deletions sentry/src/main/java/io/sentry/instrumentation/file/FileIOSpanManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ private void finishSpan() {
if (currentSpan != null) {
final String byteCountToString = StringUtils.byteCountToString(byteCount);
if (file != null) {
final String description = file.getName() + " " + "(" + byteCountToString + ")";
final String description = getDescription(file);
currentSpan.setDescription(description);
if (Platform.isAndroid() || options.isSendDefaultPii()) {
if (options.isSendDefaultPii()) {
currentSpan.setData("file.path", file.getAbsolutePath());
}
} else {
Expand All @@ -112,6 +112,22 @@ private void finishSpan() {
}
}

private @NotNull String getDescription(final @NotNull File file) {
final String byteCountToString = StringUtils.byteCountToString(byteCount);
// if we send PII, we can send the file name directly
if (options.isSendDefaultPii()) {
return file.getName() + " (" + byteCountToString + ")";
}
final int lastDotIndex = file.getName().lastIndexOf('.');
// if the file has an extension, show it in the description, even without sending PII
if (lastDotIndex > 0 && lastDotIndex < file.getName().length() - 1) {
final String fileExtension = file.getName().substring(lastDotIndex);
return "***" + fileExtension + " (" + byteCountToString + ")";
} else {
return "***" + " (" + byteCountToString + ")";
}
}

/**
* A task that returns a result and may throw an IOException. Implementors define a single method
* with no arguments called {@code call}.
Expand Down
44 changes: 38 additions & 6 deletions sentry/src/test/java/io/sentry/instrumentation/file/SentryFileInputStreamTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import kotlin.concurrent.thread
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue

Expand Down Expand Up @@ -80,6 +81,8 @@ class SentryFileInputStreamTest {

private val tmpFile: File get() = tmpDir.newFile("test.txt")

private val tmpFileWithoutExtension: File get() = tmpDir.newFile("test")

@Test
fun `when no active transaction does not capture a span`() {
fixture.getSut(tmpFile, activeTransaction = false)
Expand All @@ -104,13 +107,42 @@ class SentryFileInputStreamTest {

assertEquals(fixture.sentryTracer.children.size, 1)
val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (0 B)")
assertEquals(fileIOSpan.data["file.size"], 0L)
assertEquals(fileIOSpan.throwable, null)
assertEquals(fileIOSpan.isFinished, true)
assertEquals(fileIOSpan.status, SpanStatus.OK)
}

@Test
fun `captures file name in description and file path when isSendDefaultPii is true`() {
val fis = fixture.getSut(tmpFile, sendDefaultPii = true)
fis.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (0 B)")
assertNotNull(fileIOSpan.data["file.path"])
}

@Test
fun `captures only file extension in description when isSendDefaultPii is false`() {
val fis = fixture.getSut(tmpFile, sendDefaultPii = false)
fis.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "***.txt (0 B)")
assertNull(fileIOSpan.data["file.path"])
}

@Test
fun `captures only file size if no extension is available when isSendDefaultPii is false`() {
val fis = fixture.getSut(tmpFileWithoutExtension, sendDefaultPii = false)
fis.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "*** (0 B)")
assertNull(fileIOSpan.data["file.path"])
}

@Test
fun `when stream is closed, releases file descriptor`() {
val fis = fixture.getSut(tmpFile)
Expand All @@ -123,7 +155,7 @@ class SentryFileInputStreamTest {
fixture.getSut(tmpFile).use { it.read() }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (1 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (1 B)")
assertEquals(fileIOSpan.data["file.size"], 1L)
}

Expand All @@ -132,7 +164,7 @@ class SentryFileInputStreamTest {
fixture.getSut(tmpFile).use { it.read(ByteArray(10)) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (4 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (4 B)")
assertEquals(fileIOSpan.data["file.size"], 4L)
}

Expand All @@ -141,7 +173,7 @@ class SentryFileInputStreamTest {
fixture.getSut(tmpFile).use { it.read(ByteArray(10), 1, 3) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (3 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (3 B)")
assertEquals(fileIOSpan.data["file.size"], 3L)
}

Expand All @@ -150,7 +182,7 @@ class SentryFileInputStreamTest {
fixture.getSut(tmpFile).use { it.skip(10) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (10 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (10 B)")
assertEquals(fileIOSpan.data["file.size"], 10L)
}

Expand All @@ -159,7 +191,7 @@ class SentryFileInputStreamTest {
fixture.getSut(tmpFile).use { it.reader().readText() }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (4 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (4 B)")
assertEquals(fileIOSpan.data["file.size"], 4L)
}

Expand Down
64 changes: 52 additions & 12 deletions sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import kotlin.concurrent.thread
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue

Expand All @@ -30,14 +31,15 @@ class SentryFileOutputStreamTest {
internal fun getSut(
tmpFile: File? = null,
activeTransaction: Boolean = true,
append: Boolean = false
append: Boolean = false,
optionsConfiguration: (SentryOptions) -> Unit = {}
): SentryFileOutputStream {
whenever(scopes.options).thenReturn(
SentryOptions().apply {
threadChecker = ThreadChecker.getInstance()
addInAppInclude("org.junit")
}
)
val options = SentryOptions().apply {
threadChecker = ThreadChecker.getInstance()
addInAppInclude("org.junit")
optionsConfiguration(this)
}
whenever(scopes.options).thenReturn(options)
sentryTracer = SentryTracer(TransactionContext("name", "op"), scopes)
if (activeTransaction) {
whenever(scopes.span).thenReturn(sentryTracer)
Expand All @@ -53,6 +55,8 @@ class SentryFileOutputStreamTest {

private val tmpFile: File get() = tmpDir.newFile("test.txt")

private val tmpFileWithoutExtension: File get() = tmpDir.newFile("test")

@Test
fun `when no active transaction does not capture a span`() {
fixture.getSut(tmpFile, activeTransaction = false)
Expand All @@ -77,13 +81,49 @@ class SentryFileOutputStreamTest {

assertEquals(fixture.sentryTracer.children.size, 1)
val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (0 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (0 B)")
assertEquals(fileIOSpan.data["file.size"], 0L)
assertEquals(fileIOSpan.throwable, null)
assertEquals(fileIOSpan.isFinished, true)
assertEquals(fileIOSpan.status, SpanStatus.OK)
}

@Test
fun `captures file name in description and file path when isSendDefaultPii is true`() {
val fos = fixture.getSut(tmpFile) {
it.isSendDefaultPii = true
}
fos.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (0 B)")
assertNotNull(fileIOSpan.data["file.path"])
}

@Test
fun `captures only file extension in description when isSendDefaultPii is false`() {
val fos = fixture.getSut(tmpFile) {
it.isSendDefaultPii = false
}
fos.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "***.txt (0 B)")
assertNull(fileIOSpan.data["file.path"])
}

@Test
fun `captures only file size if no extension is available when isSendDefaultPii is false`() {
val fos = fixture.getSut(tmpFileWithoutExtension) {
it.isSendDefaultPii = false
}
fos.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "*** (0 B)")
assertNull(fileIOSpan.data["file.path"])
}

@Test
fun `when stream is closed file descriptor is also closed`() {
val fos = fixture.getSut(tmpFile)
Expand All @@ -96,7 +136,7 @@ class SentryFileOutputStreamTest {
fixture.getSut(tmpFile).use { it.write(29) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (1 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (1 B)")
assertEquals(fileIOSpan.data["file.size"], 1L)
}

Expand All @@ -105,7 +145,7 @@ class SentryFileOutputStreamTest {
fixture.getSut(tmpFile).use { it.write(ByteArray(10)) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (10 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (10 B)")
assertEquals(fileIOSpan.data["file.size"], 10L)
}

Expand All @@ -114,7 +154,7 @@ class SentryFileOutputStreamTest {
fixture.getSut(tmpFile).use { it.write(ByteArray(10), 1, 3) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (3 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (3 B)")
assertEquals(fileIOSpan.data["file.size"], 3L)
}

Expand All @@ -123,7 +163,7 @@ class SentryFileOutputStreamTest {
fixture.getSut(tmpFile).use { it.write("Text".toByteArray()) }

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (4 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (4 B)")
assertEquals(fileIOSpan.data["file.size"], 4L)
}

Expand Down
58 changes: 51 additions & 7 deletions sentry/src/test/java/io/sentry/instrumentation/file/SentryFileReaderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import org.mockito.kotlin.whenever
import java.io.File
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull

class SentryFileReaderTest {
class Fixture {
Expand All @@ -22,14 +24,15 @@ class SentryFileReaderTest {

internal fun getSut(
tmpFile: File,
activeTransaction: Boolean = true
activeTransaction: Boolean = true,
optionsConfiguration: (SentryOptions) -> Unit = {}
): SentryFileReader {
tmpFile.writeText("TEXT")
whenever(scopes.options).thenReturn(
SentryOptions().apply {
threadChecker = ThreadChecker.getInstance()
}
)
val options = SentryOptions().apply {
threadChecker = ThreadChecker.getInstance()
optionsConfiguration(this)
}
whenever(scopes.options).thenReturn(options)
sentryTracer = SentryTracer(TransactionContext("name", "op"), scopes)
if (activeTransaction) {
whenever(scopes.span).thenReturn(sentryTracer)
Expand All @@ -45,6 +48,8 @@ class SentryFileReaderTest {

private val tmpFile: File get() = tmpDir.newFile("test.txt")

private val tmpFileWithoutExtension: File get() = tmpDir.newFile("test")

@Test
fun `captures a span`() {
val reader = fixture.getSut(tmpFile)
Expand All @@ -54,11 +59,50 @@ class SentryFileReaderTest {
assertEquals(fixture.sentryTracer.children.size, 1)
val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.operation, "file.read")
assertEquals(fileIOSpan.spanContext.description, "test.txt (4 B)")
assertEquals(fileIOSpan.spanContext.description, "***.txt (4 B)")
assertEquals(fileIOSpan.data["file.size"], 4L)
assertEquals(fileIOSpan.throwable, null)
assertEquals(fileIOSpan.isFinished, true)
assertEquals(fileIOSpan.data[SpanDataConvention.BLOCKED_MAIN_THREAD_KEY], true)
assertEquals(fileIOSpan.status, OK)
}

@Test
fun `captures file name in description and file path when isSendDefaultPii is true`() {
val reader = fixture.getSut(tmpFile) {
it.isSendDefaultPii = true
}
reader.readText()
reader.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "test.txt (4 B)")
assertNotNull(fileIOSpan.data["file.path"])
}

@Test
fun `captures only file extension in description when isSendDefaultPii is false`() {
val reader = fixture.getSut(tmpFile) {
it.isSendDefaultPii = false
}
reader.readText()
reader.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "***.txt (4 B)")
assertNull(fileIOSpan.data["file.path"])
}

@Test
fun `captures only file size if no extension is available when isSendDefaultPii is false`() {
val reader = fixture.getSut(tmpFileWithoutExtension) {
it.isSendDefaultPii = false
}
reader.readText()
reader.close()

val fileIOSpan = fixture.sentryTracer.children.first()
assertEquals(fileIOSpan.spanContext.description, "*** (4 B)")
assertNull(fileIOSpan.data["file.path"])
}
}
Loading

0 comments on commit e1b0b23

Please sign in to comment.