-
Notifications
You must be signed in to change notification settings - Fork 30
Preserve connection keep-alive for JMeter compatibility #116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
104 changes: 104 additions & 0 deletions
104
src/main/java/com/blazemeter/jmeter/http2/core/JmeterRequestHeadersSupport.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package com.blazemeter.jmeter.http2.core; | ||
|
|
||
| import org.apache.jmeter.protocol.http.util.HTTPConstants; | ||
| import org.eclipse.jetty.client.Request; | ||
| import org.eclipse.jetty.http.HttpFields; | ||
| import org.eclipse.jetty.http.HttpHeader; | ||
| import org.eclipse.jetty.http.HttpVersion; | ||
|
|
||
| /** | ||
| * Preserves JMeter-visible request headers in {@code HTTPSampleResult#getRequestHeaders()} | ||
| * when Jetty omits or strips them on the wire. | ||
| * | ||
| * <p>Counterpart of {@link JmeterCompressionHeadersSupport} for outbound request headers. | ||
| */ | ||
| final class JmeterRequestHeadersSupport { | ||
|
|
||
| static final String ATTR_USE_KEEPALIVE = "bzm.useKeepAlive"; | ||
|
|
||
| private JmeterRequestHeadersSupport() { | ||
| } | ||
|
|
||
| /** | ||
| * Applies sampler-driven request headers before send, matching {@code HTTPHC4Impl#setupRequest}. | ||
| */ | ||
| static void prepareFromSampler(Request request, boolean useKeepAlive) { | ||
| if (request == null) { | ||
| return; | ||
| } | ||
| request.attribute(ATTR_USE_KEEPALIVE, useKeepAlive); | ||
| applyConnectionHeader(request, useKeepAlive); | ||
| // TODO: HC4 sends explicit empty User-Agent when none is configured (disableDefaultUserAgent). | ||
| // prepareEmptyUserAgentHeader(request); | ||
| } | ||
|
|
||
| /** | ||
| * Copies sampler header settings to a cloned request (for example HTTP/1.1 fallback). | ||
| */ | ||
| static void copySamplerHeaderState(Request from, Request to) { | ||
| if (from == null || to == null) { | ||
| return; | ||
| } | ||
| Object useKeepAlive = from.getAttributes().get(ATTR_USE_KEEPALIVE); | ||
| if (useKeepAlive != null) { | ||
| to.attribute(ATTR_USE_KEEPALIVE, useKeepAlive); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Request headers to record in the sample after Jetty may have adjusted the live fields. | ||
| */ | ||
| static HttpFields headersForSampleResult(Request request) { | ||
| if (request == null) { | ||
| return HttpFields.EMPTY; | ||
| } | ||
| HttpFields.Mutable merged = HttpFields.build(request.getHeaders()); | ||
| restoreConnectionHeaderForSample(request, merged); | ||
| // restoreEmptyUserAgentForSample(request, merged); | ||
| return merged; | ||
| } | ||
|
|
||
| private static void applyConnectionHeader(Request request, boolean useKeepAlive) { | ||
| if (!shouldSendConnectionHeader(request)) { | ||
| return; | ||
| } | ||
| HttpFields.Mutable mutableHeaders = mutableHeaders(request); | ||
| if (mutableHeaders == null || mutableHeaders.contains(HttpHeader.CONNECTION)) { | ||
| return; | ||
| } | ||
| if (useKeepAlive) { | ||
| mutableHeaders.put(HTTPConstants.HEADER_CONNECTION, HTTPConstants.KEEP_ALIVE); | ||
| } else { | ||
| mutableHeaders.put(HTTPConstants.HEADER_CONNECTION, HTTPConstants.CONNECTION_CLOSE); | ||
| } | ||
| } | ||
|
|
||
| private static void restoreConnectionHeaderForSample( | ||
| Request request, HttpFields.Mutable headers) { | ||
| Object useKeepAlive = request.getAttributes().get(ATTR_USE_KEEPALIVE); | ||
| if (useKeepAlive == null || !shouldSendConnectionHeader(request)) { | ||
| return; | ||
| } | ||
| if (Boolean.TRUE.equals(useKeepAlive)) { | ||
| headers.put(HTTPConstants.HEADER_CONNECTION, HTTPConstants.KEEP_ALIVE); | ||
| } else { | ||
| headers.put(HTTPConstants.HEADER_CONNECTION, HTTPConstants.CONNECTION_CLOSE); | ||
| } | ||
| } | ||
|
|
||
| private static boolean shouldSendConnectionHeader(Request request) { | ||
| if (request != null && request.getVersion() == HttpVersion.HTTP_2) { | ||
| return false; | ||
| } | ||
| HttpFields headers = request.getHeaders(); | ||
| return headers == null || !headers.contains(HttpHeader.CONNECTION); | ||
| } | ||
|
|
||
| private static HttpFields.Mutable mutableHeaders(Request request) { | ||
| HttpFields headers = request.getHeaders(); | ||
| if (headers instanceof HttpFields.Mutable) { | ||
| return (HttpFields.Mutable) headers; | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
132 changes: 132 additions & 0 deletions
132
src/test/java/com/blazemeter/jmeter/http2/core/HttpKeepAliveSampleHeadersTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| package com.blazemeter.jmeter.http2.core; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import com.blazemeter.jmeter.http2.sampler.HTTP2Sampler; | ||
| import com.blazemeter.jmeter.http2.sampler.JMeterTestUtils; | ||
| import java.io.InputStream; | ||
| import java.io.OutputStream; | ||
| import java.net.ServerSocket; | ||
| import java.net.Socket; | ||
| import java.net.URL; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.Future; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.apache.jmeter.protocol.http.sampler.HTTPSampleResult; | ||
| import org.apache.jmeter.protocol.http.util.HTTPConstants; | ||
| import org.apache.jmeter.util.JMeterUtils; | ||
| import org.junit.After; | ||
| import org.junit.BeforeClass; | ||
| import org.junit.Test; | ||
|
|
||
| public class HttpKeepAliveSampleHeadersTest { | ||
|
|
||
| private ExecutorService executor; | ||
|
|
||
| @BeforeClass | ||
| public static void setupJmeter() { | ||
| JMeterTestUtils.setupJmeterEnv(); | ||
| } | ||
|
|
||
| @After | ||
| public void tearDown() throws Exception { | ||
| if (executor != null) { | ||
| executor.shutdownNow(); | ||
| executor.awaitTermination(5, TimeUnit.SECONDS); | ||
| executor = null; | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void sampleRequestHeadersIncludeKeepAliveWhenEnabled() throws Exception { | ||
| configureLegacyHttp1Only(); | ||
|
|
||
| try (ServerSocket serverSocket = new ServerSocket(0)) { | ||
| int port = serverSocket.getLocalPort(); | ||
| Future<String> received = startWireCapture(serverSocket); | ||
|
|
||
| HTTP2JettyClient jettyClient = new HTTP2JettyClient(false, "keepalive-sample-test"); | ||
| jettyClient.start(); | ||
| try { | ||
| HTTPSampleResult sampleResult = samplePlainHttp(jettyClient, port, true); | ||
| String wireRequest = received.get(10, TimeUnit.SECONDS); | ||
|
|
||
| assertThat(sampleResult.getRequestHeaders()) | ||
| .contains(HTTPConstants.HEADER_CONNECTION + ": " + HTTPConstants.KEEP_ALIVE); | ||
| assertThat(wireRequest).contains("GET /test HTTP/1.1"); | ||
| assertThat(wireRequest.toLowerCase()).doesNotContain("connection: keep-alive"); | ||
| } finally { | ||
| jettyClient.stop(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void sampleRequestHeadersIncludeCloseWhenKeepAliveDisabled() throws Exception { | ||
| configureLegacyHttp1Only(); | ||
|
|
||
| try (ServerSocket serverSocket = new ServerSocket(0)) { | ||
| int port = serverSocket.getLocalPort(); | ||
| Future<String> received = startWireCapture(serverSocket); | ||
|
|
||
| HTTP2JettyClient jettyClient = new HTTP2JettyClient(false, "keepalive-close-test"); | ||
| jettyClient.start(); | ||
| try { | ||
| HTTPSampleResult sampleResult = samplePlainHttp(jettyClient, port, false); | ||
| String wireRequest = received.get(10, TimeUnit.SECONDS); | ||
|
|
||
| assertThat(sampleResult.getRequestHeaders()) | ||
| .contains(HTTPConstants.HEADER_CONNECTION + ": " | ||
| + HTTPConstants.CONNECTION_CLOSE); | ||
| assertThat(wireRequest.toLowerCase()).contains("connection: close"); | ||
| } finally { | ||
| jettyClient.stop(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void configureLegacyHttp1Only() { | ||
| JMeterUtils.setProperty("blazemeter.http.enableHttp2", "false"); | ||
| JMeterUtils.setProperty("blazemeter.http.enableHttp3", "false"); | ||
| JMeterUtils.setProperty("blazemeter.http.profile", "legacy"); | ||
| } | ||
|
|
||
| private Future<String> startWireCapture(ServerSocket serverSocket) { | ||
| executor = Executors.newSingleThreadExecutor(); | ||
| return executor.submit(() -> readRequest(serverSocket)); | ||
| } | ||
|
|
||
| private static HTTPSampleResult samplePlainHttp(HTTP2JettyClient jettyClient, int port, | ||
| boolean useKeepAlive) throws Exception { | ||
| HTTP2Sampler sampler = new HTTP2Sampler(); | ||
| sampler.setMethod("GET"); | ||
| sampler.setDomain("localhost"); | ||
| sampler.setPort(port); | ||
| sampler.setPath("/test"); | ||
| sampler.setProtocol("http"); | ||
| sampler.setUseKeepAlive(useKeepAlive); | ||
|
|
||
| URL url = new URL("http", "localhost", port, "/test"); | ||
| HTTPSampleResult result = new HTTPSampleResult(); | ||
| result.setSampleLabel("GET_NO_PARAMETERS"); | ||
| result.setHTTPMethod("GET"); | ||
| result.setURL(url); | ||
|
|
||
| return jettyClient.sample(sampler, result, false, 0); | ||
| } | ||
|
|
||
| private static String readRequest(ServerSocket serverSocket) throws Exception { | ||
| try (Socket socket = serverSocket.accept(); | ||
| InputStream in = socket.getInputStream(); | ||
| OutputStream out = socket.getOutputStream()) { | ||
| byte[] buffer = new byte[4096]; | ||
| int read = in.read(buffer); | ||
| out.write("HTTP/1.0 200 OK\r\nContent-Type: text/plain\r\n\r\n" | ||
| .getBytes(StandardCharsets.US_ASCII)); | ||
| out.flush(); | ||
| return new String(buffer, 0, read, StandardCharsets.US_ASCII); | ||
| } | ||
| } | ||
| } |
77 changes: 77 additions & 0 deletions
77
src/test/java/com/blazemeter/jmeter/http2/core/JmeterRequestHeadersSupportTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package com.blazemeter.jmeter.http2.core; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.ArgumentMatchers.anyString; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import org.apache.jmeter.protocol.http.util.HTTPConstants; | ||
| import org.eclipse.jetty.client.Request; | ||
| import org.eclipse.jetty.http.HttpFields; | ||
| import org.eclipse.jetty.http.HttpHeader; | ||
| import org.eclipse.jetty.http.HttpVersion; | ||
| import org.junit.Test; | ||
| import org.mockito.Mockito; | ||
|
|
||
| public class JmeterRequestHeadersSupportTest { | ||
|
|
||
| private static Request mockRequest(HttpFields.Mutable headers) { | ||
| Map<String, Object> attributes = new HashMap<>(); | ||
| Request request = Mockito.mock(Request.class); | ||
| Mockito.when(request.getHeaders()).thenReturn(headers); | ||
| Mockito.when(request.getVersion()).thenReturn(HttpVersion.HTTP_1_1); | ||
| Mockito.when(request.getAttributes()).thenReturn(attributes); | ||
| Mockito.doAnswer(invocation -> { | ||
| attributes.put(invocation.getArgument(0), invocation.getArgument(1)); | ||
| return invocation.getMock(); | ||
| }).when(request).attribute(anyString(), any()); | ||
| return request; | ||
| } | ||
|
|
||
| @Test | ||
| public void prepareFromSamplerAddsConnectionKeepAliveWhenEnabled() { | ||
| HttpFields.Mutable headers = HttpFields.build(); | ||
| Request request = mockRequest(headers); | ||
|
|
||
| JmeterRequestHeadersSupport.prepareFromSampler(request, true); | ||
|
|
||
| assertThat(headers.get(HttpHeader.CONNECTION)).isEqualTo(HTTPConstants.KEEP_ALIVE); | ||
| } | ||
|
|
||
| @Test | ||
| public void headersForSampleResultRestoresKeepAliveAfterHeadersCleared() { | ||
| HttpFields.Mutable headers = HttpFields.build(); | ||
| Request request = mockRequest(headers); | ||
| JmeterRequestHeadersSupport.prepareFromSampler(request, true); | ||
| headers.remove(HttpHeader.CONNECTION); | ||
|
|
||
| HttpFields sampleHeaders = JmeterRequestHeadersSupport.headersForSampleResult(request); | ||
|
|
||
| assertThat(sampleHeaders.get(HttpHeader.CONNECTION)).isEqualTo(HTTPConstants.KEEP_ALIVE); | ||
| } | ||
|
|
||
| @Test | ||
| public void headersForSampleResultRestoresConnectionCloseWhenDisabled() { | ||
| HttpFields.Mutable headers = HttpFields.build(); | ||
| Request request = mockRequest(headers); | ||
| JmeterRequestHeadersSupport.prepareFromSampler(request, false); | ||
| headers.remove(HttpHeader.CONNECTION); | ||
|
|
||
| HttpFields sampleHeaders = JmeterRequestHeadersSupport.headersForSampleResult(request); | ||
|
|
||
| assertThat(sampleHeaders.get(HttpHeader.CONNECTION)) | ||
| .isEqualTo(HTTPConstants.CONNECTION_CLOSE); | ||
| } | ||
|
|
||
| @Test | ||
| public void prepareFromSamplerDoesNotOverrideExplicitConnectionHeader() { | ||
| HttpFields.Mutable headers = HttpFields.build() | ||
| .add(HttpHeader.CONNECTION, "Upgrade"); | ||
| Request request = mockRequest(headers); | ||
|
|
||
| JmeterRequestHeadersSupport.prepareFromSampler(request, true); | ||
|
|
||
| assertThat(headers.get(HttpHeader.CONNECTION)).isEqualTo("Upgrade"); | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this dead code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a placeholder for the next pull request.
It's part of a larger project that's out there somewhere.
There's another issue related to the User Agent, and I didn't want to include it in the same pull request, but the overall refactoring and implementation structure would encompass both. I've left the placeholder comments for the larger project; once this pull request is approved, the next one will follow.