From fa85fb88968d4db411b39df21f3ff3bed90714f4 Mon Sep 17 00:00:00 2001 From: qzmo Date: Wed, 12 Mar 2025 15:55:40 +0800 Subject: [PATCH 1/4] fix: inconsistent behavior when buffer http entity content with org.apache.http.entity.BufferedHttpEntity --- .../common/ApacheHttpClientAdapter.java | 4 +- .../apache/common/ArexBufferedHttpEntity.java | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java index f8c15cd5b..a5fb0c40f 100644 --- a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java @@ -140,7 +140,7 @@ public static void bufferRequestEntity(HttpEntityEnclosingRequest enclosingReque return; } try { - enclosingRequest.setEntity(new BufferedHttpEntity(enclosingRequest.getEntity())); + enclosingRequest.setEntity(new ArexBufferedHttpEntity(enclosingRequest.getEntity())); } catch (Exception ignore) { // ignore exception } @@ -151,7 +151,7 @@ public static void bufferResponseEntity(HttpResponse response) { return; } try { - EntityUtils.updateEntity(response, new BufferedHttpEntity(response.getEntity())); + EntityUtils.updateEntity(response, new ArexBufferedHttpEntity(response.getEntity())); } catch (Exception e) { // ignore exception } diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java new file mode 100644 index 000000000..377a4ea6b --- /dev/null +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java @@ -0,0 +1,39 @@ +package io.arex.inst.httpclient.apache.common; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import org.apache.http.HttpEntity; +import org.apache.http.entity.HttpEntityWrapper; + +/** + * This class only buffer the original HttpEntity content into a byte array, tt does not modify any + * behavior of the original HttpEntity. + * This class is known to have performance issues comparing to the original BufferedHttpEntity, but + * it provides consistent behavior with the original HttpEntity. + * @see org.apache.http.entity.BufferedHttpEntity + * @author: QizhengMo + * @date: 2025/3/12 15:35 + */ +public class ArexBufferedHttpEntity extends HttpEntityWrapper { + private final byte[] buffer; + + public ArexBufferedHttpEntity(HttpEntity wrappedEntity) throws IOException { + super(wrappedEntity); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + + // This class is only used in Arex Agent, so we are almost always the first to consume the content. + wrappedEntity.writeTo(out); + out.flush(); + this.buffer = out.toByteArray(); + } + + /** + * Return a copy of the original content of the wrapped HttpEntity. + */ + @Override + public InputStream getContent() throws IOException { + return new ByteArrayInputStream(buffer); + } +} From fa588166fdbbeb0b68e0794fc989d7a5b73c0749 Mon Sep 17 00:00:00 2001 From: qzmo Date: Wed, 12 Mar 2025 16:02:27 +0800 Subject: [PATCH 2/4] fix: class guard --- .../inst/httpclient/apache/common/ApacheHttpClientAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java index a5fb0c40f..b04340f1d 100644 --- a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java @@ -158,7 +158,7 @@ public static void bufferResponseEntity(HttpResponse response) { } private byte[] getEntityBytes(HttpEntity entity) { - if (!(entity instanceof BufferedHttpEntity)) { + if (!(entity instanceof ArexBufferedHttpEntity)) { return ZERO_BYTE; } try { From a8116b0573236d6de2749c2df0096f4080013b76 Mon Sep 17 00:00:00 2001 From: qzmo Date: Wed, 12 Mar 2025 16:54:33 +0800 Subject: [PATCH 3/4] fix: override writeTo to allow repeated consumption --- .../apache/common/ApacheHttpClientAdapter.java | 4 ++-- .../apache/common/ArexBufferedHttpEntity.java | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java index b04340f1d..64a428818 100644 --- a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ApacheHttpClientAdapter.java @@ -142,7 +142,7 @@ public static void bufferRequestEntity(HttpEntityEnclosingRequest enclosingReque try { enclosingRequest.setEntity(new ArexBufferedHttpEntity(enclosingRequest.getEntity())); } catch (Exception ignore) { - // ignore exception + // ignore exception, fallback to original entity and ignore recording } } @@ -153,7 +153,7 @@ public static void bufferResponseEntity(HttpResponse response) { try { EntityUtils.updateEntity(response, new ArexBufferedHttpEntity(response.getEntity())); } catch (Exception e) { - // ignore exception + // ignore exception, fallback to original entity and ignore recording } } diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java index 377a4ea6b..88d598605 100644 --- a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/main/java/io/arex/inst/httpclient/apache/common/ArexBufferedHttpEntity.java @@ -4,6 +4,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import org.apache.http.HttpEntity; import org.apache.http.entity.HttpEntityWrapper; @@ -36,4 +37,13 @@ public ArexBufferedHttpEntity(HttpEntity wrappedEntity) throws IOException { public InputStream getContent() throws IOException { return new ByteArrayInputStream(buffer); } + + @Override + public void writeTo(final OutputStream outStream) throws IOException { + if (this.buffer != null) { + outStream.write(this.buffer); + } else { + super.writeTo(outStream); + } + } } From c08613a8e6cf8fc108e4e2b5fa70a7f8ab403f7f Mon Sep 17 00:00:00 2001 From: qzmo Date: Wed, 12 Mar 2025 17:04:42 +0800 Subject: [PATCH 4/4] fix: ut type assertion --- .../apache/async/BasicFutureInstrumentationTest.java | 3 ++- .../apache/async/RequestProducerInstrumentationTest.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/BasicFutureInstrumentationTest.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/BasicFutureInstrumentationTest.java index cc7a09a09..223339e94 100644 --- a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/BasicFutureInstrumentationTest.java +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/BasicFutureInstrumentationTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.*; +import io.arex.inst.httpclient.apache.common.ArexBufferedHttpEntity; import io.arex.inst.runtime.context.ContextManager; import java.io.IOException; import net.bytebuddy.description.method.MethodDescription; @@ -54,7 +55,7 @@ void testFutureAdvice() throws IOException { ((FutureCallbackWrapper) wrapper).setNeedRecord(true); BasicFutureInstrumentation.FutureAdvice.completed(httpResponse, wrapper); - assertInstanceOf(BufferedHttpEntity.class, httpResponse.getEntity()); + assertInstanceOf(ArexBufferedHttpEntity.class, httpResponse.getEntity()); BasicFutureInstrumentation.FutureAdvice.completed(httpResponse, null); } diff --git a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/RequestProducerInstrumentationTest.java b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/RequestProducerInstrumentationTest.java index d3bc90da6..d761d7ef0 100644 --- a/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/RequestProducerInstrumentationTest.java +++ b/arex-instrumentation/httpclient/arex-httpclient-apache-v4/src/test/java/io/arex/inst/httpclient/apache/async/RequestProducerInstrumentationTest.java @@ -4,6 +4,7 @@ import static org.mockito.Mockito.mockStatic; import io.arex.inst.httpclient.apache.async.RequestProducerInstrumentation.ConstructorAdvice; +import io.arex.inst.httpclient.apache.common.ArexBufferedHttpEntity; import io.arex.inst.runtime.context.ContextManager; import java.lang.reflect.Constructor; import net.bytebuddy.description.method.MethodDescription.ForLoadedConstructor; @@ -58,7 +59,7 @@ void onEnter() { HttpPost request = new HttpPost(); request.setEntity(new ByteArrayEntity("test".getBytes())); ConstructorAdvice.onEnter(request); - assertInstanceOf(BufferedHttpEntity.class, request.getEntity()); + assertInstanceOf(ArexBufferedHttpEntity.class, request.getEntity()); } } }