Lazily cache request body for Spring requests instead of eagerly caching it

sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/it/SentrySpringIntegrationTest.kt

@@ -46,6 +46,7 @@ import org.springframework.web.bind.annotation.ControllerAdvice
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PostMapping
+import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RestController
 import kotlin.test.BeforeTest
 import kotlin.test.Test
@@ -95,6 +96,24 @@ class SentrySpringIntegrationTest {
 
     @Test
     fun `attaches request body to SentryEvents`() {
+        val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
+        val headers = HttpHeaders().apply {
+            this.contentType = MediaType.APPLICATION_JSON
+        }
+        val httpEntity = HttpEntity("""{"body":"content"}""", headers)
+        restTemplate.exchange("http://localhost:$port/bodyAsParam", HttpMethod.POST, httpEntity, Void::class.java)
+
+        verify(transport).send(
+            checkEvent { event ->
+                assertThat(event.request).isNotNull()
+                assertThat(event.request!!.data).isEqualTo("""{"body":"content"}""")
+            },
+            anyOrNull()
+        )
+    }
+
+    @Test
+    fun `attaches request body to SentryEvents on empty ControllerMethod Params`() {
         val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
         val headers = HttpHeaders().apply {
             this.contentType = MediaType.APPLICATION_JSON
@@ -266,6 +285,11 @@ class HelloController(private val helloService: HelloService) {
     fun body() {
         Sentry.captureMessage("body")
     }
+
+    @PostMapping("/bodyAsParam")
+    fun bodyWithReadingBodyInController(@RequestBody body: String) {
+        Sentry.captureMessage("body")
+    }
 }
 
 @Service

sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt

@@ -46,6 +46,7 @@ import org.springframework.web.bind.annotation.ControllerAdvice
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PostMapping
+import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RestController
 import kotlin.test.BeforeTest
 import kotlin.test.Test
@@ -95,6 +96,24 @@ class SentrySpringIntegrationTest {
 
     @Test
     fun `attaches request body to SentryEvents`() {
+        val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
+        val headers = HttpHeaders().apply {
+            this.contentType = MediaType.APPLICATION_JSON
+        }
+        val httpEntity = HttpEntity("""{"body":"content"}""", headers)
+        restTemplate.exchange("http://localhost:$port/bodyAsParam", HttpMethod.POST, httpEntity, Void::class.java)
+
+        verify(transport).send(
+            checkEvent { event ->
+                assertThat(event.request).isNotNull()
+                assertThat(event.request!!.data).isEqualTo("""{"body":"content"}""")
+            },
+            anyOrNull()
+        )
+    }
+
+    @Test
+    fun `attaches request body to SentryEvents on empty ControllerMethod Params`() {
         val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
         val headers = HttpHeaders().apply {
             this.contentType = MediaType.APPLICATION_JSON
@@ -266,6 +285,11 @@ class HelloController(private val helloService: HelloService) {
     fun body() {
         Sentry.captureMessage("body")
     }
+
+    @PostMapping("/bodyAsParam")
+    fun bodyWithReadingBodyInController(@RequestBody body: String) {
+        Sentry.captureMessage("body")
+    }
 }
 
 @Service

sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/RequestPayloadExtractor.java

@@ -14,10 +14,10 @@ final class RequestPayloadExtractor {
   @Nullable
   String extract(final @NotNull HttpServletRequest request, final @NotNull SentryOptions options) {
     // request body can be read only once from the stream
-    // original request can be replaced with CachedBodyHttpServletRequest in SentrySpringFilter
-    if (request instanceof CachedBodyHttpServletRequest) {
+    // original request can be replaced with ContentCachingRequestWrapper in SentrySpringFilter
+    if (request instanceof SentryContentCachingRequestWrapper cachedRequest) {
       try {
-        final byte[] body = StreamUtils.copyToByteArray(request.getInputStream());
+        final byte[] body = StreamUtils.copyToByteArray(cachedRequest.getInputStream());
         return new String(body, StandardCharsets.UTF_8);
       } catch (IOException e) {
         options.getLogger().log(SentryLevel.ERROR, "Failed to set request body", e);

sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryContentCachingRequestWrapper.java

@@ -0,0 +1,78 @@
+package io.sentry.spring.jakarta;
+
+import jakarta.servlet.ReadListener;
+import jakarta.servlet.ServletInputStream;
+import jakarta.servlet.http.HttpServletRequest;
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.springframework.web.util.ContentCachingRequestWrapper;
+
+public final class SentryContentCachingRequestWrapper extends ContentCachingRequestWrapper {
+  public SentryContentCachingRequestWrapper(HttpServletRequest request) {
+    super(request);
+  }
+
+  public SentryContentCachingRequestWrapper(HttpServletRequest request, int contentCacheLimit) {
+    super(request, contentCacheLimit);
+  }
+
+  @Override
+  public @NotNull ServletInputStream getInputStream() throws IOException {
+    final ServletInputStream originalInputStream = super.getInputStream();
+    if (originalInputStream.isFinished()) {
+      return new CachedBodyServletInputStream(getContentAsByteArray());
+    } else {
+      return originalInputStream;
+    }
+  }
+
+  @Override
+  public @NotNull BufferedReader getReader() throws IOException {
+    return new BufferedReader(new InputStreamReader(getInputStream(), StandardCharsets.UTF_8));
+  }
+
+  private static final class CachedBodyServletInputStream extends ServletInputStream {
+
+    private final @NotNull InputStream cachedBodyInputStream;
+
+    public CachedBodyServletInputStream(final @NotNull byte[] cachedBody) {
+      this.cachedBodyInputStream = new ByteArrayInputStream(cachedBody);
+    }
+
+    @Override
+    @SuppressWarnings("EmptyCatch")
+    public boolean isFinished() {
+      try {
+        return cachedBodyInputStream.available() == 0;
+      } catch (IOException e) {
+      }
+      return false;
+    }
+
+    @Override
+    public boolean isReady() {
+      return true;
+    }
+
+    @Override
+    public void setReadListener(final @Nullable ReadListener readListener) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public int read() throws IOException {
+      return cachedBodyInputStream.read();
+    }
+
+    @Override
+    public int read(@NotNull byte[] b, int off, int len) throws IOException {
+      return cachedBodyInputStream.read(b, off, len);
+    }
+  }
+}

sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java

@@ -93,7 +93,7 @@ private void configureScope(
             // only if request caches body, add an event processor that sets body on the event
             // body is not on the scope, to avoid using memory when no event is triggered during
             // request processing
-            if (request instanceof CachedBodyHttpServletRequest) {
+            if (request instanceof SentryContentCachingRequestWrapper) {
               scope.addEventProcessor(
                   new RequestBodyExtractingEventProcessor(request, scopes.getOptions()));
             }
@@ -110,18 +110,7 @@ private void configureScope(
       final @NotNull IScopes scopes, final @NotNull HttpServletRequest request) {
     if (scopes.getOptions().isSendDefaultPii()
         && qualifiesForCaching(request, scopes.getOptions().getMaxRequestBodySize())) {
-      try {
-        return new CachedBodyHttpServletRequest(request);
-      } catch (IOException e) {
-        scopes
-            .getOptions()
-            .getLogger()
-            .log(
-                SentryLevel.WARNING,
-                "Failed to cache HTTP request body. Request body will not be attached to Sentry events.",
-                e);
-        return request;
-      }
+      return new SentryContentCachingRequestWrapper(request);
     }
     return request;
   }

sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentrySpringFilterTest.kt

@@ -273,7 +273,7 @@ class SentrySpringFilterTest {
 
                 verify(fixture.chain).doFilter(
                     check {
-                        assertEquals(param.expectedToBeCached, it is CachedBodyHttpServletRequest)
+                        assertEquals(param.expectedToBeCached, it is SentryContentCachingRequestWrapper)
                     },
                     any()
                 )

sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/mvc/SentrySpringIntegrationTest.kt

@@ -65,6 +65,7 @@ import org.springframework.web.bind.annotation.ControllerAdvice
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PostMapping
+import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RestController
 import org.springframework.web.reactive.function.client.ExchangeFilterFunctions
 import org.springframework.web.reactive.function.client.WebClient
@@ -137,6 +138,24 @@ class SentrySpringIntegrationTest {
 
     @Test
     fun `attaches request body to SentryEvents`() {
+        val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
+        val headers = HttpHeaders().apply {
+            this.contentType = MediaType.APPLICATION_JSON
+        }
+        val httpEntity = HttpEntity("""{"body":"content"}""", headers)
+        restTemplate.exchange("http://localhost:$port/bodyAsParam", HttpMethod.POST, httpEntity, Void::class.java)
+
+        verify(transport).send(
+            checkEvent { event ->
+                assertThat(event.request).isNotNull()
+                assertThat(event.request!!.data).isEqualTo("""{"body":"content"}""")
+            },
+            anyOrNull()
+        )
+    }
+
+    @Test
+    fun `attaches request body to SentryEvents on empty ControllerMethod Params`() {
         val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
         val headers = HttpHeaders().apply {
             this.contentType = MediaType.APPLICATION_JSON
@@ -452,6 +471,11 @@ class HelloController(private val webClient: WebClient, private val env: Environ
         Sentry.captureMessage("body")
     }
 
+    @PostMapping("/bodyAsParam")
+    fun bodyWithReadingBodyInController(@RequestBody body: String) {
+        Sentry.captureMessage("body")
+    }
+
     @GetMapping("/throws")
     fun throws() {
         throw RuntimeException("something went wrong")