Add request body extraction for Spring MVC integration (#1595)

Fixes #1334

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 * Fix: set min sdk version of sentry-android-fragment to API 14 (#1608)
+* Feat: Add request body extraction for Spring MVC integration (#1595)
 
 ## 5.1.0-beta.5
 

sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties

@@ -1,6 +1,7 @@
 # NOTE: Replace the test DSN below with YOUR OWN DSN to see the events from this app in your Sentry project/dashboard
 sentry.dsn=https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563
 sentry.send-default-pii=true
+sentry.max-request-body-size=medium
 # Sentry Spring Boot integration allows more fine-grained SentryOptions configuration
 sentry.max-breadcrumbs=150
 # Logback integration configuration options

sentry-samples/sentry-samples-spring/src/main/java/io/sentry/samples/spring/SentryConfig.java

@@ -1,5 +1,6 @@
 package io.sentry.samples.spring;
 
+import io.sentry.SentryOptions;
 import io.sentry.SentryOptions.TracesSamplerCallback;
 import io.sentry.spring.EnableSentry;
 import io.sentry.spring.tracing.SentryTracingConfiguration;
@@ -11,7 +12,8 @@
 // project/dashboard
 @EnableSentry(
     dsn = "https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563",
-    sendDefaultPii = true)
+    sendDefaultPii = true,
+    maxRequestBodySize = SentryOptions.RequestSize.MEDIUM)
 @Import(SentryTracingConfiguration.class)
 public class SentryConfig {
 

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java

@@ -11,7 +11,7 @@
 import io.sentry.protocol.SdkVersion;
 import io.sentry.spring.SentryExceptionResolver;
 import io.sentry.spring.SentryRequestResolver;
-import io.sentry.spring.SentrySpringRequestListener;
+import io.sentry.spring.SentrySpringFilter;
 import io.sentry.spring.SentryUserFilter;
 import io.sentry.spring.SentryUserProvider;
 import io.sentry.spring.SentryWebConfiguration;
@@ -124,10 +124,13 @@ static class HubConfiguration {
     @Open
     static class SentryWebMvcConfiguration {
 
+      private static final int SENTRY_SPRING_FILTER_PRECEDENCE = Ordered.HIGHEST_PRECEDENCE;
+
       @Configuration(proxyBeanMethods = false)
       @ConditionalOnClass(SecurityContextHolder.class)
       @Open
       static class SentrySecurityConfiguration {
+
         /**
          * Configures {@link SpringSecuritySentryUserProvider} only if Spring Security is on the
          * classpath. Its order is set to be higher than {@link
@@ -179,16 +182,13 @@ static class SentrySecurityConfiguration {
       }
 
       @Bean
-      public @NotNull SentrySpringRequestListener sentrySpringRequestListener(
-          final @NotNull IHub sentryHub, final @NotNull SentryRequestResolver requestResolver) {
-        return new SentrySpringRequestListener(sentryHub, requestResolver);
-      }
-
-      @Bean
-      @ConditionalOnMissingBean
-      public @NotNull SentryExceptionResolver sentryExceptionResolver(
-          final @NotNull IHub sentryHub, final @NotNull SentryProperties options) {
-        return new SentryExceptionResolver(sentryHub, options.getExceptionResolverOrder());
+      @ConditionalOnMissingBean(name = "sentrySpringFilter")
+      public @NotNull FilterRegistrationBean<SentrySpringFilter> sentrySpringFilter(
+          final @NotNull IHub hub, final @NotNull SentryRequestResolver requestResolver) {
+        FilterRegistrationBean<SentrySpringFilter> filter =
+            new FilterRegistrationBean<>(new SentrySpringFilter(hub, requestResolver));
+        filter.setOrder(SENTRY_SPRING_FILTER_PRECEDENCE);
+        return filter;
       }
 
       @Bean
@@ -198,9 +198,16 @@ public FilterRegistrationBean<SentryTracingFilter> sentryTracingFilter(
           final @NotNull IHub hub) {
         FilterRegistrationBean<SentryTracingFilter> filter =
             new FilterRegistrationBean<>(new SentryTracingFilter(hub));
-        filter.setOrder(Ordered.HIGHEST_PRECEDENCE);
+        filter.setOrder(SENTRY_SPRING_FILTER_PRECEDENCE + 1); // must run after SentrySpringFilter
         return filter;
       }
+
+      @Bean
+      @ConditionalOnMissingBean
+      public @NotNull SentryExceptionResolver sentryExceptionResolver(
+          final @NotNull IHub sentryHub, final @NotNull SentryProperties options) {
+        return new SentryExceptionResolver(sentryHub, options.getExceptionResolverOrder());
+      }
     }
 
     @Configuration(proxyBeanMethods = false)

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

@@ -33,6 +33,7 @@ import org.springframework.http.HttpEntity
 import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpMethod
 import org.springframework.http.HttpStatus
+import org.springframework.http.MediaType
 import org.springframework.http.ResponseEntity
 import org.springframework.security.config.annotation.web.builders.HttpSecurity
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter
@@ -47,13 +48,14 @@ import org.springframework.test.context.junit4.SpringRunner
 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.RestController
 
 @RunWith(SpringRunner::class)
 @SpringBootTest(
     classes = [App::class],
     webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
-    properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.enable-tracing=true", "sentry.traces-sample-rate=1.0"]
+    properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.enable-tracing=true", "sentry.traces-sample-rate=1.0", "sentry.max-request-body-size=medium"]
 )
 class SentrySpringIntegrationTest {
 
@@ -91,6 +93,23 @@ 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/body", HttpMethod.POST, httpEntity, Void::class.java)
+
+        await.untilAsserted {
+            verify(transport).send(checkEvent { event ->
+                assertThat(event.request).isNotNull()
+                assertThat(event.request!!.data).isEqualTo("""{"body":"content"}""")
+            }, anyOrNull())
+        }
+    }
+
     @Test
     fun `attaches first ip address if multiple addresses exist in a header`() {
         val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
@@ -227,6 +246,11 @@ class HelloController(private val helloService: HelloService) {
     fun logging() {
         logger.error("event from logger")
     }
+
+    @PostMapping("/body")
+    fun body() {
+        Sentry.captureMessage("body")
+    }
 }
 
 @Service

sentry-spring/api/sentry-spring.api

@@ -6,6 +6,7 @@ public final class io/sentry/spring/BuildConfig {
 public abstract interface annotation class io/sentry/spring/EnableSentry : java/lang/annotation/Annotation {
 	public abstract fun dsn ()Ljava/lang/String;
 	public abstract fun exceptionResolverOrder ()I
+	public abstract fun maxRequestBodySize ()Lio/sentry/SentryOptions$RequestSize;
 	public abstract fun sendDefaultPii ()Z
 }
 
@@ -42,6 +43,13 @@ public class io/sentry/spring/SentryRequestResolver {
 	public fun resolveSentryRequest (Ljavax/servlet/http/HttpServletRequest;)Lio/sentry/protocol/Request;
 }
 
+public class io/sentry/spring/SentrySpringFilter : org/springframework/web/filter/OncePerRequestFilter {
+	public fun <init> ()V
+	public fun <init> (Lio/sentry/IHub;)V
+	public fun <init> (Lio/sentry/IHub;Lio/sentry/spring/SentryRequestResolver;)V
+	protected fun doFilterInternal (Ljavax/servlet/http/HttpServletRequest;Ljavax/servlet/http/HttpServletResponse;Ljavax/servlet/FilterChain;)V
+}
+
 public class io/sentry/spring/SentrySpringRequestListener : javax/servlet/ServletRequestListener, org/springframework/core/Ordered {
 	public fun <init> ()V
 	public fun <init> (Lio/sentry/IHub;Lio/sentry/spring/SentryRequestResolver;)V

sentry-spring/src/main/java/io/sentry/spring/CachedBodyHttpServletRequest.java

@@ -0,0 +1,34 @@
+package io.sentry.spring;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import javax.servlet.ServletInputStream;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+import org.jetbrains.annotations.NotNull;
+import org.springframework.util.StreamUtils;
+
+final class CachedBodyHttpServletRequest extends HttpServletRequestWrapper {
+
+  private final @NotNull byte[] cachedBody;
+
+  public CachedBodyHttpServletRequest(final @NotNull HttpServletRequest request)
+      throws IOException {
+    super(request);
+    this.cachedBody = StreamUtils.copyToByteArray(request.getInputStream());
+  }
+
+  @Override
+  public @NotNull ServletInputStream getInputStream() {
+    return new CachedBodyServletInputStream(this.cachedBody);
+  }
+
+  @Override
+  public @NotNull BufferedReader getReader() {
+    return new BufferedReader(
+        new InputStreamReader(new ByteArrayInputStream(this.cachedBody), StandardCharsets.UTF_8));
+  }
+}

sentry-spring/src/main/java/io/sentry/spring/CachedBodyServletInputStream.java

@@ -0,0 +1,48 @@
+package io.sentry.spring;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import javax.servlet.ReadListener;
+import javax.servlet.ServletInputStream;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+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/src/main/java/io/sentry/spring/EnableSentry.java

@@ -1,5 +1,6 @@
 package io.sentry.spring;
 
+import io.sentry.SentryOptions;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -12,8 +13,6 @@
  * <ul>
  *   <li>creates bean of type {@link io.sentry.SentryOptions}
  *   <li>registers {@link io.sentry.IHub} for sending Sentry events
- *   <li>registers {@link SentrySpringRequestListener} for attaching request information to Sentry
- *       events
  *   <li>registers {@link SentryExceptionResolver} to send Sentry event for any uncaught exception
  *       in Spring MVC flow.
  * </ul>
@@ -44,4 +43,10 @@
    * @return the order to use for {@link SentryExceptionResolver}
    */
   int exceptionResolverOrder() default 1;
+
+  /**
+   * Controls the size of the request body to extract if any. No truncation is done by the SDK. If
+   * the request body is larger than the accepted size, nothing is sent.
+   */
+  SentryOptions.RequestSize maxRequestBodySize() default SentryOptions.RequestSize.NONE;
 }

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

@@ -0,0 +1,30 @@
+package io.sentry.spring;
+
+import io.sentry.SentryLevel;
+import io.sentry.SentryOptions;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import javax.servlet.http.HttpServletRequest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.springframework.util.StreamUtils;
+
+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) {
+      try {
+        final byte[] body = StreamUtils.copyToByteArray(request.getInputStream());
+        return new String(body, StandardCharsets.UTF_8);
+      } catch (IOException e) {
+        options.getLogger().log(SentryLevel.ERROR, "Failed to set request body", e);
+        return null;
+      }
+    } else {
+      return null;
+    }
+  }
+}

sentry-spring/src/main/java/io/sentry/spring/SentryHubRegistrar.java

@@ -47,6 +47,10 @@ private void registerSentryOptions(
     if (annotationAttributes.containsKey("sendDefaultPii")) {
       builder.addPropertyValue("sendDefaultPii", annotationAttributes.getBoolean("sendDefaultPii"));
     }
+    if (annotationAttributes.containsKey("maxRequestBodySize")) {
+      builder.addPropertyValue(
+          "maxRequestBodySize", annotationAttributes.get("maxRequestBodySize"));
+    }
 
     registry.registerBeanDefinition("sentryOptions", builder.getBeanDefinition());
   }