Fix crash when HTTP connection error message contains formatting symb…

…ols (#3002)
getsentry · Oct 23, 2023 · 3548754 · 3548754
1 parent 02e9e80
commit 3548754
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Fix crash when HTTP connection error message contains formatting symbols ([#3002](https://github.com/getsentry/sentry-java/pull/3002))
+
 ## 6.32.0
 
 ### Features

diff --git a/sentry/src/main/java/io/sentry/transport/HttpConnection.java b/sentry/src/main/java/io/sentry/transport/HttpConnection.java
@@ -182,8 +182,10 @@ HttpURLConnection open() throws IOException {
  options.getLogger().log(ERROR, "Request failed, API returned %s", responseCode);
  // double check because call is expensive
  if (options.isDebug()) {
- String errorMessage = getErrorMessageFromStream(connection);
- options.getLogger().log(ERROR, errorMessage);
+ final @NotNull String errorMessage = getErrorMessageFromStream(connection);
+ // the error message may contain anything (including formatting symbols), so provide it as
+ // an argument itself
+ options.getLogger().log(ERROR, "%s", errorMessage);
  }
 
  return TransportResult.error(responseCode);

diff --git a/sentry/src/test/java/io/sentry/transport/HttpConnectionTest.kt b/sentry/src/test/java/io/sentry/transport/HttpConnectionTest.kt
@@ -1,9 +1,11 @@
 package io.sentry.transport
 
+import io.sentry.ILogger
 import io.sentry.ISerializer
 import io.sentry.RequestDetails
 import io.sentry.SentryEnvelope
 import io.sentry.SentryEvent
+import io.sentry.SentryLevel
 import io.sentry.SentryOptions
 import io.sentry.SentryOptions.Proxy
 import io.sentry.Session
@@ -19,6 +21,7 @@ import java.io.IOException
 import java.net.InetSocketAddress
 import java.net.Proxy.Type
 import java.net.URL
+import java.nio.charset.Charset
 import javax.net.ssl.HostnameVerifier
 import javax.net.ssl.HttpsURLConnection
 import javax.net.ssl.SSLSocketFactory
@@ -40,6 +43,7 @@ class HttpConnectionTest {
  var sslSocketFactory: SSLSocketFactory? = null
  var hostnameVerifier: HostnameVerifier? = null
  val requestDetails = mock<RequestDetails>()
+ val options = SentryOptions()
 
  init {
  whenever(connection.outputStream).thenReturn(mock())
@@ -54,7 +58,6 @@ class HttpConnectionTest {
  }
 
  fun getSUT(): HttpConnection {
- val options = SentryOptions()
  options.setSerializer(serializer)
  options.proxy = proxy
  options.sslSocketFactory = sslSocketFactory
@@ -187,6 +190,34 @@ class HttpConnectionTest {
  verify(fixture.connection, never()).hostnameVerifier = any()
  }
 
+ @Test
+ fun `When connection error message contains formatting symbols, does not crash the logger`() {
+ fixture.options.isDebug = true
+ fixture.options.setLogger(object : ILogger {
+ override fun log(level: SentryLevel, message: String, vararg args: Any?) =
+ println(String.format(message, args))
+
+ override fun log(level: SentryLevel, message: String, throwable: Throwable?) =
+ println(message)
+
+ override fun log(
+ level: SentryLevel,
+ throwable: Throwable?,
+ message: String,
+ vararg args: Any?
+ ) = println(String.format(message))
+
+ override fun isEnabled(level: SentryLevel?): Boolean = true
+ })
+
+ // when error message contains funky formatting symbols
+ whenever(fixture.connection.errorStream).thenReturn("Something is off %d, %s, %s\n".byteInputStream(Charset.forName("UTF-8")))
+ val transport = fixture.getSUT()
+
+ // it should not crash
+ transport.send(createEnvelope())
+ }
+
  @Test
  fun `When Proxy host and port are given, set to connection`() {
  fixture.proxy = Proxy("proxy.example.com", "8090")