diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java index 72d20481c5..ceb0edaaa5 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java @@ -42,7 +42,7 @@ public class Request implements Recyclable { - private final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, + private static final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, new Allocator() { @Override public CharBuffer createInstance() { @@ -71,6 +71,11 @@ public void recycle(CharBuffer object) { * A parsed key-value object of cookies */ private final PotentiallyMultiValuedMap cookies = new PotentiallyMultiValuedMap(); + /** + * Data should only contain the request body (not the query string). It can either be a dictionary (for standard HTTP requests) or a raw request body. + */ + @Nullable + private String rawBody; /** * HTTP version. */ @@ -84,6 +89,7 @@ public void recycle(CharBuffer object) { private String method; @Nullable private CharBuffer bodyBuffer; + private boolean bodyBufferFinished = false; /** * Data should only contain the request body (not the query string). It can either be a dictionary (for standard HTTP requests) or a raw request body. @@ -92,16 +98,34 @@ public void recycle(CharBuffer object) { public Object getBody() { if (!postParams.isEmpty()) { return postParams; + } else if (rawBody != null) { + return rawBody; } else { return bodyBuffer; } } - public void redactBody() { + @Nullable + public String getRawBody() { + return rawBody; + } + + /** + * Sets the body as a raw string and removes any previously set {@link #postParams} or {@link #bodyBuffer}. + * + * @param rawBody the body as a raw string + */ + public void setRawBody(String rawBody) { postParams.resetState(); if (bodyBuffer != null) { - bodyBuffer.clear().append("[REDACTED]").flip(); + charBufferPool.recycle(bodyBuffer); + bodyBuffer = null; } + this.rawBody = rawBody; + } + + public void redactBody() { + setRawBody("[REDACTED]"); } public Request addFormUrlEncodedParameter(String key, String value) { @@ -132,6 +156,13 @@ public CharBuffer withBodyBuffer() { return this.bodyBuffer; } + public void endOfBufferInput() { + if (bodyBuffer != null && !bodyBufferFinished) { + bodyBufferFinished = true; + ((Buffer) bodyBuffer).flip(); + } + } + /** * Returns the associated pooled {@link CharBuffer} to record the request body. *

@@ -142,6 +173,15 @@ public CharBuffer withBodyBuffer() { */ @Nullable public CharBuffer getBodyBuffer() { + if (!bodyBufferFinished) { + return bodyBuffer; + } else { + return null; + } + } + + @Nullable + public CharBuffer getBodyBufferForSerialization() { return bodyBuffer; } @@ -231,6 +271,10 @@ public PotentiallyMultiValuedMap getCookies() { return cookies; } + void onTransactionEnd() { + endOfBufferInput(); + } + @Override public void resetState() { postParams.resetState(); @@ -240,10 +284,11 @@ public void resetState() { socket.resetState(); url.resetState(); cookies.resetState(); + bodyBufferFinished = false; if (bodyBuffer != null) { charBufferPool.recycle(bodyBuffer); + bodyBuffer = null; } - bodyBuffer = null; } public void copyFrom(Request other) { @@ -255,12 +300,12 @@ public void copyFrom(Request other) { this.url.copyFrom(other.url); this.cookies.copyFrom(other.cookies); if (other.bodyBuffer != null) { - final CharBuffer otherBuffer = other.getBodyBuffer(); + final CharBuffer otherBuffer = other.bodyBuffer; final CharBuffer thisBuffer = this.withBodyBuffer(); for (int i = 0; i < otherBuffer.length(); i++) { thisBuffer.append(otherBuffer.charAt(i)); } - thisBuffer.flip(); + ((Buffer) thisBuffer).flip(); } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java index c46bc61242..2249508966 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java @@ -92,4 +92,8 @@ public void resetState() { request.resetState(); user.resetState(); } + + public void onTransactionEnd() { + request.onTransactionEnd(); + } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java index 4a1ae4323b..bb1d7077ec 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java @@ -38,7 +38,7 @@ */ public class Db implements Recyclable { - private final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, + private static final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, new Allocator() { @Override public CharBuffer createInstance() { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java index b52922c2ba..43237e6672 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java @@ -171,6 +171,7 @@ public void doEnd(long epochMicros) { if (type == null) { type = "custom"; } + context.onTransactionEnd(); this.tracer.endTransaction(this); } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java index 6198ebd045..6ea0eae734 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java @@ -128,6 +128,18 @@ public static WildcardMatcher valueOf(final String wildcardString) { return new CompoundWildcardMatcher(wildcardString, matcher, matchers); } + /** + * Returns the first {@link WildcardMatcher} {@linkplain WildcardMatcher#matches(String) matching} the provided string. + * + * @param matchers the matchers which should be used to match the provided string + * @param s the string to match against + * @return the first matching {@link WildcardMatcher}, or {@code null} if none match. + */ + @Nullable + public static boolean isAnyMatch(List matchers, @Nullable String s) { + return anyMatch(matchers, s) != null; + } + /** * Returns {@code true}, if any of the matchers match the provided string. * @@ -136,17 +148,20 @@ public static WildcardMatcher valueOf(final String wildcardString) { * @return {@code true}, if any of the matchers match the provided string */ @Nullable - public static WildcardMatcher anyMatch(List matchers, String s) { + public static WildcardMatcher anyMatch(List matchers, @Nullable String s) { + if (s == null) { + return null; + } return anyMatch(matchers, s, null); } /** - * Returns {@code true}, if any of the matchers match the provided partitioned string. + * Returns the first {@link WildcardMatcher} {@linkplain WildcardMatcher#matches(String) matching} the provided partitioned string. * * @param matchers the matchers which should be used to match the provided string * @param firstPart The first part of the string to match against. * @param secondPart The second part of the string to match against. - * @return {@code true}, if any of the matchers match the provided partitioned string + * @return the first matching {@link WildcardMatcher}, or {@code null} if none match. * @see #matches(String, String) */ @Nullable diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java index 3d8ae2150a..2086cd50e8 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java @@ -816,10 +816,15 @@ private void serializeRequest(final Request request) { // only one of those can be non-empty if (!request.getFormUrlEncodedParameters().isEmpty()) { writeField("body", request.getFormUrlEncodedParameters()); - } else if (request.getBodyBuffer() != null && request.getBodyBuffer().length() > 0) { - writeFieldName("body"); - jw.writeString(request.getBodyBuffer()); - jw.writeByte(COMMA); + } else if (request.getRawBody() != null) { + writeField("body", request.getRawBody()); + } else { + final CharBuffer bodyBuffer = request.getBodyBufferForSerialization(); + if (bodyBuffer != null && bodyBuffer.length() > 0) { + writeFieldName("body"); + jw.writeString(bodyBuffer); + jw.writeByte(COMMA); + } } if (request.getUrl().hasContent()) { serializeUrl(request.getUrl()); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java new file mode 100644 index 0000000000..79a34c5625 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java @@ -0,0 +1,125 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package co.elastic.apm.agent.servlet; + +import co.elastic.apm.agent.bci.ElasticApmInstrumentation; +import co.elastic.apm.agent.bci.HelperClassManager; +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.impl.transaction.Transaction; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.NamedElement; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +import javax.annotation.Nullable; +import javax.servlet.ServletInputStream; +import java.util.Arrays; +import java.util.Collection; + +import static co.elastic.apm.agent.servlet.ServletInstrumentation.SERVLET_API; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +public class RequestStreamRecordingInstrumentation extends ElasticApmInstrumentation { + + @Nullable + @VisibleForAdvice + // referring to InputStreamWrapperFactory is legal because of type erasure + public static HelperClassManager wrapperHelperClassManager; + + @Override + public void init(ElasticApmTracer tracer) { + wrapperHelperClassManager = HelperClassManager.ForSingleClassLoader.of(tracer, + "co.elastic.apm.agent.servlet.helper.InputStreamFactoryHelperImpl", + "co.elastic.apm.agent.servlet.helper.RecordingServletInputStreamWrapper"); + } + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameContains("Request"); + } + + @Override + public ElementMatcher getTypeMatcher() { + return hasSuperType(named("javax.servlet.ServletRequest")).and(not(isInterface())); + } + + @Override + public ElementMatcher getMethodMatcher() { + return named("getInputStream").and(returns(named("javax.servlet.ServletInputStream"))); + } + + @Override + public Collection getInstrumentationGroupNames() { + return Arrays.asList(SERVLET_API, "servlet-input-stream"); + } + + @Override + public Class getAdviceClass() { + return GetInputStreamAdvice.class; + } + + public interface InputStreamWrapperFactory { + ServletInputStream wrap(Request request, ServletInputStream servletInputStream); + } + + public static class GetInputStreamAdvice { + + @VisibleForAdvice + public static final ThreadLocal nestedThreadLocal = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onReadEnter(@Advice.This Object thiz, + @Advice.Local("transaction") Transaction transaction, + @Advice.Local("nested") boolean nested) { + nested = nestedThreadLocal.get(); + nestedThreadLocal.set(Boolean.TRUE); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void afterGetInputStream(@Advice.Return(readOnly = false) ServletInputStream inputStream, + @Advice.Local("nested") boolean nested) { + if (nested || tracer == null || wrapperHelperClassManager == null) { + return; + } + try { + final Transaction transaction = tracer.currentTransaction(); + // only wrap if the body buffer has been initialized via ServletTransactionHelper.startCaptureBody + if (transaction != null && transaction.getContext().getRequest().getBodyBuffer() != null) { + inputStream = wrapperHelperClassManager.getForClassLoaderOfClass(inputStream.getClass()).wrap(transaction.getContext().getRequest(), inputStream); + } + } finally { + nestedThreadLocal.set(Boolean.FALSE); + } + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index ecb856069a..f18f5fddeb 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -117,7 +117,7 @@ public static void onEnterServletService(@Advice.Argument(0) ServletRequest serv servletTransactionHelper.fillRequestContext(transaction, request.getProtocol(), request.getMethod(), request.isSecure(), request.getScheme(), request.getServerName(), request.getServerPort(), request.getRequestURI(), request.getQueryString(), - request.getRemoteAddr()); + request.getRemoteAddr(), request.getHeader("Content-Type")); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index c731d02576..6633c445fe 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -55,6 +55,7 @@ public class ServletTransactionHelper { public static final String TRANSACTION_ATTRIBUTE = ServletApiAdvice.class.getName() + ".transaction"; @VisibleForAdvice public static final String ASYNC_ATTRIBUTE = ServletApiAdvice.class.getName() + ".async"; + private static final String CONTENT_TYPE_FROM_URLENCODED = "application/x-www-form-urlencoded"; private final Logger logger = LoggerFactory.getLogger(ServletTransactionHelper.class); @@ -111,12 +112,30 @@ public Transaction onBefore(ClassLoader classLoader, String servletPath, @Nullab @VisibleForAdvice public void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, String scheme, String serverName, int serverPort, String requestURI, String queryString, - String remoteAddr) { + String remoteAddr, @Nullable String contentTypeHeader) { final Request request = transaction.getContext().getRequest(); + startCaptureBody(transaction, method, contentTypeHeader); fillRequest(request, protocol, method, secure, scheme, serverName, serverPort, requestURI, queryString, remoteAddr); } + + private void startCaptureBody(Transaction transaction, String method, @Nullable String contentTypeHeader) { + Request request = transaction.getContext().getRequest(); + if (hasBody(contentTypeHeader, method)) { + if (webConfiguration.getCaptureBody() != OFF + && contentTypeHeader != null + // form parameters are recorded via ServletRequest.getParameterMap + // as the container might not call ServletRequest.getInputStream + && !contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED) + && WildcardMatcher.isAnyMatch(webConfiguration.getCaptureContentTypes(), contentTypeHeader)) { + request.withBodyBuffer(); + } else { + request.redactBody(); + } + } + } + @VisibleForAdvice public static void setUsernameIfUnset(@Nullable String userName, TransactionContext context) { // only set username if not manually set @@ -182,9 +201,7 @@ private void fillRequestParameters(Transaction transaction, String method, @Null Request request = transaction.getContext().getRequest(); if (hasBody(contentTypeHeader, method)) { if (webConfiguration.getCaptureBody() != OFF && parameterMap != null) { - captureBody(request, parameterMap, contentTypeHeader); - } else { - request.redactBody(); + captureParameters(request, parameterMap, contentTypeHeader); } } } @@ -192,9 +209,10 @@ private void fillRequestParameters(Transaction transaction, String method, @Null @VisibleForAdvice public boolean captureParameters(String method, @Nullable String contentTypeHeader) { return contentTypeHeader != null - && contentTypeHeader.startsWith("application/x-www-form-urlencoded") + && contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED) && hasBody(contentTypeHeader, method) - && webConfiguration.getCaptureBody() != OFF; + && webConfiguration.getCaptureBody() != OFF + && WildcardMatcher.isAnyMatch(webConfiguration.getCaptureContentTypes(), contentTypeHeader); } private boolean isExcluded(String servletPath, @Nullable String pathInfo, @Nullable String userAgentHeader) { @@ -242,14 +260,11 @@ private boolean hasBody(@Nullable String contentTypeHeader, String method) { return METHODS_WITH_BODY.contains(method) && contentTypeHeader != null; } - private void captureBody(Request request, Map params, @Nullable String contentTypeHeader) { - if (contentTypeHeader != null && contentTypeHeader.startsWith("application/x-www-form-urlencoded")) { + private void captureParameters(Request request, Map params, @Nullable String contentTypeHeader) { + if (contentTypeHeader != null && contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED)) { for (Map.Entry param : params.entrySet()) { request.addFormUrlEncodedParameters(param.getKey(), param.getValue()); } - } else { - // this content-type is not supported (yet) - request.redactBody(); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java new file mode 100644 index 0000000000..170074997c --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java @@ -0,0 +1,32 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package co.elastic.apm.agent.servlet.helper; + +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation; + +import javax.servlet.ServletInputStream; + +public class InputStreamFactoryHelperImpl implements RequestStreamRecordingInstrumentation.InputStreamWrapperFactory { + @Override + public ServletInputStream wrap(Request request, ServletInputStream servletInputStream) { + return new RecordingServletInputStreamWrapper(request, servletInputStream); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java new file mode 100644 index 0000000000..9db1eec842 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java @@ -0,0 +1,190 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package co.elastic.apm.agent.servlet.helper; + +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.util.IOUtils; + +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import java.io.IOException; +import java.nio.CharBuffer; +import java.nio.charset.CoderResult; + +@VisibleForAdvice +public class RecordingServletInputStreamWrapper extends ServletInputStream { + + private final Request request; + private final ServletInputStream servletInputStream; + + @VisibleForAdvice + public RecordingServletInputStreamWrapper(Request request, ServletInputStream servletInputStream) { + this.request = request; + this.servletInputStream = servletInputStream; + } + + @Override + public int read() throws IOException { + try { + final int b = servletInputStream.read(); + decode(b); + return b; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public int read(byte[] b) throws IOException { + try { + final int read; + read = servletInputStream.read(b); + decode(b, 0, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + try { + final int read = servletInputStream.read(b, off, len); + decode(b, off, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public int readLine(byte[] b, int off, int len) throws IOException { + try { + final int read = servletInputStream.readLine(b, off, len); + decode(b, off, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public byte[] readAllBytes() throws IOException { + try { + final byte[] bytes = servletInputStream.readAllBytes(); + decode(bytes, 0, bytes.length); + return bytes; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public void close() throws IOException { + try { + servletInputStream.close(); + } finally { + request.endOfBufferInput(); + } + } + + @Override + public long skip(long n) throws IOException { + return servletInputStream.skip(n); + } + + @Override + public int available() throws IOException { + return servletInputStream.available(); + } + + @Override + public void mark(int readlimit) { + servletInputStream.mark(readlimit); + } + + @Override + public void reset() throws IOException { + try { + servletInputStream.reset(); + } finally { + // don't read things twice from the stream, assume we have read everything by now + request.endOfBufferInput(); + } + } + + @Override + public boolean markSupported() { + return servletInputStream.markSupported(); + } + + @Override + public boolean isFinished() { + return servletInputStream.isFinished(); + } + + @Override + public boolean isReady() { + return servletInputStream.isReady(); + } + + @Override + public void setReadListener(ReadListener readListener) { + servletInputStream.setReadListener(readListener); + } + + private void decode(byte[] b, int off, int read) { + if (read == -1) { + request.endOfBufferInput(); + } else { + final CharBuffer bodyBuffer = request.getBodyBuffer(); + if (bodyBuffer != null) { + final CoderResult coderResult = IOUtils.decodeUtf8Bytes(b, off, read, bodyBuffer); + handleCoderResult(coderResult); + } + } + } + + private void decode(int b) { + if (b == -1) { + request.endOfBufferInput(); + } else { + final CharBuffer bodyBuffer = request.getBodyBuffer(); + if (bodyBuffer != null) { + final CoderResult coderResult = IOUtils.decodeUtf8Byte((byte) b, bodyBuffer); + handleCoderResult(coderResult); + } + } + } + + private void handleCoderResult(CoderResult coderResult) { + if (coderResult.isError()) { + request.setRawBody("[Non UTF-8 data]"); + } else if (coderResult.isOverflow()) { + request.endOfBufferInput(); + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation index 661ea5b24b..6cf18d9f78 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation @@ -3,3 +3,4 @@ co.elastic.apm.agent.servlet.FilterChainInstrumentation co.elastic.apm.agent.servlet.AsyncInstrumentation$StartAsyncInstrumentation co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation co.elastic.apm.agent.servlet.ServletContextServiceNameInstrumentation +co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java index b5302e4a15..a61fae6ca6 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java @@ -94,21 +94,6 @@ void testURLTransaction() throws IOException, ServletException { assertThat(url.getFull().toString()).isEqualTo("http://localhost/foo/bar?foo=bar"); } - @Test - void testTrackPostParams() throws IOException, ServletException { - when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo/bar"); - request.addParameter("foo", "bar"); - request.addParameter("baz", "qux", "quux"); - request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=uft-8"); - - filterChain.doFilter(request, new MockHttpServletResponse()); - assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isInstanceOf(PotentiallyMultiValuedMap.class); - PotentiallyMultiValuedMap params = (PotentiallyMultiValuedMap) reporter.getFirstTransaction().getContext().getRequest().getBody(); - assertThat(params.get("foo")).isEqualTo("bar"); - assertThat(params.get("baz")).isEqualTo(Arrays.asList("qux", "quux")); - } - @Test void captureException() { // we can't use mock(Servlet.class) here as the agent would instrument the created mock which confuses mockito diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java new file mode 100644 index 0000000000..70f181f064 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -0,0 +1,273 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package co.elastic.apm.agent.servlet; + +import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.impl.error.ErrorCapture; +import co.elastic.apm.agent.impl.transaction.TraceContext; +import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.report.serialize.DslJsonSerializer; +import co.elastic.apm.agent.util.PotentiallyMultiValuedMap; +import co.elastic.apm.agent.web.WebConfiguration; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.mock.web.MockFilterChain; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import javax.annotation.Nonnull; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +class TestRequestBodyCapturing extends AbstractInstrumentationTest { + + private static final byte[] BUFFER = new byte[1024]; + private InputStreamConsumer streamConsumer; + private InputStreamCloser streamCloser; + private WebConfiguration webConfiguration; + private MockFilterChain filterChain; + + static Stream streamConsumers() { + return Stream.of( + InputStream::read, + is -> is.read(BUFFER), + is -> is.read(BUFFER, 0, BUFFER.length), + is -> is.read(BUFFER, 42, BUFFER.length / 2), + is -> is.readLine(BUFFER, 0, BUFFER.length), + is -> { + is.readNBytes(BUFFER, 0, BUFFER.length); + return -1; + }, + is -> { + is.readAllBytes(); + return -1; + } + ); + } + + @BeforeEach + void setUp() { + webConfiguration = tracer.getConfig(WebConfiguration.class); + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); + filterChain = new MockFilterChain(new HttpServlet() { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final ServletInputStream is = req.getInputStream(); + int read; + do { + read = streamConsumer.read(is); + } while (read != -1); + streamCloser.close(is); + } + }); + streamConsumer = is -> is.readLine(BUFFER, 0, BUFFER.length); + streamCloser = InputStream::close; + + } + + @ParameterizedTest + @MethodSource("streamConsumers") + void testReadTextPlain(InputStreamConsumer consumer) throws Exception { + streamConsumer = consumer; + executeRequest(filterChain, "foo\nbar".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("foo\nbar"); + } + + @Test + void testCaptureBodyOff() throws Exception { + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.OFF); + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("[REDACTED]"); + } + + @ParameterizedTest + @ValueSource(strings = {"ALL", "TRANSACTIONS", "ERRORS"}) + void testCaptureBodyNotOff(WebConfiguration.EventType eventType) throws Exception { + streamCloser = is -> { throw new RuntimeException(); }; + + when(webConfiguration.getCaptureBody()).thenReturn(eventType); + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Transaction transaction = reporter.getFirstTransaction(); + final Object body = transaction.getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + // this is not [REDACTED] in this test as the BodyProcessor is not active in MockReporter + assertThat(body.toString()).isEqualTo("foo"); + + final ErrorCapture error = reporter.getFirstError(); + assertThat(error).isNotNull(); + assertThat(error.getContext().getRequest().getBody().toString()).isEqualTo("foo"); + } + + @Test + void testReadWithoutClose() throws Exception { + streamCloser = is -> {}; + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("foo"); + } + + @Test + void testReadLongText() throws Exception { + final byte[] longBody = RandomStringUtils.randomAlphanumeric(DslJsonSerializer.MAX_LONG_STRING_VALUE_LENGTH * 2).getBytes(StandardCharsets.UTF_8); + executeRequest(filterChain, longBody, "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString().length()).isEqualTo(DslJsonSerializer.MAX_LONG_STRING_VALUE_LENGTH); + } + + @Test + void testReadTextPlainNonUtf8() throws Exception { + executeRequest(filterChain, "foo{}".getBytes(StandardCharsets.UTF_16), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("[Non UTF-8 data]"); + } + + @Test + void testReadUnknownContentType() throws Exception { + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "application/unknown"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("[REDACTED]"); + } + + @Test + void testNotReading() throws Exception { + MockFilterChain filterChain = new MockFilterChain(new HttpServlet() { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) { + } + }); + + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo(""); + } + + @Test + void testReadOtherStream() throws Exception { + MockFilterChain filterChain = new MockFilterChain(new HttpServlet() { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + new ByteArrayInputStream("foo".getBytes(StandardCharsets.UTF_8)).read(new byte[42]); + } + }); + + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo(""); + } + + @Test + void testTrackPostParams() throws IOException, ServletException { + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo/bar"); + request.addParameter("foo", "bar"); + request.addParameter("baz", "qux", "quux"); + request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=uft-8"); + + filterChain.doFilter(request, new MockHttpServletResponse()); + assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isInstanceOf(PotentiallyMultiValuedMap.class); + PotentiallyMultiValuedMap params = (PotentiallyMultiValuedMap) reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(params.get("foo")).isEqualTo("bar"); + assertThat(params.get("baz")).isEqualTo(Arrays.asList("qux", "quux")); + } + + @Test + void testTrackPostParamsDisabled() throws IOException, ServletException { + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); + when(webConfiguration.getCaptureContentTypes()).thenReturn(Collections.emptyList()); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo/bar"); + request.addParameter("foo", "bar"); + request.addParameter("baz", "qux", "quux"); + request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=uft-8"); + + filterChain.doFilter(request, new MockHttpServletResponse()); + assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isEqualTo("[REDACTED]"); + } + + @Test + void testNoExplicitEndOfInput() { + final Transaction transaction = tracer.startTransaction(TraceContext.asRoot(), null, getClass().getClassLoader()); + transaction.getContext().getRequest().withBodyBuffer(); + transaction.end(); + assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody().toString()).isEqualTo(""); + } + + private void executeRequest(MockFilterChain filterChain, byte[] bytes, String contentType) throws IOException, ServletException { + try { + filterChain.doFilter(createMockRequest(bytes, contentType), new MockHttpServletResponse()); + } catch (Exception e) { + e.printStackTrace(); + } + } + + @Nonnull + private HttpServletRequest createMockRequest(byte[] bytes, String contentType) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo"); + request.setContent(bytes); + request.setContentType(contentType); + return new HttpServletRequestWrapper(request); + } + + interface InputStreamConsumer { + int read(ServletInputStream inputStream) throws IOException; + } + + interface InputStreamCloser { + void close(ServletInputStream inputStream) throws IOException; + } +} diff --git a/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java b/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java index 4026bbeb60..1d5d6def32 100644 --- a/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java +++ b/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java @@ -43,14 +43,34 @@ public class WebConfiguration extends ConfigurationOptionProvider { "\n" + "This option is case-insensitive.\n" + "\n" + - "NOTE: Currently, only `application/x-www-form-urlencoded` (form parameters) are supported.\n" + - "Forms which include a file upload (`multipart/form-data`) are not supported.\n" + + "NOTE: Currently, only UTF-8 encoded plain text content types are supported.\n" + + "The option <> determines which content types are captured.\n" + "\n" + - "WARNING: request bodies often contain sensitive values like passwords, credit card numbers etc." + - "If your service handles data like this, we advise to only enable this feature with care.") + "WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc.\n" + + "If your service handles data like this, we advise to only enable this feature with care.\n" + + "Turning on body capturing can also significantly increase the overhead in terms of heap usage,\n" + + "network utilisation and Elasticsearch index size.") .dynamic(true) .buildWithDefault(EventType.OFF); + private final ConfigurationOption> captureContentTypes = ConfigurationOption + .builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class) + .key("capture_body_content_types") + .configurationCategory(HTTP_CATEGORY) + .tags("added[1.5.0]", "performance") + .description("Configures which content types should be recorded.\n" + + "\n" + + "The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well.\n" + + "\n" + + WildcardMatcher.DOCUMENTATION) + .dynamic(true) + .buildWithDefault(Arrays.asList( + WildcardMatcher.valueOf("application/x-www-form-urlencoded*"), + WildcardMatcher.valueOf("text/*"), + WildcardMatcher.valueOf("application/json*"), + WildcardMatcher.valueOf("application/xml*") + )); + private final ConfigurationOption captureHeaders = ConfigurationOption.booleanOption() .key("capture_headers") .configurationCategory(HTTP_CATEGORY) @@ -157,6 +177,10 @@ public boolean isCaptureHeaders() { return captureHeaders.get(); } + public List getCaptureContentTypes() { + return captureContentTypes.get(); + } + public enum EventType { /** * Request bodies will never be reported diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 4a348c636a..2eeed79b61 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -72,6 +72,7 @@ Click on a key to get more information. ** <> * <> ** <> +** <> ** <> ** <> ** <> @@ -319,7 +320,7 @@ you should add an additional entry to this list (make sure to also include the d ==== `disable_instrumentations` A list of instrumentations which should be disabled. -Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. +Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. If you want to try out incubating features, set the value to an empty string. @@ -426,10 +427,13 @@ If the request has a body and this setting is disabled, the body will be shown a This option is case-insensitive. -NOTE: Currently, only `application/x-www-form-urlencoded` (form parameters) are supported. -Forms which include a file upload (`multipart/form-data`) are not supported. +NOTE: Currently, only UTF-8 encoded plain text content types are supported. +The option <> determines which content types are captured. -WARNING: request bodies often contain sensitive values like passwords, credit card numbers etc.If your service handles data like this, we advise to only enable this feature with care. +WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc. +If your service handles data like this, we advise to only enable this feature with care. +Turning on body capturing can also significantly increase the overhead in terms of heap usage, +network utilisation and Elasticsearch index size. Valid options: `off`, `errors`, `transactions`, `all` @@ -446,6 +450,33 @@ Valid options: `off`, `errors`, `transactions`, `all` | `elastic.apm.capture_body` | `capture_body` | `ELASTIC_APM_CAPTURE_BODY` |============ +[float] +[[config-capture-body-content-types]] +==== `capture_body_content_types` (added[1.5.0] performance) + +Configures which content types should be recorded. + +The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well. + +This option supports the wildcard `*`, which matches zero or more characters. +Examples: `/foo/*/bar/*/baz*`, `*foo*`. +Matching is case insensitive by default. +Prepending an element with `(?-i)` makes the matching case sensitive. + + +[options="header"] +|============ +| Default | Type | Dynamic +| `application/x-www-form-urlencoded*, text/*, application/json*, application/xml*` | List | true +|============ + + +[options="header"] +|============ +| Java System Properties | Property file | Environment +| `elastic.apm.capture_body_content_types` | `capture_body_content_types` | `ELASTIC_APM_CAPTURE_BODY_CONTENT_TYPES` +|============ + [float] [[config-capture-headers]] ==== `capture_headers` (performance) @@ -1173,7 +1204,7 @@ If the service name is set explicitly, it overrides all of the above. # sanitize_field_names=password,passwd,pwd,secret,*key,*token*,*session*,*credit*,*card*,authorization,set-cookie # A list of instrumentations which should be disabled. -# Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. +# Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. # If you want to try out incubating features, # set the value to an empty string. # @@ -1246,10 +1277,13 @@ If the service name is set explicitly, it overrides all of the above. # # This option is case-insensitive. # -# NOTE: Currently, only `application/x-www-form-urlencoded` (form parameters) are supported. -# Forms which include a file upload (`multipart/form-data`) are not supported. +# NOTE: Currently, only UTF-8 encoded plain text content types are supported. +# The option <> determines which content types are captured. # -# WARNING: request bodies often contain sensitive values like passwords, credit card numbers etc.If your service handles data like this, we advise to only enable this feature with care. +# WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc. +# If your service handles data like this, we advise to only enable this feature with care. +# Turning on body capturing can also significantly increase the overhead in terms of heap usage, +# network utilisation and Elasticsearch index size. # # Valid options: off, errors, transactions, all # This setting can be changed at runtime @@ -1258,6 +1292,21 @@ If the service name is set explicitly, it overrides all of the above. # # capture_body=OFF +# Configures which content types should be recorded. +# +# The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well. +# +# This option supports the wildcard `*`, which matches zero or more characters. +# Examples: `/foo/*/bar/*/baz*`, `*foo*`. +# Matching is case insensitive by default. +# Prepending an element with `(?-i)` makes the matching case sensitive. +# +# This setting can be changed at runtime +# Type: comma separated list +# Default value: application/x-www-form-urlencoded*,text/*,application/json*,application/xml* +# +# capture_body_content_types=application/x-www-form-urlencoded*,text/*,application/json*,application/xml* + # If set to `true`, # the agent will capture request and response headers, including cookies. # diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java index 11db400bed..6d5a8c9c85 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java @@ -122,6 +122,7 @@ protected AbstractServletContainerIntegrationTest(GenericContainer servletCon .withEnv("ELASTIC_APM_IGNORE_URLS", "/status*,/favicon.ico") .withEnv("ELASTIC_APM_REPORT_SYNC", "true") .withEnv("ELASTIC_APM_LOGGING_LOG_LEVEL", "DEBUG") + .withEnv("ELASTIC_APM_CAPTURE_BODY", "all") .withLogConsumer(new StandardOutLogConsumer().withPrefix(containerName)) .withExposedPorts(webPort) .withFileSystemBind(pathToJavaagent, "/elastic-apm-agent.jar") @@ -220,7 +221,7 @@ public void clearMockServerLog() { } public JsonNode assertTransactionReported(String pathToTest, int expectedResponseCode) { - final List reportedTransactions = getAllReported(500, this::getReportedTransactions, 1); + final List reportedTransactions = getAllReported(this::getReportedTransactions, 1); JsonNode transaction = reportedTransactions.iterator().next(); assertThat(transaction.get("context").get("request").get("url").get("pathname").textValue()).isEqualTo(pathToTest); assertThat(transaction.get("context").get("response").get("status_code").intValue()).isEqualTo(expectedResponseCode); @@ -248,23 +249,25 @@ public Response executeRequest(String pathToTest) throws IOException { } @Nonnull - public List getAllReported(int timeoutMs, Supplier> supplier, int expected) { + public List getAllReported(Supplier> supplier, int expected) { + long timeout = ENABLE_DEBUGGING ? 600_000 : 500; long start = System.currentTimeMillis(); List reportedTransactions; do { reportedTransactions = supplier.get(); - } while (reportedTransactions.size() != expected && System.currentTimeMillis() - start < timeoutMs); + } while (reportedTransactions.size() != expected && System.currentTimeMillis() - start < timeout); assertThat(reportedTransactions).hasSize(expected); return reportedTransactions; } @Nonnull - public List assertSpansTransactionId(int timeoutMs, Supplier> supplier, String transactionId) { + public List assertSpansTransactionId(Supplier> supplier, String transactionId) { + long timeout = ENABLE_DEBUGGING ? 600_000 : 500; long start = System.currentTimeMillis(); List reportedSpans; do { reportedSpans = supplier.get(); - } while (reportedSpans.size() == 0 && System.currentTimeMillis() - start < timeoutMs); + } while (reportedSpans.size() == 0 && System.currentTimeMillis() - start < timeout); assertThat(reportedSpans.size()).isGreaterThanOrEqualTo(1); for (JsonNode span : reportedSpans) { assertThat(span.get("transaction_id").textValue()).isEqualTo(transactionId); @@ -318,7 +321,7 @@ public boolean isExpectedStacktrace(String path) { return !path.equals("/async-start-servlet"); } - private String getBaseUrl() { + public String getBaseUrl() { return "http://" + servletContainer.getContainerIpAddress() + ":" + servletContainer.getMappedPort(webPort); } @@ -412,4 +415,8 @@ private void waitFor(String path) { .withStartupTimeout(Duration.ofMinutes(ENABLE_DEBUGGING ? 1_000 : 5)) .waitUntilReady(servletContainer); } + + public OkHttpClient getHttpClient() { + return httpClient; + } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java index aa9e8d070e..4f0f7e9ded 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java @@ -42,7 +42,6 @@ public JettyIT(final String version) { .withEnv("ELASTIC_APM_IGNORE_URLS", "/status*,/favicon.ico") .withEnv("ELASTIC_APM_REPORT_SYNC", "true") .withEnv("ELASTIC_APM_LOGGING_LOG_LEVEL", "DEBUG") - .withLogConsumer(new StandardOutLogConsumer().withPrefix("jetty")) .withExposedPorts(8080), "jetty-application", "/var/lib/jetty/webapps", diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java index 6a169bea47..25a3182373 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/WebSphereIT.java @@ -51,11 +51,6 @@ public static Iterable data() { return Arrays.asList(new Object[][]{{"8.5.5"}, {"webProfile7"}}); } - @Override - protected void enableDebugging(GenericContainer servletContainer) { - servletContainer.withEnv("JVM_ARGS", "-javaagent:/elastic-apm-agent.jar -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"); - } - @Override public boolean isExpectedStacktrace(String path) { return true; diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java index 1d4550b4db..8a839a8a00 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java @@ -51,7 +51,6 @@ private void testJsfRequest(AbstractServletContainerIntegrationTest containerInt assertThat(transaction.get("name").textValue()).isEqualTo(testedPath); String transactionId = transaction.get("id").textValue(); List spans = containerIntegrationTest.assertSpansTransactionId( - 500, containerIntegrationTest::getReportedSpans, transactionId); assertThat(spans.size()).isEqualTo(2); diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java index 6ce4fc203f..0053592286 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java @@ -21,6 +21,10 @@ import co.elastic.apm.servlet.AbstractServletContainerIntegrationTest; import com.fasterxml.jackson.databind.JsonNode; +import okhttp3.MediaType; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; import java.io.IOException; import java.util.List; @@ -40,6 +44,23 @@ public void test(AbstractServletContainerIntegrationTest test) throws Exception testSpanErrorReporting(test); testExecutorService(test); testHttpUrlConnection(test); + testCaptureBody(test); + } + + private void testCaptureBody(AbstractServletContainerIntegrationTest test) throws Exception { + for (String readMethod : List.of("read-byte", "read-bytes", "read-offset", "read-line")) { + test.clearMockServerLog(); + final Response response = test.getHttpClient().newCall(new Request.Builder() + .post(RequestBody.create(MediaType.parse("text/plain"), "{foo}\n{bar}")) + .url(test.getBaseUrl() + "/simple-webapp/echo?read-method=" + readMethod) + .build()) + .execute(); + assertThat(response.code()).isEqualTo(200); + assertThat(response.body().string()).isEqualTo("{foo}\n{bar}"); + + final JsonNode transaction = test.assertTransactionReported("/simple-webapp/echo", 200); + assertThat(transaction.get("context").get("request").get("body").textValue()).isEqualTo("{foo}\n{bar}"); + } } private void testExecutorService(AbstractServletContainerIntegrationTest test) throws Exception { @@ -47,7 +68,7 @@ private void testExecutorService(AbstractServletContainerIntegrationTest test) t final String pathToTest = "/simple-webapp/executor-service-servlet"; test.executeAndValidateRequest(pathToTest, null, 200); String transactionId = test.assertTransactionReported(pathToTest, 200).get("id").textValue(); - final List spans = test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + final List spans = test.assertSpansTransactionId(test::getReportedSpans, transactionId); assertThat(spans).hasSize(1); } @@ -56,11 +77,11 @@ private void testHttpUrlConnection(AbstractServletContainerIntegrationTest test) final String pathToTest = "/simple-webapp/http-url-connection"; test.executeAndValidateRequest(pathToTest, "Hello World!", 200); - final List reportedTransactions = test.getAllReported(500, test::getReportedTransactions, 2); + final List reportedTransactions = test.getAllReported(test::getReportedTransactions, 2); final JsonNode innerTransaction = reportedTransactions.get(0); final JsonNode outerTransaction = reportedTransactions.get(1); - final List spans = test.assertSpansTransactionId(500, test::getReportedSpans, outerTransaction.get("id").textValue()); + final List spans = test.assertSpansTransactionId(test::getReportedSpans, outerTransaction.get("id").textValue()); assertThat(spans).hasSize(1); final JsonNode span = spans.get(0); @@ -79,7 +100,7 @@ private void testTransactionReporting(AbstractServletContainerIntegrationTest te test.executeAndValidateRequest(pathToTest, "Hello World", 200); JsonNode transaction = test.assertTransactionReported(pathToTest, 200); String transactionId = transaction.get("id").textValue(); - List spans = test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + List spans = test.assertSpansTransactionId(test::getReportedSpans, transactionId); for (JsonNode span : spans) { assertThat(span.get("type").textValue()).isEqualTo("db.h2.query"); } @@ -93,7 +114,7 @@ private void testSpanErrorReporting(AbstractServletContainerIntegrationTest test test.executeAndValidateRequest(pathToTest + "?cause_db_error=true", "DB Error", 200); JsonNode transaction = test.assertTransactionReported(pathToTest, 200); String transactionId = transaction.get("id").textValue(); - test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + test.assertSpansTransactionId(test::getReportedSpans, transactionId); test.assertErrorContent(500, test::getReportedErrors, transactionId, "Column \"NON_EXISTING_COLUMN\" not found"); } } @@ -106,7 +127,7 @@ private void testTransactionErrorReporting(AbstractServletContainerIntegrationTe test.executeAndValidateRequest(fullPathToTest + "?cause_transaction_error=true", "", null); JsonNode transaction = test.assertTransactionReported(fullPathToTest, 500); String transactionId = transaction.get("id").textValue(); - test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + test.assertSpansTransactionId(test::getReportedSpans, transactionId); // we currently only report errors when Exceptions are caught, still this test is relevant for response code capturing if (test.isExpectedStacktrace(pathToTest)) { test.assertErrorContent(500, test::getReportedErrors, transactionId, "Transaction failure"); diff --git a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java new file mode 100644 index 0000000000..0e5b0d4a1e --- /dev/null +++ b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java @@ -0,0 +1,63 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package co.elastic.webapp; + +import javax.servlet.ServletInputStream; +import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +public class EchoServlet extends HttpServlet { + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + byte[] buffer = new byte[1024]; + int read; + final ServletOutputStream os = resp.getOutputStream(); + final ServletInputStream is = req.getInputStream(); + switch (req.getParameter("read-method")) { + case "read-byte": + while ((read = is.read()) > 0) { + os.write(read); + } + break; + case "read-bytes": + while ((read = is.read(buffer)) > 0) { + os.write(buffer, 0, read); + } + break; + case "read-offset": + while ((read = is.read(buffer, 42, buffer.length / 2)) > 0) { + os.write(buffer, 42, read); + } + break; + case "read-line": + while ((read = is.readLine(buffer, 0, buffer.length)) > 0) { + os.write(buffer, 0, read); + } + break; + default: + throw new IllegalArgumentException("Invalid read-method"); + } + resp.setContentType(req.getContentType()); + } +} diff --git a/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml b/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml index af0430b321..446ab6850a 100644 --- a/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml +++ b/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml @@ -60,4 +60,14 @@ /http-url-connection + + EchoServlet + co.elastic.webapp.EchoServlet + 1 + + + EchoServlet + /echo + +