diff --git a/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetPrintWriterTest.java b/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetPrintWriterTest.java index eedc842a5792..796665225deb 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetPrintWriterTest.java +++ b/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetPrintWriterTest.java @@ -26,8 +26,8 @@ void testInjectToTextHtml() throws IOException { String html = readFileAsString("beforeSnippetInjection.html"); InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); - SnippetInjectingResponseWrapper responseWrapper = - new SnippetInjectingResponseWrapper(response, snippet); + Servlet3SnippetInjectingResponseWrapper responseWrapper = + new Servlet3SnippetInjectingResponseWrapper(response, snippet); responseWrapper.getWriter().write(html); responseWrapper.getWriter().flush(); @@ -42,8 +42,8 @@ void testInjectToChineseTextHtml() throws IOException { String html = readFileAsString("beforeSnippetInjectionChinese.html"); InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); - SnippetInjectingResponseWrapper responseWrapper = - new SnippetInjectingResponseWrapper(response, snippet); + Servlet3SnippetInjectingResponseWrapper responseWrapper = + new Servlet3SnippetInjectingResponseWrapper(response, snippet); responseWrapper.getWriter().write(html); responseWrapper.getWriter().flush(); @@ -59,8 +59,8 @@ void shouldNotInjectToTextHtml() throws IOException { InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("not/text"); - SnippetInjectingResponseWrapper responseWrapper = - new SnippetInjectingResponseWrapper(response, snippet); + Servlet3SnippetInjectingResponseWrapper responseWrapper = + new Servlet3SnippetInjectingResponseWrapper(response, snippet); responseWrapper.getWriter().write(html); responseWrapper.getWriter().flush(); @@ -74,8 +74,8 @@ void testWriteInt() throws IOException { String html = readFileAsString("beforeSnippetInjection.html"); InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); - SnippetInjectingResponseWrapper responseWrapper = - new SnippetInjectingResponseWrapper(response, snippet); + Servlet3SnippetInjectingResponseWrapper responseWrapper = + new Servlet3SnippetInjectingResponseWrapper(response, snippet); byte[] originalBytes = html.getBytes(Charset.defaultCharset()); for (byte originalByte : originalBytes) { @@ -93,8 +93,8 @@ void testWriteCharArray() throws IOException { String html = readFileAsString("beforeSnippetInjectionChinese.html"); InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); - SnippetInjectingResponseWrapper responseWrapper = - new SnippetInjectingResponseWrapper(response, snippet); + Servlet3SnippetInjectingResponseWrapper responseWrapper = + new Servlet3SnippetInjectingResponseWrapper(response, snippet); char[] originalChars = html.toCharArray(); responseWrapper.getWriter().write(originalChars, 0, originalChars.length); @@ -112,8 +112,8 @@ void testWriteWithOffset() throws IOException { html = extraBuffer + html; InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); - SnippetInjectingResponseWrapper responseWrapper = - new SnippetInjectingResponseWrapper(response, snippet); + Servlet3SnippetInjectingResponseWrapper responseWrapper = + new Servlet3SnippetInjectingResponseWrapper(response, snippet); responseWrapper .getWriter() diff --git a/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetServletOutputStreamTest.java b/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetServletOutputStreamTest.java index 46018baba9eb..c1d712572a7f 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetServletOutputStreamTest.java +++ b/instrumentation/servlet/servlet-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetServletOutputStreamTest.java @@ -11,9 +11,12 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.OutputStreamSnippetInjectionHelper; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.Charset; +import java.util.function.Supplier; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; @@ -28,7 +31,9 @@ void testInjectionForStringContainHeadTag() throws IOException { InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); InMemoryServletOutputStream out = new InMemoryServletOutputStream(); - OutputStreamSnippetInjectionHelper helper = new OutputStreamSnippetInjectionHelper(snippet); + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); boolean injected = helper.handleWrite(obj, out, html, 0, html.length); assertThat(obj.getHeadTagBytesSeen()).isEqualTo(-1); assertThat(injected).isEqualTo(true); @@ -45,7 +50,9 @@ void testInjectionForChinese() throws IOException { InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); InMemoryServletOutputStream out = new InMemoryServletOutputStream(); - OutputStreamSnippetInjectionHelper helper = new OutputStreamSnippetInjectionHelper(snippet); + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); boolean injected = helper.handleWrite(obj, out, html, 0, html.length); byte[] expectedHtml = readFileAsBytes("afterSnippetInjectionChinese.html"); @@ -61,8 +68,9 @@ void testInjectionForStringWithoutHeadTag() throws IOException { InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); InMemoryServletOutputStream out = new InMemoryServletOutputStream(); - - OutputStreamSnippetInjectionHelper helper = new OutputStreamSnippetInjectionHelper(snippet); + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); boolean injected = helper.handleWrite(obj, out, html, 0, html.length); assertThat(injected).isFalse(); @@ -79,7 +87,9 @@ void testHeadTagSplitAcrossTwoWrites() throws IOException { InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); InMemoryServletOutputStream out = new InMemoryServletOutputStream(); - OutputStreamSnippetInjectionHelper helper = new OutputStreamSnippetInjectionHelper(snippet); + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); boolean injected = helper.handleWrite(obj, out, htmlFirstPartBytes, 0, htmlFirstPartBytes.length); @@ -119,8 +129,7 @@ private static InjectionState createInjectionStateForTesting(String snippet, Cha HttpServletResponse response = mock(HttpServletResponse.class); when(response.isCommitted()).thenReturn(false); when(response.getCharacterEncoding()).thenReturn(charset.name()); - - return new InjectionState(new SnippetInjectingResponseWrapper(response, snippet)); + return new InjectionState(new Servlet3SnippetInjectingResponseWrapper(response, snippet)); } private static class InMemoryServletOutputStream extends ServletOutputStream { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java index 9a9c44dedba3..1461bf8fd78f 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java @@ -16,7 +16,7 @@ import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge; import io.opentelemetry.javaagent.bootstrap.servlet.MappingResolver; import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext; -import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.SnippetInjectingResponseWrapper; +import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.Servlet3SnippetInjectingResponseWrapper; import javax.servlet.Servlet; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -46,8 +46,9 @@ public static void onEnter( String snippet = getSnippetInjectionHelper().getSnippet(); if (!snippet.isEmpty() && !((HttpServletResponse) response) - .containsHeader(SnippetInjectingResponseWrapper.FAKE_SNIPPET_HEADER)) { - response = new SnippetInjectingResponseWrapper((HttpServletResponse) response, snippet); + .containsHeader(Servlet3SnippetInjectingResponseWrapper.FAKE_SNIPPET_HEADER)) { + response = + new Servlet3SnippetInjectingResponseWrapper((HttpServletResponse) response, snippet); } callDepth = CallDepth.forClass(AppServerBridge.getCallDepthKey()); callDepth.getAndIncrement(); diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAdvice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAdvice.java index 8cf67e5166ae..bf524211a45d 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAdvice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAdvice.java @@ -7,7 +7,7 @@ import static io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3Singletons.getSnippetInjectionHelper; -import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.ServletOutputStreamInjectionState; import java.io.IOException; import javax.servlet.ServletOutputStream; diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAndOffsetAdvice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAndOffsetAdvice.java index 563693e795ad..22ed7cfc1618 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAndOffsetAdvice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAndOffsetAdvice.java @@ -7,7 +7,7 @@ import static io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3Singletons.getSnippetInjectionHelper; -import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.ServletOutputStreamInjectionState; import java.io.IOException; import javax.servlet.ServletOutputStream; diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteIntAdvice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteIntAdvice.java index a1e140376998..151e38621130 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteIntAdvice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteIntAdvice.java @@ -7,7 +7,7 @@ import static io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3Singletons.getSnippetInjectionHelper; -import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.ServletOutputStreamInjectionState; import java.io.IOException; import javax.servlet.ServletOutputStream; diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Singletons.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Singletons.java index 9a1a38f1b1e7..cadb80274913 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Singletons.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Singletons.java @@ -15,7 +15,7 @@ import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext; import io.opentelemetry.javaagent.instrumentation.servlet.ServletResponseContext; import io.opentelemetry.javaagent.instrumentation.servlet.common.response.ResponseInstrumenterFactory; -import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.OutputStreamSnippetInjectionHelper; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.OutputStreamSnippetInjectionHelper; import javax.servlet.Filter; import javax.servlet.Servlet; import javax.servlet.http.HttpServletRequest; diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/Servlet3SnippetInjectingResponseWrapper.java similarity index 88% rename from instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java rename to instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/Servlet3SnippetInjectingResponseWrapper.java index 1b2c7eafe092..d45b27648b4e 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/Servlet3SnippetInjectingResponseWrapper.java @@ -8,6 +8,8 @@ import static io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.ServletOutputStreamInjectionState.initializeInjectionStateIfNeeded; import static java.util.logging.Level.FINE; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.SnippetInjectingPrintWriter; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.SnippetInjectingResponseWrapper; import java.io.IOException; import java.io.PrintWriter; import java.lang.invoke.MethodHandle; @@ -29,9 +31,11 @@ * not been submitted, in which case it is likely using the real length of content that has been * written, including the snippet length. */ -public class SnippetInjectingResponseWrapper extends HttpServletResponseWrapper { +public class Servlet3SnippetInjectingResponseWrapper extends HttpServletResponseWrapper + implements SnippetInjectingResponseWrapper { - private static final Logger logger = Logger.getLogger(HttpServletResponseWrapper.class.getName()); + private static final Logger logger = + Logger.getLogger(Servlet3SnippetInjectingResponseWrapper.class.getName()); public static final String FAKE_SNIPPET_HEADER = "FAKE_SNIPPET_HEADER"; @@ -48,7 +52,7 @@ public class SnippetInjectingResponseWrapper extends HttpServletResponseWrapper private SnippetInjectingPrintWriter snippetInjectingPrintWriter = null; - public SnippetInjectingResponseWrapper(HttpServletResponse response, String snippet) { + public Servlet3SnippetInjectingResponseWrapper(HttpServletResponse response, String snippet) { super(response); this.snippet = snippet; snippetLength = snippet.length(); @@ -89,7 +93,7 @@ public void addHeader(String name, String value) { try { contentLength = Long.parseLong(value); } catch (NumberFormatException ex) { - logger.log(FINE, "NumberFormatException", ex); + logger.log(FINE, "Failed to parse the Content-Length header", ex); } } super.addHeader(name, value); @@ -143,7 +147,8 @@ public void setContentLengthLong(long length) throws Throwable { } } - boolean isContentTypeTextHtml() { + @Override + public boolean isContentTypeTextHtml() { String contentType = super.getContentType(); if (contentType == null) { contentType = super.getHeader("content-type"); @@ -170,13 +175,15 @@ public PrintWriter getWriter() throws IOException { return snippetInjectingPrintWriter; } - void updateContentLengthIfPreviouslySet() { + @Override + public void updateContentLengthIfPreviouslySet() { if (contentLength != UNSET) { setContentLength((int) contentLength + snippetLength); } } - boolean isNotSafeToInject() { + @Override + public boolean isNotSafeToInject() { // if content-length was set and response was already committed (headers sent to the client), // then it is not safe to inject because the content-length header cannot be updated to account // for the snippet length diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/ServletOutputStreamInjectionState.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/ServletOutputStreamInjectionState.java index 75bda0c2dfdd..26f338a123f6 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/ServletOutputStreamInjectionState.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/ServletOutputStreamInjectionState.java @@ -6,16 +6,16 @@ package io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet; import io.opentelemetry.instrumentation.api.util.VirtualField; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; import javax.annotation.Nullable; import javax.servlet.ServletOutputStream; public class ServletOutputStreamInjectionState { - private static final VirtualField virtualField = VirtualField.find(ServletOutputStream.class, InjectionState.class); public static void initializeInjectionStateIfNeeded( - ServletOutputStream servletOutputStream, SnippetInjectingResponseWrapper wrapper) { + ServletOutputStream servletOutputStream, Servlet3SnippetInjectingResponseWrapper wrapper) { InjectionState state = virtualField.get(servletOutputStream); if (!wrapper.isContentTypeTextHtml()) { virtualField.set(servletOutputStream, null); diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/java/RequestDispatcherServlet.java b/instrumentation/servlet/servlet-3.0/javaagent/src/test/java/RequestDispatcherServlet.java index 7d9b749a107b..63391a170868 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/java/RequestDispatcherServlet.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/java/RequestDispatcherServlet.java @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint; import java.io.IOException; import javax.servlet.RequestDispatcher; import javax.servlet.ServletContext; @@ -39,10 +38,10 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) ServletContext context = getServletContext(); RequestDispatcher dispatcher = context.getRequestDispatcher(target); // for HTML test case, set the content type before calling include because - // setContentType will be rejected if called inside of include - // check https://statics.teams.cdn.office.net/evergreen-assets/safelinks/1/atp-safelinks.html - if (ServerEndpoint.forPath(target) == ServerEndpoint.forPath("/htmlPrintWriter") - || ServerEndpoint.forPath(target) == ServerEndpoint.forPath("/htmlServletOutputStream")) { + // setContentType will be rejected if called inside include + // check + // https://docs.oracle.com/javaee/7/api/javax/servlet/RequestDispatcher.html#include-javax.servlet.ServletRequest-javax.servlet.ServletResponse- + if ("/htmlPrintWriter".equals(target) || "/htmlServletOutputStream".equals(target)) { resp.setContentType("text/html"); } dispatcher.include(req, resp); diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/build.gradle.kts b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/build.gradle.kts new file mode 100644 index 000000000000..f7df7d5f4bbb --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/build.gradle.kts @@ -0,0 +1,8 @@ +plugins { + id("otel.java-conventions") +} + +dependencies { + testImplementation("jakarta.servlet:jakarta.servlet-api:5.0.0") + testImplementation(project(":instrumentation:servlet:servlet-5.0:javaagent")) +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/SnippetPrintWriterTest.java b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/SnippetPrintWriterTest.java new file mode 100644 index 000000000000..ebe5adba05d6 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/SnippetPrintWriterTest.java @@ -0,0 +1,157 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpServletResponseWrapper; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.nio.charset.Charset; +import org.junit.jupiter.api.Test; + +class SnippetPrintWriterTest { + + @Test + void testInjectToTextHtml() throws IOException { + String snippet = "\n "; + String html = TestUtil.readFileAsString("beforeSnippetInjection.html"); + + InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); + Servlet5SnippetInjectingResponseWrapper responseWrapper = + new Servlet5SnippetInjectingResponseWrapper(response, snippet); + + responseWrapper.getWriter().write(html); + responseWrapper.getWriter().flush(); + + String expectedHtml = TestUtil.readFileAsString("afterSnippetInjection.html"); + assertThat(response.getStringContent()).isEqualTo(expectedHtml); + } + + @Test + void testInjectToChineseTextHtml() throws IOException { + String snippet = "\n "; + String html = TestUtil.readFileAsString("beforeSnippetInjectionChinese.html"); + + InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); + Servlet5SnippetInjectingResponseWrapper responseWrapper = + new Servlet5SnippetInjectingResponseWrapper(response, snippet); + + responseWrapper.getWriter().write(html); + responseWrapper.getWriter().flush(); + + String expectedHtml = TestUtil.readFileAsString("afterSnippetInjectionChinese.html"); + assertThat(response.getStringContent()).isEqualTo(expectedHtml); + } + + @Test + void shouldNotInjectToTextHtml() throws IOException { + String snippet = "\n "; + String html = TestUtil.readFileAsString("beforeSnippetInjection.html"); + + InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("not/text"); + + Servlet5SnippetInjectingResponseWrapper responseWrapper = + new Servlet5SnippetInjectingResponseWrapper(response, snippet); + + responseWrapper.getWriter().write(html); + responseWrapper.getWriter().flush(); + + assertThat(response.getStringContent()).isEqualTo(html); + } + + @Test + void testWriteInt() throws IOException { + String snippet = "\n "; + String html = TestUtil.readFileAsString("beforeSnippetInjection.html"); + + InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); + Servlet5SnippetInjectingResponseWrapper responseWrapper = + new Servlet5SnippetInjectingResponseWrapper(response, snippet); + + byte[] originalBytes = html.getBytes(Charset.defaultCharset()); + for (byte originalByte : originalBytes) { + responseWrapper.getWriter().write(originalByte); + } + responseWrapper.getWriter().flush(); + + String expectedHtml = TestUtil.readFileAsString("afterSnippetInjection.html"); + assertThat(response.getStringContent()).isEqualTo(expectedHtml); + } + + @Test + void testWriteCharArray() throws IOException { + String snippet = "\n "; + String html = TestUtil.readFileAsString("beforeSnippetInjectionChinese.html"); + + InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); + Servlet5SnippetInjectingResponseWrapper responseWrapper = + new Servlet5SnippetInjectingResponseWrapper(response, snippet); + + char[] originalChars = html.toCharArray(); + responseWrapper.getWriter().write(originalChars, 0, originalChars.length); + responseWrapper.getWriter().flush(); + + String expectedHtml = TestUtil.readFileAsString("afterSnippetInjectionChinese.html"); + assertThat(response.getStringContent()).isEqualTo(expectedHtml); + } + + @Test + void testWriteWithOffset() throws IOException { + String snippet = "\n "; + String html = TestUtil.readFileAsString("beforeSnippetInjectionChinese.html"); + String extraBuffer = "this buffer should not be print out"; + html = extraBuffer + html; + + InMemoryHttpServletResponse response = createInMemoryHttpServletResponse("text/html"); + Servlet5SnippetInjectingResponseWrapper responseWrapper = + new Servlet5SnippetInjectingResponseWrapper(response, snippet); + + responseWrapper + .getWriter() + .write(html, extraBuffer.length(), html.length() - extraBuffer.length()); + responseWrapper.getWriter().flush(); + + String expectedHtml = TestUtil.readFileAsString("afterSnippetInjectionChinese.html"); + assertThat(response.getStringContent()).isEqualTo(expectedHtml); + } + + private static InMemoryHttpServletResponse createInMemoryHttpServletResponse(String contentType) { + HttpServletResponse response = mock(HttpServletResponse.class); + when(response.getContentType()).thenReturn(contentType); + when(response.getStatus()).thenReturn(200); + when(response.containsHeader("content-type")).thenReturn(true); + return new InMemoryHttpServletResponse(response); + } + + private static class InMemoryHttpServletResponse extends HttpServletResponseWrapper { + + private PrintWriter printWriter; + private StringWriter stringWriter; + + InMemoryHttpServletResponse(HttpServletResponse delegate) { + super(delegate); + } + + @Override + public PrintWriter getWriter() { + if (printWriter == null) { + stringWriter = new StringWriter(); + printWriter = new PrintWriter(stringWriter); + } + return printWriter; + } + + String getStringContent() { + printWriter.flush(); + return stringWriter.toString(); + } + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/SnippetServletOutputStreamTest.java b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/SnippetServletOutputStreamTest.java new file mode 100644 index 000000000000..5d432516932c --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/SnippetServletOutputStreamTest.java @@ -0,0 +1,159 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet.TestUtil.readFileAsBytes; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.OutputStreamSnippetInjectionHelper; +import jakarta.servlet.ServletOutputStream; +import jakarta.servlet.WriteListener; +import jakarta.servlet.http.HttpServletResponse; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.function.Supplier; +import org.junit.jupiter.api.Test; + +class SnippetServletOutputStreamTest { + + @Test + void testInjectionForStringContainHeadTag() throws IOException { + String snippet = "\n "; + byte[] html = readFileAsBytes("beforeSnippetInjection.html"); + + InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); + InMemoryServletOutputStream out = new InMemoryServletOutputStream(); + + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); + boolean injected = helper.handleWrite(obj, out, html, 0, html.length); + assertThat(obj.getHeadTagBytesSeen()).isEqualTo(-1); + assertThat(injected).isEqualTo(true); + + byte[] expectedHtml = readFileAsBytes("afterSnippetInjection.html"); + assertThat(out.getBytes()).isEqualTo(expectedHtml); + } + + @Test + void testInjectionForChinese() throws IOException { + String snippet = "\n "; + byte[] html = readFileAsBytes("beforeSnippetInjectionChinese.html"); + + InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); + InMemoryServletOutputStream out = new InMemoryServletOutputStream(); + + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); + boolean injected = helper.handleWrite(obj, out, html, 0, html.length); + + byte[] expectedHtml = readFileAsBytes("afterSnippetInjectionChinese.html"); + assertThat(injected).isTrue(); + assertThat(obj.getHeadTagBytesSeen()).isEqualTo(-1); + assertThat(out.getBytes()).isEqualTo(expectedHtml); + } + + @Test + void testInjectionForStringWithoutHeadTag() throws IOException { + String snippet = "\n "; + byte[] html = readFileAsBytes("htmlWithoutHeadTag.html"); + + InjectionState obj = createInjectionStateForTesting(snippet, UTF_8); + InMemoryServletOutputStream out = new InMemoryServletOutputStream(); + + Supplier stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); + boolean injected = helper.handleWrite(obj, out, html, 0, html.length); + + assertThat(injected).isFalse(); + assertThat(obj.getHeadTagBytesSeen()).isEqualTo(0); + assertThat(out.getBytes()).isEmpty(); + } + + @Test + void testHeadTagSplitAcrossTwoWrites() throws IOException { + String snippet = "\n "; + String htmlFirstPart = "\n\n stringSupplier = snippet::toString; + OutputStreamSnippetInjectionHelper helper = + new OutputStreamSnippetInjectionHelper(stringSupplier); + boolean injected = + helper.handleWrite(obj, out, htmlFirstPartBytes, 0, htmlFirstPartBytes.length); + + assertThat(injected).isFalse(); + assertThat(obj.getHeadTagBytesSeen()).isEqualTo(3); + assertThat(out.getBytes()).isEmpty(); + + String htmlSecondPart = + "ad>\n" + + " \n" + + " Title\n" + + "\n" + + "\n" + + "\n" + + "\n" + + ""; + byte[] htmlSecondPartBytes = htmlSecondPart.getBytes(UTF_8); + injected = helper.handleWrite(obj, out, htmlSecondPartBytes, 0, htmlSecondPartBytes.length); + + assertThat(injected).isTrue(); + assertThat(obj.getHeadTagBytesSeen()).isEqualTo(-1); + + String expectedSecondPart = + "ad>\n" + + " \n" + + " \n" + + " Title\n" + + "\n" + + "\n" + + "\n" + + "\n" + + ""; + assertThat(out.getBytes()).isEqualTo(expectedSecondPart.getBytes(UTF_8)); + } + + private static InjectionState createInjectionStateForTesting(String snippet, Charset charset) { + HttpServletResponse response = mock(HttpServletResponse.class); + when(response.isCommitted()).thenReturn(false); + when(response.getCharacterEncoding()).thenReturn(charset.name()); + + return new InjectionState(new Servlet5SnippetInjectingResponseWrapper(response, snippet)); + } + + private static class InMemoryServletOutputStream extends ServletOutputStream { + + private final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + + public byte[] getBytes() { + return baos.toByteArray(); + } + + @Override + public boolean isReady() { + return false; + } + + @Override + public void setWriteListener(WriteListener writeListener) {} + + @Override + public void write(int b) throws IOException { + baos.write(b); + } + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/TestUtil.java b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/TestUtil.java new file mode 100644 index 000000000000..1b58b0c1e030 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/TestUtil.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; + +public class TestUtil { + + protected static byte[] readFileAsBytes(String resourceName) throws IOException { + InputStream in = + SnippetPrintWriterTest.class.getClassLoader().getResourceAsStream(resourceName); + ByteArrayOutputStream result = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int length; + while ((length = in.read(buffer)) != -1) { + result.write(buffer, 0, length); + } + return result.toByteArray(); + } + + protected static String readFileAsString(String resourceName) throws IOException { + return new String(readFileAsBytes(resourceName), UTF_8); + } + + private TestUtil() {} +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/afterSnippetInjection.html b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/afterSnippetInjection.html new file mode 100644 index 000000000000..ba5ff4ad3ac1 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/afterSnippetInjection.html @@ -0,0 +1,11 @@ + + + + + + Title + + + + + \ No newline at end of file diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/afterSnippetInjectionChinese.html b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/afterSnippetInjectionChinese.html new file mode 100644 index 000000000000..81166f54dd69 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/afterSnippetInjectionChinese.html @@ -0,0 +1,11 @@ + + + + + + Title + + +

欢迎光临

+ + \ No newline at end of file diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/beforeSnippetInjection.html b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/beforeSnippetInjection.html new file mode 100644 index 000000000000..de155ea51d7d --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/beforeSnippetInjection.html @@ -0,0 +1,10 @@ + + + + + Title + + + + + \ No newline at end of file diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/beforeSnippetInjectionChinese.html b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/beforeSnippetInjectionChinese.html new file mode 100644 index 000000000000..5594942811a7 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/beforeSnippetInjectionChinese.html @@ -0,0 +1,10 @@ + + + + + Title + + +

欢迎光临

+ + \ No newline at end of file diff --git a/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/htmlWithoutHeadTag.html b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/htmlWithoutHeadTag.html new file mode 100644 index 000000000000..e0a5f5d9d61c --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent-unit-tests/src/test/resources/htmlWithoutHeadTag.html @@ -0,0 +1,7 @@ + + + + +

without head tag

+ + \ No newline at end of file diff --git a/instrumentation/servlet/servlet-5.0/javaagent/build.gradle.kts b/instrumentation/servlet/servlet-5.0/javaagent/build.gradle.kts index 9203379bb520..8c5c43e9b5bf 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/build.gradle.kts +++ b/instrumentation/servlet/servlet-5.0/javaagent/build.gradle.kts @@ -17,6 +17,8 @@ dependencies { compileOnly("jakarta.servlet:jakarta.servlet-api:5.0.0") + testImplementation(project(":instrumentation:servlet:servlet-common:bootstrap")) + testLibrary("org.eclipse.jetty:jetty-server:11.0.0") testLibrary("org.eclipse.jetty:jetty-servlet:11.0.0") testLibrary("org.apache.tomcat.embed:tomcat-embed-core:10.0.0") diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java index 3fbba69b2496..4915f1e84ebd 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java @@ -13,6 +13,7 @@ import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncStartInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation; +import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletOutputStreamInstrumentation; import java.util.Arrays; import java.util.List; @@ -37,6 +38,11 @@ BASE_PACKAGE, adviceClassName(".async.AsyncContextStartAdvice")), adviceClassName(".service.JakartaServletServiceAdvice"), adviceClassName(".service.JakartaServletInitAdvice"), adviceClassName(".service.JakartaServletFilterInitAdvice")), + new ServletOutputStreamInstrumentation( + BASE_PACKAGE, + adviceClassName(".Servlet5OutputStreamWriteBytesAndOffsetAdvice"), + adviceClassName(".Servlet5OutputStreamWriteBytesAdvice"), + adviceClassName(".Servlet5OutputStreamWriteIntAdvice")), new HttpServletResponseInstrumentation( BASE_PACKAGE, adviceClassName(".response.ResponseSendAdvice"))); } diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteBytesAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteBytesAdvice.java new file mode 100644 index 000000000000..ba83897750f8 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteBytesAdvice.java @@ -0,0 +1,34 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons.getSnippetInjectionHelper; + +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet.ServletOutputStreamInjectionState; +import jakarta.servlet.ServletOutputStream; +import java.io.IOException; +import net.bytebuddy.asm.Advice; + +public class Servlet5OutputStreamWriteBytesAdvice { + + @Advice.OnMethodEnter(skipOn = Advice.OnDefaultValue.class, suppress = Throwable.class) + public static boolean methodEnter( + @Advice.This ServletOutputStream servletOutputStream, @Advice.Argument(0) byte[] write) + throws IOException { + InjectionState state = ServletOutputStreamInjectionState.getInjectionState(servletOutputStream); + if (state == null) { + return true; + } + // if handleWrite returns true, then it means the original bytes + the snippet were written + // to the servletOutputStream, and so we no longer need to execute the original method + // call (see skipOn above) + // if it returns false, then it means nothing was written to the servletOutputStream and the + // original method call should be executed + return !getSnippetInjectionHelper() + .handleWrite(state, servletOutputStream, write, 0, write.length); + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteBytesAndOffsetAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteBytesAndOffsetAdvice.java new file mode 100644 index 000000000000..e272afa3c58e --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteBytesAndOffsetAdvice.java @@ -0,0 +1,35 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons.getSnippetInjectionHelper; + +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet.ServletOutputStreamInjectionState; +import jakarta.servlet.ServletOutputStream; +import java.io.IOException; +import net.bytebuddy.asm.Advice; + +public class Servlet5OutputStreamWriteBytesAndOffsetAdvice { + @Advice.OnMethodEnter(skipOn = Advice.OnDefaultValue.class, suppress = Throwable.class) + public static boolean methodEnter( + @Advice.This ServletOutputStream servletOutputStream, + @Advice.Argument(value = 0) byte[] write, + @Advice.Argument(value = 1) int off, + @Advice.Argument(value = 2) int len) + throws IOException { + InjectionState state = ServletOutputStreamInjectionState.getInjectionState(servletOutputStream); + if (state == null) { + return true; + } + // if handleWrite returns true, then it means the original bytes + the snippet were written + // to the servletOutputStream, and so we no longer need to execute the original method + // call (see skipOn above) + // if it returns false, then it means nothing was written to the servletOutputStream and the + // original method call should be executed + return !getSnippetInjectionHelper().handleWrite(state, servletOutputStream, write, off, len); + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteIntAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteIntAdvice.java new file mode 100644 index 000000000000..fc478b945113 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5OutputStreamWriteIntAdvice.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons.getSnippetInjectionHelper; + +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; +import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet.ServletOutputStreamInjectionState; +import jakarta.servlet.ServletOutputStream; +import java.io.IOException; +import net.bytebuddy.asm.Advice; + +public class Servlet5OutputStreamWriteIntAdvice { + + @Advice.OnMethodEnter(skipOn = Advice.OnDefaultValue.class, suppress = Throwable.class) + public static boolean methodEnter( + @Advice.This ServletOutputStream servletOutputStream, @Advice.Argument(0) int write) + throws IOException { + InjectionState state = ServletOutputStreamInjectionState.getInjectionState(servletOutputStream); + if (state == null) { + return true; + } + // if handleWrite returns true, then it means the original bytes + the snippet were written + // to the servletOutputStream, and so we no longer need to execute the original method + // call (see skipOn above) + // if it returns false, then it means nothing was written to the servletOutputStream and the + // original method call should be executed + return !getSnippetInjectionHelper().handleWrite(state, servletOutputStream, write); + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Singletons.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Singletons.java index cdda410ae88d..fbb82815a9d1 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Singletons.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Singletons.java @@ -8,12 +8,14 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.util.ClassAndMethod; import io.opentelemetry.instrumentation.api.util.VirtualField; +import io.opentelemetry.javaagent.bootstrap.servlet.ExperimentalSnippetHolder; import io.opentelemetry.javaagent.bootstrap.servlet.MappingResolver; import io.opentelemetry.javaagent.instrumentation.servlet.ServletHelper; import io.opentelemetry.javaagent.instrumentation.servlet.ServletInstrumenterBuilder; import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext; import io.opentelemetry.javaagent.instrumentation.servlet.ServletResponseContext; import io.opentelemetry.javaagent.instrumentation.servlet.common.response.ResponseInstrumenterFactory; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.OutputStreamSnippetInjectionHelper; import jakarta.servlet.Filter; import jakarta.servlet.Servlet; import jakarta.servlet.http.HttpServletRequest; @@ -39,6 +41,8 @@ public final class Servlet5Singletons { private static final Instrumenter RESPONSE_INSTRUMENTER = ResponseInstrumenterFactory.createInstrumenter(INSTRUMENTATION_NAME); + private static final OutputStreamSnippetInjectionHelper SNIPPET_INJECTION_HELPER = + new OutputStreamSnippetInjectionHelper(() -> ExperimentalSnippetHolder.getSnippet()); public static ServletHelper helper() { return HELPER; @@ -48,6 +52,10 @@ public static Instrumenter responseInstrumenter() { return RESPONSE_INSTRUMENTER; } + public static OutputStreamSnippetInjectionHelper getSnippetInjectionHelper() { + return SNIPPET_INJECTION_HELPER; + } + public static MappingResolver getMappingResolver(Object servletOrFilter) { MappingResolver.Factory factory = getMappingResolverFactory(servletOrFilter); if (factory != null) { diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java index ced8758574df..7fc8c2326806 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.service; +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons.getSnippetInjectionHelper; import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons.helper; import io.opentelemetry.context.Context; @@ -17,6 +18,7 @@ import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext; import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Accessor; import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons; +import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet.Servlet5SnippetInjectingResponseWrapper; import jakarta.servlet.Servlet; import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; @@ -43,6 +45,14 @@ public static void onEnter( } HttpServletRequest httpServletRequest = (HttpServletRequest) request; + String snippet = getSnippetInjectionHelper().getSnippet(); + if (!snippet.isEmpty() + && !((HttpServletResponse) response) + .containsHeader(Servlet5SnippetInjectingResponseWrapper.FAKE_SNIPPET_HEADER)) { + response = + new Servlet5SnippetInjectingResponseWrapper((HttpServletResponse) response, snippet); + } + callDepth = CallDepth.forClass(AppServerBridge.getCallDepthKey()); callDepth.getAndIncrement(); diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/Servlet5SnippetInjectingResponseWrapper.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/Servlet5SnippetInjectingResponseWrapper.java new file mode 100644 index 000000000000..6c420de9753d --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/Servlet5SnippetInjectingResponseWrapper.java @@ -0,0 +1,165 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet.ServletOutputStreamInjectionState.initializeInjectionStateIfNeeded; +import static java.util.logging.Level.FINE; + +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.SnippetInjectingPrintWriter; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.SnippetInjectingResponseWrapper; +import jakarta.servlet.ServletOutputStream; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpServletResponseWrapper; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.logging.Logger; + +/** + * Notes on Content-Length: the snippet length is only added to the content length when injection + * occurs and the content length was set previously. + * + *

If the Content-Length is set after snippet injection occurs (either for the first time or is + * set again for some reason), we intentionally do not add the snippet length, because the + * application server may be making that call at the end of a request when it sees the request has + * not been submitted, in which case it is likely using the real length of content that has been + * written, including the snippet length. + */ +public class Servlet5SnippetInjectingResponseWrapper extends HttpServletResponseWrapper + implements SnippetInjectingResponseWrapper { + + private static final Logger logger = + Logger.getLogger(Servlet5SnippetInjectingResponseWrapper.class.getName()); + + public static final String FAKE_SNIPPET_HEADER = "FAKE_SNIPPET_HEADER"; + + private static final int UNSET = -1; + private final String snippet; + private final int snippetLength; + + private long contentLength = UNSET; + + private SnippetInjectingPrintWriter snippetInjectingPrintWriter = null; + + public Servlet5SnippetInjectingResponseWrapper(HttpServletResponse response, String snippet) { + super(response); + this.snippet = snippet; + snippetLength = snippet.length(); + } + + @Override + public boolean containsHeader(String name) { + // this function is overridden in order to make sure the response is wrapped + // but not wrapped twice + // we don't use req.setAttribute + // because async requests pass down their attributes, but don't pass down our wrapped response + // and so we would see the presence of the attribute and think the response was already wrapped + // when it really is not + // see also https://docs.oracle.com/javaee/7/api/javax/servlet/AsyncContext.html + if (name.equals(FAKE_SNIPPET_HEADER)) { + return true; + } + return super.containsHeader(name); + } + + @Override + public void setHeader(String name, String value) { + // checking content-type is just an optimization to avoid unnecessary parsing + if ("Content-Length".equalsIgnoreCase(name) && isContentTypeTextHtml()) { + try { + contentLength = Long.parseLong(value); + } catch (NumberFormatException ex) { + logger.log(FINE, "NumberFormatException", ex); + } + } + super.setHeader(name, value); + } + + @Override + public void addHeader(String name, String value) { + // checking content-type is just an optimization to avoid unnecessary parsing + if ("Content-Length".equalsIgnoreCase(name) && isContentTypeTextHtml()) { + try { + contentLength = Long.parseLong(value); + } catch (NumberFormatException ex) { + logger.log(FINE, "Failed to parse the Content-Length header", ex); + } + } + super.addHeader(name, value); + } + + @Override + public void setIntHeader(String name, int value) { + // checking content-type is just an optimization to avoid unnecessary parsing + if ("Content-Length".equalsIgnoreCase(name) && isContentTypeTextHtml()) { + contentLength = value; + } + super.setIntHeader(name, value); + } + + @Override + public void addIntHeader(String name, int value) { + // checking content-type is just an optimization to avoid unnecessary parsing + if ("Content-Length".equalsIgnoreCase(name) && isContentTypeTextHtml()) { + contentLength = value; + } + super.addIntHeader(name, value); + } + + @Override + public void setContentLength(int len) { + contentLength = len; + super.setContentLength(len); + } + + @Override + public void setContentLengthLong(long length) { + contentLength = length; + super.setContentLengthLong(length); + } + + @Override + public ServletOutputStream getOutputStream() throws IOException { + ServletOutputStream output = super.getOutputStream(); + initializeInjectionStateIfNeeded(output, this); + return output; + } + + @Override + public PrintWriter getWriter() throws IOException { + if (!isContentTypeTextHtml()) { + return super.getWriter(); + } + if (snippetInjectingPrintWriter == null) { + snippetInjectingPrintWriter = + new SnippetInjectingPrintWriter(super.getWriter(), snippet, this); + } + return snippetInjectingPrintWriter; + } + + @Override + public boolean isContentTypeTextHtml() { + String contentType = super.getContentType(); + if (contentType == null) { + contentType = super.getHeader("content-type"); + } + return contentType != null && contentType.startsWith("text/html"); + } + + @Override + public void updateContentLengthIfPreviouslySet() { + if (contentLength != UNSET) { + setContentLength((int) contentLength + snippetLength); + } + } + + @Override + public boolean isNotSafeToInject() { + // if content-length was set and response was already committed (headers sent to the client), + // then it is not safe to inject because the content-length header cannot be updated to account + // for the snippet length + return contentLength != UNSET && isCommitted(); + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/ServletOutputStreamInjectionState.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/ServletOutputStreamInjectionState.java new file mode 100644 index 000000000000..407b63b5bb8e --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/ServletOutputStreamInjectionState.java @@ -0,0 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.snippet; + +import io.opentelemetry.instrumentation.api.util.VirtualField; +import io.opentelemetry.javaagent.instrumentation.servlet.snippet.InjectionState; +import jakarta.servlet.ServletOutputStream; +import javax.annotation.Nullable; + +public class ServletOutputStreamInjectionState { + private static final VirtualField virtualField = + VirtualField.find(ServletOutputStream.class, InjectionState.class); + + public static void initializeInjectionStateIfNeeded( + ServletOutputStream servletOutputStream, Servlet5SnippetInjectingResponseWrapper wrapper) { + InjectionState state = virtualField.get(servletOutputStream); + if (!wrapper.isContentTypeTextHtml()) { + virtualField.set(servletOutputStream, null); + return; + } + if (state == null || state.getWrapper() != wrapper) { + state = new InjectionState(wrapper); + virtualField.set(servletOutputStream, state); + } + } + + @Nullable + public static InjectionState getInjectionState(ServletOutputStream servletOutputStream) { + return virtualField.get(servletOutputStream); + } + + private ServletOutputStreamInjectionState() {} +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy index bb6175902824..2347ffdc2f63 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy @@ -7,6 +7,7 @@ import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint +import io.opentelemetry.javaagent.bootstrap.servlet.ExperimentalSnippetHolder import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest import jakarta.servlet.Servlet @@ -37,6 +38,33 @@ abstract class AbstractServlet5Test extends HttpServerTest servlet) + public static final ServerEndpoint HTML_PRINT_WRITER = + new ServerEndpoint("HTML_PRINT_WRITER", "htmlPrintWriter", + 200, + "\n" + + "\n" + + "\n" + + " \n" + + " Title\n" + + "\n" + + "\n" + + "

test works

\n" + + "\n" + + "") + public static final ServerEndpoint HTML_SERVLET_OUTPUT_STREAM = + new ServerEndpoint("HTML_SERVLET_OUTPUT_STREAM", "htmlServletOutputStream", + 200, + "\n" + + "\n" + + "\n" + + " \n" + + " Title\n" + + "\n" + + "\n" + + "

test works

\n" + + "\n" + + "") + protected void setupServlets(CONTEXT context) { def servlet = servlet() @@ -49,6 +77,8 @@ abstract class AbstractServlet5Test extends HttpServerTest extends HttpServerTest Test ") + def request = request(HTML_SERVLET_OUTPUT_STREAM, "GET") + def response = client.execute(request).aggregate().join() + + expect: + response.status().code() == HTML_SERVLET_OUTPUT_STREAM.status + String result = "\n" + + "\n" + + "\n" + + " \n" + + " \n" + + " Title\n" + + "\n" + + "\n" + + "

test works

\n" + + "\n" + + "" + response.contentUtf8() == result + response.headers().contentLength() == result.length() + } + + def "snippet injection with PrintWriter"() { + setup: + ExperimentalSnippetHolder.setSnippet("\n ") + def request = request(HTML_PRINT_WRITER, "GET") + def response = client.execute(request).aggregate().join() + + expect: + response.status().code() == HTML_PRINT_WRITER.status + String result = "\n" + + "\n" + + "\n" + + " \n" + + " \n" + + " Title\n" + + "\n" + + "\n" + + "

test works

\n" + + "\n" + + "" + + response.contentUtf8() == result + response.headers().contentLength() == result.length() + } } diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy index 050884806c2a..f9825014e63f 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy @@ -143,6 +143,8 @@ class JettyServlet5TestForward extends JettyDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, RequestDispatcherServlet.Forward) addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) @@ -181,6 +183,8 @@ class JettyServlet5TestInclude extends JettyDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, RequestDispatcherServlet.Include) addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) @@ -203,7 +207,8 @@ class JettyServlet5TestDispatchImmediate extends JettyDispatchTest { @Override protected void setupServlets(Object context) { super.setupServlets(context) - + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, TestServlet5.DispatchImmediate) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch" + SUCCESS.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch" + QUERY_PARAM.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch" + ERROR.path, TestServlet5.DispatchImmediate) @@ -229,6 +234,8 @@ class JettyServlet5TestDispatchAsync extends JettyDispatchTest { super.setupServlets(context) addServlet(context, "/dispatch" + SUCCESS.path, TestServlet5.DispatchAsync) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, TestServlet5.DispatchAsync) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, TestServlet5.DispatchAsync) addServlet(context, "/dispatch" + QUERY_PARAM.path, TestServlet5.DispatchAsync) addServlet(context, "/dispatch" + ERROR.path, TestServlet5.DispatchAsync) addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet5.DispatchAsync) diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy index 077a9c5d3131..68b167184ff9 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy @@ -71,6 +71,19 @@ class TestServlet5 { break case EXCEPTION: throw new ServletException(endpoint.body) + case AbstractServlet5Test.HTML_PRINT_WRITER: + resp.contentType = "text/html" + resp.status = endpoint.status + resp.setContentLengthLong(endpoint.body.length()) + resp.writer.print(endpoint.body) + break + case AbstractServlet5Test.HTML_SERVLET_OUTPUT_STREAM: + resp.contentType = "text/html" + resp.status = endpoint.status + resp.setContentLength(endpoint.body.length()) + byte[] body = endpoint.body.getBytes() + resp.getOutputStream().write(body, 0, body.length) + break } } } @@ -142,6 +155,21 @@ class TestServlet5 { writer.close() } throw new ServletException(endpoint.body) + break + case AbstractServlet5Test.HTML_PRINT_WRITER: + resp.contentType = "text/html" + resp.status = endpoint.status + resp.setContentLength(endpoint.body.length()) + resp.writer.print(endpoint.body) + context.complete() + break + case AbstractServlet5Test.HTML_SERVLET_OUTPUT_STREAM: + resp.contentType = "text/html" + resp.status = endpoint.status + resp.getOutputStream().print(endpoint.body) + context.complete() + break + } } } finally { @@ -198,6 +226,17 @@ class TestServlet5 { resp.status = endpoint.status resp.writer.print(endpoint.body) throw new ServletException(endpoint.body) + case AbstractServlet5Test.HTML_PRINT_WRITER: + // intentionally testing setting status before contentType here to cover that case somewhere + resp.status = endpoint.status + resp.contentType = "text/html" + resp.writer.print(endpoint.body) + break + case AbstractServlet5Test.HTML_SERVLET_OUTPUT_STREAM: + resp.contentType = "text/html" + resp.status = endpoint.status + resp.getOutputStream().print(endpoint.body) + break } } } finally { diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy index a3b061aff9ea..cbe7d915cf0a 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy @@ -343,6 +343,8 @@ class TomcatServlet5TestForward extends TomcatDispatchTest { addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, RequestDispatcherServlet.Forward) addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, RequestDispatcherServlet.Forward) addServlet(context, "/dispatch" + INDEXED_CHILD.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, RequestDispatcherServlet.Forward) } } @@ -385,6 +387,8 @@ class TomcatServlet5TestInclude extends TomcatDispatchTest { addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, RequestDispatcherServlet.Include) addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, RequestDispatcherServlet.Include) addServlet(context, "/dispatch" + INDEXED_CHILD.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, RequestDispatcherServlet.Include) } } @@ -412,6 +416,8 @@ class TomcatServlet5TestDispatchImmediate extends TomcatDispatchTest { addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet5.DispatchImmediate) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, TestServlet5.DispatchImmediate) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch/recursive", TestServlet5.DispatchRecursive) } } @@ -435,6 +441,8 @@ class TomcatServlet5TestDispatchAsync extends TomcatDispatchTest { addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, TestServlet5.DispatchAsync) addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, TestServlet5.DispatchAsync) addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet5.DispatchAsync) + addServlet(context, "/dispatch" + HTML_PRINT_WRITER.path, TestServlet5.DispatchAsync) + addServlet(context, "/dispatch" + HTML_SERVLET_OUTPUT_STREAM.path, TestServlet5.DispatchAsync) addServlet(context, "/dispatch/recursive", TestServlet5.DispatchRecursive) } diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/java/RequestDispatcherServlet.java b/instrumentation/servlet/servlet-5.0/javaagent/src/test/java/RequestDispatcherServlet.java index 3a917bb21999..d36a7b2284f2 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/java/RequestDispatcherServlet.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/java/RequestDispatcherServlet.java @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletException; @@ -37,6 +38,13 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) String target = req.getServletPath().replace("/dispatch", ""); ServletContext context = getServletContext(); RequestDispatcher dispatcher = context.getRequestDispatcher(target); + // for HTML test case, set the content type before calling include because + // setContentType will be rejected if called inside of include + // check https://statics.teams.cdn.office.net/evergreen-assets/safelinks/1/atp-safelinks.html + if (ServerEndpoint.forPath(target) == ServerEndpoint.forPath("/htmlPrintWriter") + || ServerEndpoint.forPath(target) == ServerEndpoint.forPath("/htmlServletOutputStream")) { + resp.setContentType("text/html"); + } dispatcher.include(req, resp); } } diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionState.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/InjectionState.java similarity index 96% rename from instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionState.java rename to instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/InjectionState.java index a535d81fd152..78ada6e57e13 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionState.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/InjectionState.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet; +package io.opentelemetry.javaagent.instrumentation.servlet.snippet; // this is shared by both ServletOutputStream and PrintWriter injection public class InjectionState { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/OutputStreamSnippetInjectionHelper.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/OutputStreamSnippetInjectionHelper.java similarity index 90% rename from instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/OutputStreamSnippetInjectionHelper.java rename to instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/OutputStreamSnippetInjectionHelper.java index 287992446c73..2ac8fb839a4b 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/OutputStreamSnippetInjectionHelper.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/OutputStreamSnippetInjectionHelper.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet; +package io.opentelemetry.javaagent.instrumentation.servlet.snippet; import static java.util.logging.Level.FINE; @@ -20,10 +20,6 @@ public class OutputStreamSnippetInjectionHelper { private final Supplier snippetSupplier; - public OutputStreamSnippetInjectionHelper(String snippet) { - this(() -> snippet); - } - public OutputStreamSnippetInjectionHelper(Supplier snippetSupplier) { this.snippetSupplier = snippetSupplier; } @@ -64,7 +60,7 @@ public boolean handleWrite( try { snippetBytes = snippetSupplier.get().getBytes(state.getCharacterEncoding()); } catch (UnsupportedEncodingException e) { - logger.log(FINE, "UnsupportedEncodingException", e); + logger.log(FINE, "Failed getting snippet bytes", e); return false; } // updating Content-Length before any further writing in case that writing triggers a flush @@ -90,7 +86,7 @@ public boolean handleWrite(InjectionState state, OutputStream out, int b) throws try { snippetBytes = snippetSupplier.get().getBytes(state.getCharacterEncoding()); } catch (UnsupportedEncodingException e) { - logger.log(FINE, "UnsupportedEncodingException", e); + logger.log(FINE, "Failed getting snippet bytes", e); return false; } state.getWrapper().updateContentLengthIfPreviouslySet(); diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingPrintWriter.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/SnippetInjectingPrintWriter.java similarity index 94% rename from instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingPrintWriter.java rename to instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/SnippetInjectingPrintWriter.java index 55d70d8d9568..bbbce6271444 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingPrintWriter.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/SnippetInjectingPrintWriter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet; +package io.opentelemetry.javaagent.instrumentation.servlet.snippet; import java.io.PrintWriter; diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/SnippetInjectingResponseWrapper.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/SnippetInjectingResponseWrapper.java new file mode 100644 index 000000000000..bce927bea8ff --- /dev/null +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet/SnippetInjectingResponseWrapper.java @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.snippet; + +public interface SnippetInjectingResponseWrapper { + + boolean isContentTypeTextHtml(); + + void updateContentLengthIfPreviouslySet(); + + boolean isNotSafeToInject(); + + String getCharacterEncoding(); +} diff --git a/settings.gradle.kts b/settings.gradle.kts index e3afe3b9c679..34db76186bcf 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -469,6 +469,7 @@ hideFromDependabot(":instrumentation:servlet:servlet-2.2:javaagent") hideFromDependabot(":instrumentation:servlet:servlet-3.0:javaagent") hideFromDependabot(":instrumentation:servlet:servlet-3.0:javaagent-unit-tests") hideFromDependabot(":instrumentation:servlet:servlet-5.0:javaagent") +hideFromDependabot(":instrumentation:servlet:servlet-5.0:javaagent-unit-tests") hideFromDependabot(":instrumentation:spark-2.3:javaagent") hideFromDependabot(":instrumentation:spring:spring-batch-3.0:javaagent") hideFromDependabot(":instrumentation:spring:spring-boot-actuator-autoconfigure-2.0:javaagent")