Skip to content

Commit

Permalink
Fix SentryFileWriter and SentryFileOutputStream append overwrites fil…
Browse files Browse the repository at this point in the history
…e contents (#2304)
  • Loading branch information
romtsn authored Oct 18, 2022
1 parent a4fb390 commit 221a5df
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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))
- Fix `SentryFileWriter`/`SentryFileOutputStream` append overwrites file contents ([#2304](https://github.com/getsentry/sentry-java/pull/2304))

### Features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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(
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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`() {
Expand All @@ -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())
}
}

0 comments on commit 221a5df

Please sign in to comment.