From f4c3c85052a9e5c10d3ab2ae905e52221015b56a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 17 Oct 2022 21:13:48 +0200 Subject: [PATCH 1/3] Fix SentryFileWriter and SentryFileOutputStream append overwrites file contents --- .../file/SentryFileOutputStream.java | 10 ++++----- .../file/SentryFileWriter.java | 5 +++-- .../file/SentryFileOutputStreamTest.kt | 3 ++- .../file/SentryFileWriterTest.kt | 21 +++++++++++++++++-- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java index a48e10def0..791e2dcdc3 100644 --- a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java +++ b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java @@ -24,7 +24,7 @@ public final class SentryFileOutputStream extends FileOutputStream { private final @NotNull FileIOSpanManager spanManager; public SentryFileOutputStream(final @Nullable String name) throws FileNotFoundException { - this(name != null ? new File(name) : null, HubAdapter.getInstance()); + this(name != null ? new File(name) : null, false, HubAdapter.getInstance()); } public SentryFileOutputStream(final @Nullable String name, final boolean append) @@ -33,7 +33,7 @@ public SentryFileOutputStream(final @Nullable String name, final boolean append) } public SentryFileOutputStream(final @Nullable File file) throws FileNotFoundException { - this(file, HubAdapter.getInstance()); + this(file, false, HubAdapter.getInstance()); } public SentryFileOutputStream(final @Nullable File file, final boolean append) @@ -45,9 +45,9 @@ public SentryFileOutputStream(final @NotNull FileDescriptor fdObj) { this(init(fdObj, null, HubAdapter.getInstance()), fdObj); } - SentryFileOutputStream(final @Nullable File file, final @NotNull IHub hub) + SentryFileOutputStream(final @Nullable File file, final boolean append, final @NotNull IHub hub) throws FileNotFoundException { - this(init(file, false, null, hub)); + this(init(file, append, null, hub)); } private SentryFileOutputStream( @@ -72,7 +72,7 @@ private static FileOutputStreamInitData init( throws FileNotFoundException { final ISpan span = FileIOSpanManager.startSpan(hub, "file.write"); if (delegate == null) { - delegate = new FileOutputStream(file); + delegate = new FileOutputStream(file, append); } return new FileOutputStreamInitData( file, append, span, delegate, hub.getOptions().isSendDefaultPii()); diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java index 60afc8d11c..9588984612 100644 --- a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java +++ b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileWriter.java @@ -30,7 +30,8 @@ public SentryFileWriter(final @NotNull FileDescriptor fd) { super(new SentryFileOutputStream(fd)); } - SentryFileWriter(final @NotNull File file, final @NotNull IHub hub) throws FileNotFoundException { - super(new SentryFileOutputStream(file, hub)); + SentryFileWriter(final @NotNull File file, final boolean append, final @NotNull IHub hub) + throws FileNotFoundException { + super(new SentryFileOutputStream(file, append, hub)); } } diff --git a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt index b455a6642f..6753715138 100644 --- a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt +++ b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt @@ -22,13 +22,14 @@ class SentryFileOutputStreamTest { internal fun getSut( tmpFile: File? = null, activeTransaction: Boolean = true, + append: Boolean = false ): SentryFileOutputStream { whenever(hub.options).thenReturn(SentryOptions()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) if (activeTransaction) { whenever(hub.span).thenReturn(sentryTracer) } - return SentryFileOutputStream(tmpFile, hub) + return SentryFileOutputStream(tmpFile, append, hub) } } diff --git a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt index f8f8fe1ea6..99d53ba865 100644 --- a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt +++ b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileWriterTest.kt @@ -21,13 +21,14 @@ class SentryFileWriterTest { internal fun getSut( tmpFile: File, activeTransaction: Boolean = true, + append: Boolean = false ): SentryFileWriter { whenever(hub.options).thenReturn(SentryOptions()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) if (activeTransaction) { whenever(hub.span).thenReturn(sentryTracer) } - return SentryFileWriter(tmpFile, hub) + return SentryFileWriter(tmpFile, append, hub) } } @@ -36,7 +37,7 @@ class SentryFileWriterTest { private val fixture = Fixture() - private val tmpFile: File get() = tmpDir.newFile("test.txt") + private val tmpFile: File by lazy { tmpDir.newFile("test.txt") } @Test fun `captures a span`() { @@ -53,4 +54,20 @@ class SentryFileWriterTest { assertEquals(fileIOSpan.isFinished, true) assertEquals(fileIOSpan.status, OK) } + + @Test + fun `append works`() { + val writer1 = fixture.getSut(tmpFile, append = true) + writer1.use { + it.append("test") + } + + // second writer should not overwrite the file contents, but append to the existing content + val writer2 = fixture.getSut(tmpFile, append = true) + writer2.use { + it.append("test2") + } + + assertEquals("testtest2", tmpFile.readText()) + } } From 6abadd1ffa3b3efaa9a0c124aa21f37cd2915f20 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 17 Oct 2022 21:17:28 +0200 Subject: [PATCH 2/3] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f00c2d97b6..7badaddbf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) +- Fix `SentryFileWriter`/`SentryFileOutputStream` append overwrites file contents ([#2304](https://github.com/getsentry/sentry-java/pull/2304)) ### Features From 5b18409d88225c09f12933c047038328d8fbc8b4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 18 Oct 2022 11:59:42 +0200 Subject: [PATCH 3/3] Fix flakey test --- .../android/core/SendCachedEnvelopeIntegrationTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt index d0dedbf823..4a1b836886 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt @@ -100,13 +100,13 @@ class SendCachedEnvelopeIntegrationTest { @Test fun `when synchronous send times out, continues the task on a background thread`() { - val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 50) - fixture.options.startupCrashFlushTimeoutMillis = 10 + val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 1000) + fixture.options.startupCrashFlushTimeoutMillis = 100 sut.register(fixture.hub, fixture.options) // first wait until synchronous send times out and check that the logger was hit in the catch block - await.atLeast(11, MILLISECONDS) + await.atLeast(500, MILLISECONDS) verify(fixture.logger).log( eq(DEBUG), eq("Synchronous send timed out, continuing in the background.")