Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add request body extraction for Spring MVC integration #1595

Merged
merged 25 commits into from
Jul 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fcae2d6
Request Body WIP.
maciejwalkowiak Jul 8, 2021
833cb86
Deprecate SentrySpringRequestListener.
maciejwalkowiak Jul 12, 2021
0047ab4
Cleanup.
maciejwalkowiak Jul 12, 2021
27121ad
Polish.
maciejwalkowiak Jul 12, 2021
0c0fc14
Polish.
maciejwalkowiak Jul 12, 2021
39034b2
Cache request only if request matches conditions.
maciejwalkowiak Jul 12, 2021
836fbf0
Polish.
maciejwalkowiak Jul 12, 2021
7c65064
Polish.
maciejwalkowiak Jul 12, 2021
41cb714
Enable capturing request body in samples.
maciejwalkowiak Jul 13, 2021
cebef50
Refactoring.
maciejwalkowiak Jul 13, 2021
ed75beb
Add the ability to overwrite `sentrySpringFilter`.
maciejwalkowiak Jul 13, 2021
65f5fd8
Javadocs.
maciejwalkowiak Jul 13, 2021
288677a
Changelog.
maciejwalkowiak Jul 13, 2021
996d462
Merge branch 'main' into gh-1334
maciejwalkowiak Jul 13, 2021
49b17b4
Merge branch 'main' into gh-1334
bruno-garcia Jul 14, 2021
4d30a62
Extract request body in event processor to not use memory when not ne…
maciejwalkowiak Jul 14, 2021
c71230f
Ensure filter order.
maciejwalkowiak Jul 14, 2021
e6779ba
Merge branch 'gh-1334' of https://github.com/getsentry/sentry-java in…
maciejwalkowiak Jul 14, 2021
296312b
Ensure filter order.
maciejwalkowiak Jul 14, 2021
52a79ab
Send request body only if sendDefaultPii is set to true
maciejwalkowiak Jul 15, 2021
d6c30b3
Merge remote-tracking branch 'origin/main' into gh-1334
maciejwalkowiak Jul 19, 2021
e60c764
Support reading `maxRequestBodySize` from external properties.
maciejwalkowiak Jul 19, 2021
8d89b0d
Support reading `maxRequestBodySize` from external properties.
maciejwalkowiak Jul 19, 2021
989a208
Spotless.
maciejwalkowiak Jul 19, 2021
6d9ec37
Do not set body on transactions.
maciejwalkowiak Jul 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions sentry-spring/api/sentry-spring.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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>
Expand Down Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
@@ -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());
bruno-garcia marked this conversation as resolved.
Show resolved Hide resolved
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Loading