From f2151451e0b037dc1afb2f228f70cefe1f2b03a7 Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Wed, 4 Dec 2024 08:34:00 +0100 Subject: [PATCH 1/7] Fix memory leak when transaction has parents Improve sampler cleanup during thread run --- .../apache/jmeter/samplers/SampleResult.java | 103 +++++++++++++++++- .../apache/jmeter/threads/JMeterThread.java | 8 +- 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java index 66589162432..a9ee530588c 100644 --- a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java +++ b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java @@ -32,7 +32,10 @@ import org.apache.jmeter.assertions.AssertionResult; import org.apache.jmeter.gui.Searchable; import org.apache.jmeter.testelement.TestPlan; +import org.apache.jmeter.threads.JMeterContext; +import org.apache.jmeter.threads.JMeterContextService; import org.apache.jmeter.threads.JMeterContext.TestLogicalAction; +import org.apache.jmeter.control.TransactionSampler; import org.apache.jmeter.util.JMeterUtils; import org.apache.jorphan.util.JOrphanUtils; import org.slf4j.Logger; @@ -1594,10 +1597,62 @@ public void setStartNextThreadLoop(boolean startNextThreadLoop) { } /** - * Clean up cached data + * Clean up cached data, but only if this is a root result (no parent) + * and not part of a parent transaction. */ public void cleanAfterSample() { + // Only clean if this is a root result (no parent) + if (parent != null) { + return; + } + + // Check if we're part of a parent transaction by walking up the sampler hierarchy + JMeterContext context = JMeterContextService.getContext(); + if (context != null) { + Sampler currentSampler = context.getCurrentSampler(); + if (currentSampler instanceof TransactionSampler) { + TransactionSampler transSampler = (TransactionSampler) currentSampler; + // Get the parent sampler from the transaction + Sampler parentSampler = transSampler.getSubSampler(); + // If there's a parent sampler and it's a transaction, we're nested + if (parentSampler instanceof TransactionSampler) { + return; + } + } + } + + cleanRecursively(); + } + + /** + * Internal method to clean this result and all its sub-results + */ + private void cleanRecursively() { + // Clean sub-results first + if (subResults != null) { + for (SampleResult subResult : subResults) { + if (subResult != null) { + subResult.cleanRecursively(); + } + } + subResults.clear(); + subResults = null; + } + + // Clean assertion results + if (assertionResults != null) { + assertionResults.clear(); + assertionResults = null; + } + + // Clear only memory-heavy data and caches, preserve samplerData + this.parent = null; this.responseDataAsString = null; + this.responseData = EMPTY_BA; + this.responseHeaders = ""; + this.requestHeaders = ""; + this.samplerData = null; + } @Override @@ -1665,4 +1720,50 @@ public TestLogicalAction getTestLogicalAction() { public void setTestLogicalAction(TestLogicalAction testLogicalAction) { this.testLogicalAction = testLogicalAction; } + + /** + * Create a deep copy for async listeners + * @return A deep copy of this result + */ + public SampleResult cloneForListeners() { + // Create clone with the correct type + SampleResult clone; + try { + // Use the same constructor that was used to create this instance + clone = this.getClass().getConstructor(this.getClass()).newInstance(this); + } catch (Exception e) { + // Fallback to base class if constructor is not available + log.debug("Could not create clone with type: " + this.getClass().getName() + ", using base class", e); + clone = new SampleResult(this); + } + + // Deep copy mutable fields that the copy constructor doesn't handle deeply + if (responseData != EMPTY_BA) { + clone.responseData = responseData.clone(); + } + + // Deep copy subResults + if (subResults != null) { + clone.subResults = new ArrayList<>(subResults.size()); + for (SampleResult sub : subResults) { + SampleResult subClone = sub.cloneForListeners(); + subClone.setParent(clone); + clone.subResults.add(subClone); + } + } + + // Deep copy assertion results + if (assertionResults != null) { + clone.assertionResults = new ArrayList<>(assertionResults.size()); + for (AssertionResult assertionResult : assertionResults) { + clone.assertionResults.add(assertionResult); // AssertionResult is immutable + } + } + + // Clear only the caches and unnecessary references in the clone + clone.responseDataAsString = null; + clone.parent = null; // Parent reference not needed in the clone + + return clone; + } } diff --git a/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java b/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java index 8b7f13f7900..fae46708c2e 100644 --- a/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java +++ b/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java @@ -678,6 +678,12 @@ private SampleResult doEndTransactionSampler( notifyListeners(transactionPack.getSampleListeners(), transactionResult); } compiler.done(transactionPack); + + // Clean up the transaction result after listeners have processed it + if (transactionResult != null) { + transactionResult.cleanAfterSample(); + } + return transactionResult; } @@ -1038,7 +1044,7 @@ void notifyTestListeners() { } private void notifyListeners(List listeners, SampleResult result) { - SampleEvent event = new SampleEvent(result, threadGroup.getName(), threadVars); + SampleEvent event = new SampleEvent(result.cloneForListeners(), threadGroup.getName(), threadVars); notifier.notifyListeners(event, listeners); } From afc2337f059dd8ff1dcb8f80cb7e8f4fe0cb5722 Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Wed, 4 Dec 2024 09:43:11 +0100 Subject: [PATCH 2/7] Fix styling --- .../apache/jmeter/samplers/SampleResult.java | 26 +++++++++---------- .../apache/jmeter/threads/JMeterThread.java | 4 +-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java index a9ee530588c..2675bd75661 100644 --- a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java +++ b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java @@ -30,12 +30,12 @@ import java.util.concurrent.TimeUnit; import org.apache.jmeter.assertions.AssertionResult; +import org.apache.jmeter.control.TransactionSampler; import org.apache.jmeter.gui.Searchable; import org.apache.jmeter.testelement.TestPlan; import org.apache.jmeter.threads.JMeterContext; -import org.apache.jmeter.threads.JMeterContextService; import org.apache.jmeter.threads.JMeterContext.TestLogicalAction; -import org.apache.jmeter.control.TransactionSampler; +import org.apache.jmeter.threads.JMeterContextService; import org.apache.jmeter.util.JMeterUtils; import org.apache.jorphan.util.JOrphanUtils; import org.slf4j.Logger; @@ -1605,7 +1605,7 @@ public void cleanAfterSample() { if (parent != null) { return; } - + // Check if we're part of a parent transaction by walking up the sampler hierarchy JMeterContext context = JMeterContextService.getContext(); if (context != null) { @@ -1620,14 +1620,14 @@ public void cleanAfterSample() { } } } - + cleanRecursively(); } - + /** * Internal method to clean this result and all its sub-results */ - private void cleanRecursively() { + private void cleanRecursively() { // Clean sub-results first if (subResults != null) { for (SampleResult subResult : subResults) { @@ -1638,13 +1638,13 @@ private void cleanRecursively() { subResults.clear(); subResults = null; } - + // Clean assertion results if (assertionResults != null) { assertionResults.clear(); assertionResults = null; } - + // Clear only memory-heavy data and caches, preserve samplerData this.parent = null; this.responseDataAsString = null; @@ -1736,12 +1736,12 @@ public SampleResult cloneForListeners() { log.debug("Could not create clone with type: " + this.getClass().getName() + ", using base class", e); clone = new SampleResult(this); } - + // Deep copy mutable fields that the copy constructor doesn't handle deeply if (responseData != EMPTY_BA) { clone.responseData = responseData.clone(); } - + // Deep copy subResults if (subResults != null) { clone.subResults = new ArrayList<>(subResults.size()); @@ -1751,7 +1751,7 @@ public SampleResult cloneForListeners() { clone.subResults.add(subClone); } } - + // Deep copy assertion results if (assertionResults != null) { clone.assertionResults = new ArrayList<>(assertionResults.size()); @@ -1759,11 +1759,11 @@ public SampleResult cloneForListeners() { clone.assertionResults.add(assertionResult); // AssertionResult is immutable } } - + // Clear only the caches and unnecessary references in the clone clone.responseDataAsString = null; clone.parent = null; // Parent reference not needed in the clone - + return clone; } } diff --git a/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java b/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java index fae46708c2e..3efca9d13a6 100644 --- a/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java +++ b/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java @@ -678,12 +678,12 @@ private SampleResult doEndTransactionSampler( notifyListeners(transactionPack.getSampleListeners(), transactionResult); } compiler.done(transactionPack); - + // Clean up the transaction result after listeners have processed it if (transactionResult != null) { transactionResult.cleanAfterSample(); } - + return transactionResult; } From 78a51ceb09cb43dca8418abc9bc62581a30b39be Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Sun, 8 Dec 2024 16:47:11 +0100 Subject: [PATCH 3/7] Revert "doc: update documentation for sampleresult.default.encoding default value" This reverts commit 4340b2a048b04169b98d54f6a1a1fb4ca7059cc7. --- bin/jmeter.properties | 4 +-- .../apache/jmeter/samplers/SampleResult.java | 7 +++- xdocs/changes.xml | 35 +++++++++++++++++++ xdocs/usermanual/properties_reference.xml | 2 +- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/bin/jmeter.properties b/bin/jmeter.properties index bac04e85bc9..db56807105e 100644 --- a/bin/jmeter.properties +++ b/bin/jmeter.properties @@ -1101,8 +1101,8 @@ csvdataset.file.encoding_list=UTF-8|UTF-16|ISO-8859-15|US-ASCII # List of extra HTTP methods that should be available in select box #httpsampler.user_defined_methods=VERSION-CONTROL,REPORT,CHECKOUT,CHECKIN,UNCHECKOUT,MKWORKSPACE,UPDATE,LABEL,MERGE,BASELINE-CONTROL,MKACTIVITY -# The encoding to be used if none is provided (default UTF-8 since JMeter 5.6.1) -#sampleresult.default.encoding=UTF-8 +# The encoding to be used if none is provided (default ISO-8859-1) +#sampleresult.default.encoding=ISO-8859-1 # CookieManager behaviour - should cookies with null/empty values be deleted? # Default is true. Use false to revert to original behaviour diff --git a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java index 2675bd75661..189f49e3d0f 100644 --- a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java +++ b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java @@ -66,10 +66,15 @@ public class SampleResult implements Serializable, Cloneable, Searchable { private static final String INVALID_CALL_SEQUENCE_MSG = "Invalid call sequence"; // $NON-NLS-1$ + // Bug 33196 - encoding ISO-8859-1 is only suitable for Western countries + // However the suggested System.getProperty("file.encoding") is Cp1252 on + // Windows + // So use a new property with the original value as default + // needs to be accessible from test code /** * The default encoding to be used to decode the responseData byte array. * The value is defined by the property "sampleresult.default.encoding" - * with a default of {@link #DEFAULT_HTTP_ENCODING} if that is not defined. + * with a default of DEFAULT_HTTP_ENCODING if that is not defined. */ protected static final String DEFAULT_ENCODING = JMeterUtils.getPropDefault("sampleresult.default.encoding", // $NON-NLS-1$ diff --git a/xdocs/changes.xml b/xdocs/changes.xml index f50ac08b508..20182c4832e 100644 --- a/xdocs/changes.xml +++ b/xdocs/changes.xml @@ -69,6 +69,41 @@ Summary Timer relative to start of Thread Group instead of the start of the test.
  • 63576358 Ensure writable directories when copying template files while report generation.
    • +New and Noteworthy + +Improvements + +

    HTTP Samplers and Test Script Recorder

    +
      +
    • 6010Use UTF-8 as a default encoding in HTTP sampler. It enables sending parameter names, and filenames with unicode characters
      • +
    • 6010Test Recorder will use UTF-8 encoding by default, so it will infer human-readable arguments rather than percent-encoded ones
      • +
    + +Non-functional changes +
      +
    • 6000Add release-drafter for populating GitHub releases info based on the merged PRs
      • +
    • 5989Use Gradle toolchains for JDK provisioning, enable building and testing with different JDKs, start testing with Java 21
      • +
    • 5991Update jackson-core, jackson-databind, jackson-annotations to 2.15.2 (from 2.15.1)
      • +
    • 5993Update ph-commons to 10.2.5 (from 10.2.4)
      • +
    • 6017Update kotlin-stdlib to 1.8.22 (from 1.8.21)
      • +
    • 6020Update error_prone_annotations to 2.20.0 (from 2.19.1)
      • +
    • 6023Update checker-qual to 3.35.0 (from 3.34.0)
      • +
    + + + +Bug fixes + +

    Thread Groups

    +
      +
    • 6011Regression since 5.6: ThreadGroups are running endlessly in non-gui mode: use default value + for LoopController.continue_forever rather than initializing it in the constructor
      • +
    + +

    Other Samplers

    +
      +
    • 6012 Java Request sampler cannot be enabled again after disabling in UI (regression since 5.6)
      • +
    diff --git a/xdocs/usermanual/properties_reference.xml b/xdocs/usermanual/properties_reference.xml index d74ece0613d..8fc816a5c04 100644 --- a/xdocs/usermanual/properties_reference.xml +++ b/xdocs/usermanual/properties_reference.xml @@ -1399,7 +1399,7 @@ JMETER-SERVER The encoding to be used if none is provided.
    - Defaults to: UTF-8 (since 5.6.1) + Defaults to: ISO-8859-1     CookieManager behaviour - should cookies with null/empty values be deleted?
    From cbf186f8372a82580fb308b5232ba5b8812fba74 Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Tue, 10 Dec 2024 13:35:07 +0100 Subject: [PATCH 4/7] Clear transaction from subresults --- .../apache/jmeter/samplers/SampleResult.java | 110 +----------------- .../apache/jmeter/threads/JMeterThread.java | 8 +- .../apache/jmeter/threads/TestCompiler.java | 13 ++- .../org/apache/jmeter/junit/JMeterTest.java | 3 +- 4 files changed, 17 insertions(+), 117 deletions(-) diff --git a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java index 189f49e3d0f..66589162432 100644 --- a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java +++ b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java @@ -30,12 +30,9 @@ import java.util.concurrent.TimeUnit; import org.apache.jmeter.assertions.AssertionResult; -import org.apache.jmeter.control.TransactionSampler; import org.apache.jmeter.gui.Searchable; import org.apache.jmeter.testelement.TestPlan; -import org.apache.jmeter.threads.JMeterContext; import org.apache.jmeter.threads.JMeterContext.TestLogicalAction; -import org.apache.jmeter.threads.JMeterContextService; import org.apache.jmeter.util.JMeterUtils; import org.apache.jorphan.util.JOrphanUtils; import org.slf4j.Logger; @@ -66,15 +63,10 @@ public class SampleResult implements Serializable, Cloneable, Searchable { private static final String INVALID_CALL_SEQUENCE_MSG = "Invalid call sequence"; // $NON-NLS-1$ - // Bug 33196 - encoding ISO-8859-1 is only suitable for Western countries - // However the suggested System.getProperty("file.encoding") is Cp1252 on - // Windows - // So use a new property with the original value as default - // needs to be accessible from test code /** * The default encoding to be used to decode the responseData byte array. * The value is defined by the property "sampleresult.default.encoding" - * with a default of DEFAULT_HTTP_ENCODING if that is not defined. + * with a default of {@link #DEFAULT_HTTP_ENCODING} if that is not defined. */ protected static final String DEFAULT_ENCODING = JMeterUtils.getPropDefault("sampleresult.default.encoding", // $NON-NLS-1$ @@ -1602,62 +1594,10 @@ public void setStartNextThreadLoop(boolean startNextThreadLoop) { } /** - * Clean up cached data, but only if this is a root result (no parent) - * and not part of a parent transaction. + * Clean up cached data */ public void cleanAfterSample() { - // Only clean if this is a root result (no parent) - if (parent != null) { - return; - } - - // Check if we're part of a parent transaction by walking up the sampler hierarchy - JMeterContext context = JMeterContextService.getContext(); - if (context != null) { - Sampler currentSampler = context.getCurrentSampler(); - if (currentSampler instanceof TransactionSampler) { - TransactionSampler transSampler = (TransactionSampler) currentSampler; - // Get the parent sampler from the transaction - Sampler parentSampler = transSampler.getSubSampler(); - // If there's a parent sampler and it's a transaction, we're nested - if (parentSampler instanceof TransactionSampler) { - return; - } - } - } - - cleanRecursively(); - } - - /** - * Internal method to clean this result and all its sub-results - */ - private void cleanRecursively() { - // Clean sub-results first - if (subResults != null) { - for (SampleResult subResult : subResults) { - if (subResult != null) { - subResult.cleanRecursively(); - } - } - subResults.clear(); - subResults = null; - } - - // Clean assertion results - if (assertionResults != null) { - assertionResults.clear(); - assertionResults = null; - } - - // Clear only memory-heavy data and caches, preserve samplerData - this.parent = null; this.responseDataAsString = null; - this.responseData = EMPTY_BA; - this.responseHeaders = ""; - this.requestHeaders = ""; - this.samplerData = null; - } @Override @@ -1725,50 +1665,4 @@ public TestLogicalAction getTestLogicalAction() { public void setTestLogicalAction(TestLogicalAction testLogicalAction) { this.testLogicalAction = testLogicalAction; } - - /** - * Create a deep copy for async listeners - * @return A deep copy of this result - */ - public SampleResult cloneForListeners() { - // Create clone with the correct type - SampleResult clone; - try { - // Use the same constructor that was used to create this instance - clone = this.getClass().getConstructor(this.getClass()).newInstance(this); - } catch (Exception e) { - // Fallback to base class if constructor is not available - log.debug("Could not create clone with type: " + this.getClass().getName() + ", using base class", e); - clone = new SampleResult(this); - } - - // Deep copy mutable fields that the copy constructor doesn't handle deeply - if (responseData != EMPTY_BA) { - clone.responseData = responseData.clone(); - } - - // Deep copy subResults - if (subResults != null) { - clone.subResults = new ArrayList<>(subResults.size()); - for (SampleResult sub : subResults) { - SampleResult subClone = sub.cloneForListeners(); - subClone.setParent(clone); - clone.subResults.add(subClone); - } - } - - // Deep copy assertion results - if (assertionResults != null) { - clone.assertionResults = new ArrayList<>(assertionResults.size()); - for (AssertionResult assertionResult : assertionResults) { - clone.assertionResults.add(assertionResult); // AssertionResult is immutable - } - } - - // Clear only the caches and unnecessary references in the clone - clone.responseDataAsString = null; - clone.parent = null; // Parent reference not needed in the clone - - return clone; - } } diff --git a/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java b/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java index 3efca9d13a6..8b7f13f7900 100644 --- a/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java +++ b/src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java @@ -678,12 +678,6 @@ private SampleResult doEndTransactionSampler( notifyListeners(transactionPack.getSampleListeners(), transactionResult); } compiler.done(transactionPack); - - // Clean up the transaction result after listeners have processed it - if (transactionResult != null) { - transactionResult.cleanAfterSample(); - } - return transactionResult; } @@ -1044,7 +1038,7 @@ void notifyTestListeners() { } private void notifyListeners(List listeners, SampleResult result) { - SampleEvent event = new SampleEvent(result.cloneForListeners(), threadGroup.getName(), threadVars); + SampleEvent event = new SampleEvent(result, threadGroup.getName(), threadVars); notifier.notifyListeners(event, listeners); } diff --git a/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java b/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java index f212dda90a9..233cbb2ad17 100644 --- a/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java +++ b/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java @@ -115,10 +115,21 @@ public SamplePackage configureTransactionSampler(TransactionSampler transactionS } /** - * Reset pack to its initial state + * Reset pack to its initial state and clean up transaction results if needed * @param pack the {@link SamplePackage} to reset */ public void done(SamplePackage pack) { + Sampler sampler = pack.getSampler(); + if (sampler instanceof TransactionSampler) { + TransactionSampler transactionSampler = (TransactionSampler) sampler; + TransactionController controller = transactionSampler.getTransactionController(); + if (transactionSampler.isTransactionDone()) { + // Create new sampler for next iteration + TransactionSampler newSampler = new TransactionSampler(controller, transactionSampler.getName()); + SamplePackage newPack = transactionControllerConfigMap.get(controller); + newPack.setSampler(newSampler); + } + } pack.recoverRunningVersion(); } diff --git a/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java b/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java index 5fd25546bb7..798f83ce62a 100644 --- a/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java +++ b/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java @@ -581,7 +581,8 @@ static Stream serializableObjects() throws Throwable { return getObjects(Serializable.class) .stream() .map(Serializable.class::cast) - .filter(o -> !o.getClass().getName().endsWith("_Stub")); + .filter(o -> !o.getClass().getName().endsWith("_Stub")) + .filter(o -> o.getClass().getName().startsWith("org.apache.jmeter.")); } /* From 47cbb0575281be9ba62f51f9a131d3e2998ab41d Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Tue, 10 Dec 2024 13:44:24 +0100 Subject: [PATCH 5/7] Revert "Revert "doc: update documentation for sampleresult.default.encoding default value"" This reverts commit 78a51ceb09cb43dca8418abc9bc62581a30b39be. --- bin/jmeter.properties | 4 +-- xdocs/changes.xml | 35 ----------------------- xdocs/usermanual/properties_reference.xml | 2 +- 3 files changed, 3 insertions(+), 38 deletions(-) diff --git a/bin/jmeter.properties b/bin/jmeter.properties index db56807105e..bac04e85bc9 100644 --- a/bin/jmeter.properties +++ b/bin/jmeter.properties @@ -1101,8 +1101,8 @@ csvdataset.file.encoding_list=UTF-8|UTF-16|ISO-8859-15|US-ASCII # List of extra HTTP methods that should be available in select box #httpsampler.user_defined_methods=VERSION-CONTROL,REPORT,CHECKOUT,CHECKIN,UNCHECKOUT,MKWORKSPACE,UPDATE,LABEL,MERGE,BASELINE-CONTROL,MKACTIVITY -# The encoding to be used if none is provided (default ISO-8859-1) -#sampleresult.default.encoding=ISO-8859-1 +# The encoding to be used if none is provided (default UTF-8 since JMeter 5.6.1) +#sampleresult.default.encoding=UTF-8 # CookieManager behaviour - should cookies with null/empty values be deleted? # Default is true. Use false to revert to original behaviour diff --git a/xdocs/changes.xml b/xdocs/changes.xml index 20182c4832e..f50ac08b508 100644 --- a/xdocs/changes.xml +++ b/xdocs/changes.xml @@ -69,41 +69,6 @@ Summary Timer relative to start of Thread Group instead of the start of the test.
  • 63576358 Ensure writable directories when copying template files while report generation.
    • -New and Noteworthy - -Improvements - -

    HTTP Samplers and Test Script Recorder

    -
      -
    • 6010Use UTF-8 as a default encoding in HTTP sampler. It enables sending parameter names, and filenames with unicode characters
      • -
    • 6010Test Recorder will use UTF-8 encoding by default, so it will infer human-readable arguments rather than percent-encoded ones
      • -
    - -Non-functional changes -
      -
    • 6000Add release-drafter for populating GitHub releases info based on the merged PRs
      • -
    • 5989Use Gradle toolchains for JDK provisioning, enable building and testing with different JDKs, start testing with Java 21
      • -
    • 5991Update jackson-core, jackson-databind, jackson-annotations to 2.15.2 (from 2.15.1)
      • -
    • 5993Update ph-commons to 10.2.5 (from 10.2.4)
      • -
    • 6017Update kotlin-stdlib to 1.8.22 (from 1.8.21)
      • -
    • 6020Update error_prone_annotations to 2.20.0 (from 2.19.1)
      • -
    • 6023Update checker-qual to 3.35.0 (from 3.34.0)
      • -
    - - - -Bug fixes - -

    Thread Groups

    -
      -
    • 6011Regression since 5.6: ThreadGroups are running endlessly in non-gui mode: use default value - for LoopController.continue_forever rather than initializing it in the constructor
      • -
    - -

    Other Samplers

    -
      -
    • 6012 Java Request sampler cannot be enabled again after disabling in UI (regression since 5.6)
      • -
    diff --git a/xdocs/usermanual/properties_reference.xml b/xdocs/usermanual/properties_reference.xml index 8fc816a5c04..d74ece0613d 100644 --- a/xdocs/usermanual/properties_reference.xml +++ b/xdocs/usermanual/properties_reference.xml @@ -1399,7 +1399,7 @@ JMETER-SERVER     The encoding to be used if none is provided.
    - Defaults to: ISO-8859-1 + Defaults to: UTF-8 (since 5.6.1)     CookieManager behaviour - should cookies with null/empty values be deleted?
    From 3fbe4503f3e06e6c8a66cbd15039d9b495677603 Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Wed, 11 Dec 2024 22:30:39 +0100 Subject: [PATCH 6/7] Store raw body in responseData and only decompress when responseBody is accessed --- src/core/build.gradle.kts | 1 + .../apache/jmeter/samplers/SampleResult.java | 96 ++++++++++++++++++- src/protocol/http/build.gradle.kts | 1 - .../protocol/http/sampler/HTTPHC4Impl.java | 89 ++--------------- .../protocol/http/sampler/HTTPJavaImpl.java | 26 +---- 5 files changed, 105 insertions(+), 108 deletions(-) diff --git a/src/core/build.gradle.kts b/src/core/build.gradle.kts index 98141cdcab4..e788e130724 100644 --- a/src/core/build.gradle.kts +++ b/src/core/build.gradle.kts @@ -114,6 +114,7 @@ dependencies { isTransitive = false } implementation("org.apache.xmlgraphics:xmlgraphics-commons") + implementation("org.brotli:dec") implementation("org.freemarker:freemarker") implementation("org.jodd:jodd-core") implementation("org.jodd:jodd-props") diff --git a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java index 66589162432..3b0a53f784c 100644 --- a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java +++ b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java @@ -17,6 +17,10 @@ package org.apache.jmeter.samplers; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.io.Serializable; import java.io.UnsupportedEncodingException; import java.net.HttpURLConnection; @@ -25,9 +29,13 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.zip.GZIPInputStream; +import java.util.zip.Inflater; +import java.util.zip.InflaterInputStream; import org.apache.jmeter.assertions.AssertionResult; import org.apache.jmeter.gui.Searchable; @@ -160,6 +168,8 @@ public class SampleResult implements Serializable, Cloneable, Searchable { private byte[] responseData = EMPTY_BA; + private String contentEncoding; // Stores gzip/deflate encoding if response is compressed + private String responseCode = "";// Never return null private String label = "";// Never return null @@ -217,7 +227,7 @@ public class SampleResult implements Serializable, Cloneable, Searchable { // TODO do contentType and/or dataEncoding belong in HTTPSampleResult instead? private String dataEncoding;// (is this really the character set?) e.g. - // ISO-8895-1, UTF-8 + // ISO-8895-1, UTF-8 private String contentType = ""; // e.g. text/html; charset=utf-8 @@ -791,6 +801,27 @@ public void setResponseData(final String response, final String encoding) { * @return the responseData value (cannot be null) */ public byte[] getResponseData() { + if (responseData == null) { + return EMPTY_BA; + } + if (contentEncoding != null && responseData.length > 0) { + try { + switch (contentEncoding.toLowerCase(Locale.ROOT)) { + case "gzip": + return decompressGzip(responseData); + case "x-gzip": + return decompressGzip(responseData); + case "deflate": + return decompressDeflate(responseData); + case "br": + return decompressBrotli(responseData); + default: + return responseData; + } + } catch (IOException e) { + log.warn("Failed to decompress response data", e); + } + } return responseData; } @@ -802,12 +833,12 @@ public byte[] getResponseData() { public String getResponseDataAsString() { try { if(responseDataAsString == null) { - responseDataAsString= new String(responseData,getDataEncodingWithDefault()); + responseDataAsString= new String(getResponseData(),getDataEncodingWithDefault()); } return responseDataAsString; } catch (UnsupportedEncodingException e) { log.warn("Using platform default as {} caused {}", getDataEncodingWithDefault(), e.getLocalizedMessage()); - return new String(responseData,Charset.defaultCharset()); // N.B. default charset is used deliberately here + return new String(getResponseData(),Charset.defaultCharset()); // N.B. default charset is used deliberately here } } @@ -1665,4 +1696,63 @@ public TestLogicalAction getTestLogicalAction() { public void setTestLogicalAction(TestLogicalAction testLogicalAction) { this.testLogicalAction = testLogicalAction; } + + /** + * Sets the response data and its compression encoding. + * @param data The response data + * @param encoding The content encoding (e.g. gzip, deflate) + */ + public void setResponseData(byte[] data, String encoding) { + responseData = data == null ? EMPTY_BA : data; + contentEncoding = encoding; + responseDataAsString = null; + } + + private static byte[] decompressGzip(byte[] in) throws IOException { + try (GZIPInputStream gis = new GZIPInputStream(new ByteArrayInputStream(in)); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] buf = new byte[8192]; + int len; + while ((len = gis.read(buf)) > 0) { + out.write(buf, 0, len); + } + return out.toByteArray(); + } + } + + private static byte[] decompressDeflate(byte[] in) throws IOException { + // Try with ZLIB wrapper first + try { + return decompressWithInflater(in, false); + } catch (IOException e) { + // If that fails, try with NO_WRAP for raw DEFLATE + return decompressWithInflater(in, true); + } + } + + private static byte[] decompressWithInflater(byte[] in, boolean nowrap) throws IOException { + try (InflaterInputStream iis = new InflaterInputStream( + new ByteArrayInputStream(in), + new Inflater(nowrap)); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] buf = new byte[8192]; + int len; + while ((len = iis.read(buf)) > 0) { + out.write(buf, 0, len); + } + return out.toByteArray(); + } + } + + private static byte[] decompressBrotli(byte[] in) throws IOException { + try (InputStream bis = new org.brotli.dec.BrotliInputStream(new ByteArrayInputStream(in)); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] buf = new byte[8192]; + int len; + while ((len = bis.read(buf)) > 0) { + out.write(buf, 0, len); + } + return out.toByteArray(); + } + } } diff --git a/src/protocol/http/build.gradle.kts b/src/protocol/http/build.gradle.kts index 2b1dd7f1aca..2dfe96039dd 100644 --- a/src/protocol/http/build.gradle.kts +++ b/src/protocol/http/build.gradle.kts @@ -73,7 +73,6 @@ dependencies { implementation("dnsjava:dnsjava") implementation("org.apache.httpcomponents:httpmime") implementation("org.apache.httpcomponents:httpcore") - implementation("org.brotli:dec") implementation("com.miglayout:miglayout-swing") implementation("com.fasterxml.jackson.core:jackson-core") implementation("com.fasterxml.jackson.core:jackson-databind") diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java index bc4478eecb3..7cdf7f2482d 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java @@ -57,7 +57,6 @@ import org.apache.http.HttpRequest; import org.apache.http.HttpRequestInterceptor; import org.apache.http.HttpResponse; -import org.apache.http.HttpResponseInterceptor; import org.apache.http.NameValuePair; import org.apache.http.StatusLine; import org.apache.http.auth.AuthSchemeProvider; @@ -72,7 +71,6 @@ import org.apache.http.client.config.AuthSchemes; import org.apache.http.client.config.CookieSpecs; import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.entity.InputStreamFactory; import org.apache.http.client.entity.UrlEncodedFormEntity; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; @@ -86,7 +84,6 @@ import org.apache.http.client.methods.HttpTrace; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.client.protocol.ResponseContentEncoding; import org.apache.http.config.Lookup; import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; @@ -147,8 +144,6 @@ import org.apache.jmeter.protocol.http.control.DynamicKerberosSchemeFactory; import org.apache.jmeter.protocol.http.control.DynamicSPNegoSchemeFactory; import org.apache.jmeter.protocol.http.control.HeaderManager; -import org.apache.jmeter.protocol.http.sampler.hc.LaxDeflateInputStream; -import org.apache.jmeter.protocol.http.sampler.hc.LaxGZIPInputStream; import org.apache.jmeter.protocol.http.sampler.hc.LazyLayeredConnectionSocketFactory; import org.apache.jmeter.protocol.http.util.ConversionUtils; import org.apache.jmeter.protocol.http.util.HTTPArgument; @@ -166,7 +161,6 @@ import org.apache.jmeter.util.JsseSSLManager; import org.apache.jmeter.util.SSLManager; import org.apache.jorphan.util.JOrphanUtils; -import org.brotli.dec.BrotliInputStream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -195,20 +189,8 @@ public class HTTPHC4Impl extends HTTPHCAbstractImpl { private static final boolean DISABLE_DEFAULT_UA = JMeterUtils.getPropDefault("httpclient4.default_user_agent_disabled", false); - private static final boolean GZIP_RELAX_MODE = JMeterUtils.getPropDefault("httpclient4.gzip_relax_mode", false); - - private static final boolean DEFLATE_RELAX_MODE = JMeterUtils.getPropDefault("httpclient4.deflate_relax_mode", false); - private static final Logger log = LoggerFactory.getLogger(HTTPHC4Impl.class); - private static final InputStreamFactory GZIP = - instream -> new LaxGZIPInputStream(instream, GZIP_RELAX_MODE); - - private static final InputStreamFactory DEFLATE = - instream -> new LaxDeflateInputStream(instream, DEFLATE_RELAX_MODE); - - private static final InputStreamFactory BROTLI = BrotliInputStream::new; - private static final class ManagedCredentialsProvider implements CredentialsProvider { private final AuthManager authManager; private final Credentials proxyCredentials; @@ -472,55 +454,6 @@ protected HttpResponse doSendRequest( } }; - private static final String[] HEADERS_TO_SAVE = new String[]{ - "content-length", - "content-encoding", - "content-md5" - }; - - /** - * Custom implementation that backups headers related to Compressed responses - * that HC core {@link ResponseContentEncoding} removes after uncompressing - * See Bug 59401 - */ - @SuppressWarnings("UnnecessaryAnonymousClass") - private static final HttpResponseInterceptor RESPONSE_CONTENT_ENCODING = new ResponseContentEncoding(createLookupRegistry()) { - @Override - public void process(HttpResponse response, HttpContext context) - throws HttpException, IOException { - ArrayList headersToSave = null; - - final HttpEntity entity = response.getEntity(); - final HttpClientContext clientContext = HttpClientContext.adapt(context); - final RequestConfig requestConfig = clientContext.getRequestConfig(); - // store the headers if necessary - if (requestConfig.isContentCompressionEnabled() && entity != null && entity.getContentLength() != 0) { - final Header ceheader = entity.getContentEncoding(); - if (ceheader != null) { - headersToSave = new ArrayList<>(3); - for(String name : HEADERS_TO_SAVE) { - Header[] hdr = response.getHeaders(name); // empty if none - headersToSave.add(hdr); - } - } - } - - // Now invoke original parent code - super.process(response, clientContext); - // Should this be in a finally ? - if(headersToSave != null) { - for (Header[] headers : headersToSave) { - for (Header headerToRestore : headers) { - if (response.containsHeader(headerToRestore.getName())) { - break; - } - response.addHeader(headerToRestore); - } - } - } - } - }; - /** * 1 HttpClient instance per combination of (HttpClient,HttpClientKey) */ @@ -558,19 +491,6 @@ protected HTTPHC4Impl(HTTPSamplerBase testElement) { super(testElement); } - /** - * Customize to plug Brotli - * @return {@link Lookup} - */ - private static Lookup createLookupRegistry() { - return - RegistryBuilder.create() - .register("br", BROTLI) - .register("gzip", GZIP) - .register("x-gzip", GZIP) - .register("deflate", DEFLATE).build(); - } - /** * Implementation that allows GET method to have a body */ @@ -675,7 +595,12 @@ protected HTTPSampleResult sample(URL url, String method, } HttpEntity entity = httpResponse.getEntity(); if (entity != null) { - res.setResponseData(readResponse(res, entity.getContent(), entity.getContentLength())); + Header contentEncodingHeader = entity.getContentEncoding(); + if (contentEncodingHeader != null) { + res.setResponseData(EntityUtils.toByteArray(entity), contentEncodingHeader.getValue()); + } else { + res.setResponseData(EntityUtils.toByteArray(entity)); + } } res.sampleEnd(); // Done with the sampling proper. @@ -1157,7 +1082,7 @@ private MutableTriple Date: Wed, 11 Dec 2024 23:05:49 +0100 Subject: [PATCH 7/7] Store raw body in responseData and only decompress when responseBody is accessed --- .../org/apache/jmeter/threads/TestCompiler.java | 13 +------------ .../java/org/apache/jmeter/junit/JMeterTest.java | 3 +-- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java b/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java index 233cbb2ad17..f212dda90a9 100644 --- a/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java +++ b/src/core/src/main/java/org/apache/jmeter/threads/TestCompiler.java @@ -115,21 +115,10 @@ public SamplePackage configureTransactionSampler(TransactionSampler transactionS } /** - * Reset pack to its initial state and clean up transaction results if needed + * Reset pack to its initial state * @param pack the {@link SamplePackage} to reset */ public void done(SamplePackage pack) { - Sampler sampler = pack.getSampler(); - if (sampler instanceof TransactionSampler) { - TransactionSampler transactionSampler = (TransactionSampler) sampler; - TransactionController controller = transactionSampler.getTransactionController(); - if (transactionSampler.isTransactionDone()) { - // Create new sampler for next iteration - TransactionSampler newSampler = new TransactionSampler(controller, transactionSampler.getName()); - SamplePackage newPack = transactionControllerConfigMap.get(controller); - newPack.setSampler(newSampler); - } - } pack.recoverRunningVersion(); } diff --git a/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java b/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java index 798f83ce62a..5fd25546bb7 100644 --- a/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java +++ b/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java @@ -581,8 +581,7 @@ static Stream serializableObjects() throws Throwable { return getObjects(Serializable.class) .stream() .map(Serializable.class::cast) - .filter(o -> !o.getClass().getName().endsWith("_Stub")) - .filter(o -> o.getClass().getName().startsWith("org.apache.jmeter.")); + .filter(o -> !o.getClass().getName().endsWith("_Stub")); } /*