You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "kalayciburak (via GitHub)" <gi...@apache.org> on 2026/09/21 09:53:56 UTC
[PR] Don't fail XML save when sample data contains illegal XML characters [jmeter]
kalayciburak opened a new pull request, #6767:
URL: https://github.com/apache/jmeter/pull/6767
## Description
Configure the Woodstox `XMLOutputFactory` used by `JMeterStaxDriver` to replace characters that are illegal in XML 1.0 (NUL and other C0 controls) instead of aborting the write.
After the xmlpull to Woodstox migration, saving a JTL sample or a JMX test plan that contains those characters throws `WstxIOException` (`Invalid null character` / `Invalid white space character`). Binary request bodies recorded from browsers (for example Mozilla telemetry) hit this path.
## Motivation and Context
Fixes #6761
Woodstox's default `InvalidCharHandler.FailingHandler` rejects those characters. Replacing them with a space keeps the XML well-formed so recording and test-plan save succeed, matching the previous xmlpull behaviour of not failing the save.
## How Has This Been Tested?
- RED then GREEN: `./gradlew :src:core:test --tests org.apache.jmeter.save.SaveServiceInvalidXmlCharTest` — 2/0
- Nearby: `./gradlew :src:core:test --tests org.apache.jmeter.save.SaveServiceInvalidXmlCharTest --tests org.apache.jmeter.save.TestCSVSaveService` — 13/0
- `./gradlew :src:core:style` — BUILD SUCCESSFUL
## Types of changes
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
- [x] My code follows the [code style][style-guide] of this project.
- [x] I have updated the documentation accordingly.
[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Don't fail XML save when sample data contains illegal XML characters [jmeter]
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #6767:
URL: https://github.com/apache/jmeter/pull/6767#discussion_r4098269958
##########
src/core/src/main/kotlin/org/apache/jmeter/save/JMeterStaxDriver.kt:
##########
@@ -17,13 +17,27 @@
package org.apache.jmeter.save
+import com.ctc.wstx.api.InvalidCharHandler
+import com.ctc.wstx.api.WstxOutputProperties
import com.thoughtworks.xstream.io.xml.StaxDriver
import javax.xml.stream.XMLOutputFactory
public class JMeterStaxDriver(
public val xmlHeader: Boolean = true,
public val indent: Boolean = true,
) : StaxDriver() {
- override fun createOutputFactory(): XMLOutputFactory =
- XMLOutputFactoryDelegate(super.createOutputFactory(), xmlHeader = xmlHeader, indent = indent)
+ override fun createOutputFactory(): XMLOutputFactory {
+ val factory = super.createOutputFactory()
Review Comment:
`StaxDriver.createOutputFactory()` returns `XMLOutputFactory.newInstance()`, which is not necessarily Woodstox. A plugin in `lib/ext` that ships another StAX implementation (Aalto, for instance), or a `javax.xml.stream.XMLOutputFactory` system property, selects a different factory. `setProperty` with a Woodstox-only key then throws `IllegalArgumentException`, and every save fails. Please create `WstxOutputFactory` explicitly, or check `isPropertySupported` before setting the property.
##########
src/core/src/main/kotlin/org/apache/jmeter/save/JMeterStaxDriver.kt:
##########
@@ -17,13 +17,27 @@
package org.apache.jmeter.save
+import com.ctc.wstx.api.InvalidCharHandler
+import com.ctc.wstx.api.WstxOutputProperties
import com.thoughtworks.xstream.io.xml.StaxDriver
import javax.xml.stream.XMLOutputFactory
public class JMeterStaxDriver(
public val xmlHeader: Boolean = true,
public val indent: Boolean = true,
) : StaxDriver() {
- override fun createOutputFactory(): XMLOutputFactory =
- XMLOutputFactoryDelegate(super.createOutputFactory(), xmlHeader = xmlHeader, indent = indent)
+ override fun createOutputFactory(): XMLOutputFactory {
+ val factory = super.createOutputFactory()
+ // Woodstox rejects XML 1.0-illegal characters (NUL, C0 controls) by default.
+ // Replace them so JTL/JMX save of binary payloads does not fail (#6761).
+ factory.setProperty(
+ WstxOutputProperties.P_OUTPUT_INVALID_CHAR_HANDLER,
+ InvalidCharHandler.ReplacingHandler(INVALID_XML_CHAR_REPLACEMENT)
+ )
Review Comment:
The handler replaces characters in attribute values too: a sample label `"lab\u0002el"` is written to the JTL as `lb="lab el"`. Please cover that in a test and in the changelog, whichever behavior the rework settles on.
##########
src/core/src/main/kotlin/org/apache/jmeter/save/JMeterStaxDriver.kt:
##########
@@ -17,13 +17,27 @@
package org.apache.jmeter.save
+import com.ctc.wstx.api.InvalidCharHandler
+import com.ctc.wstx.api.WstxOutputProperties
import com.thoughtworks.xstream.io.xml.StaxDriver
import javax.xml.stream.XMLOutputFactory
public class JMeterStaxDriver(
public val xmlHeader: Boolean = true,
public val indent: Boolean = true,
) : StaxDriver() {
- override fun createOutputFactory(): XMLOutputFactory =
- XMLOutputFactoryDelegate(super.createOutputFactory(), xmlHeader = xmlHeader, indent = indent)
+ override fun createOutputFactory(): XMLOutputFactory {
+ val factory = super.createOutputFactory()
+ // Woodstox rejects XML 1.0-illegal characters (NUL, C0 controls) by default.
+ // Replace them so JTL/JMX save of binary payloads does not fail (#6761).
Review Comment:
NUL is itself a C0 control, and tab, line feed, and carriage return are C0 controls that XML 1.0 allows, so "NUL, C0 controls" names a larger set than the one Woodstox rejects. The comment also leaves out the consequence a maintainer needs: the replacement cannot be undone, and it applies to attribute values as well as text. For the current approach, something like:
```kotlin
// Woodstox rejects C0 control characters other than tab, LF, and CR.
// Each one is written as a space, in text and in attribute values, so the original character is lost.
```
##########
src/core/src/main/kotlin/org/apache/jmeter/save/JMeterStaxDriver.kt:
##########
@@ -17,13 +17,27 @@
package org.apache.jmeter.save
+import com.ctc.wstx.api.InvalidCharHandler
+import com.ctc.wstx.api.WstxOutputProperties
import com.thoughtworks.xstream.io.xml.StaxDriver
import javax.xml.stream.XMLOutputFactory
public class JMeterStaxDriver(
public val xmlHeader: Boolean = true,
public val indent: Boolean = true,
) : StaxDriver() {
- override fun createOutputFactory(): XMLOutputFactory =
- XMLOutputFactoryDelegate(super.createOutputFactory(), xmlHeader = xmlHeader, indent = indent)
+ override fun createOutputFactory(): XMLOutputFactory {
+ val factory = super.createOutputFactory()
+ // Woodstox rejects XML 1.0-illegal characters (NUL, C0 controls) by default.
+ // Replace them so JTL/JMX save of binary payloads does not fail (#6761).
+ factory.setProperty(
+ WstxOutputProperties.P_OUTPUT_INVALID_CHAR_HANDLER,
+ InvalidCharHandler.ReplacingHandler(INVALID_XML_CHAR_REPLACEMENT)
+ )
+ return XMLOutputFactoryDelegate(factory, xmlHeader = xmlHeader, indent = indent)
+ }
+
+ private companion object {
+ const val INVALID_XML_CHAR_REPLACEMENT = ' '
+ }
Review Comment:
A companion object for one private `Char` is more structure than the value needs. A file-level `private const val` does the same.
##########
src/core/src/test/java/org/apache/jmeter/save/SaveServiceInvalidXmlCharTest.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jmeter.save;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.ByteArrayOutputStream;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.jmeter.junit.JMeterTestCase;
+import org.apache.jmeter.samplers.SampleEvent;
+import org.apache.jmeter.samplers.SampleResult;
+import org.apache.jmeter.samplers.SampleSaveConfiguration;
+import org.apache.jmeter.testelement.property.StringProperty;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for <a href="https://github.com/apache/jmeter/issues/6761">#6761</a>:
+ * Woodstox rejects characters that are illegal in XML 1.0 when saving JTL/JMX.
+ */
Review Comment:
The comment opens with the defect in the present tense, and after the fix it is no longer true. State the rule the test checks first, then the old defect in one past-tense sentence. With the current behavior, for example:
```java
/**
* A character that XML 1.0 does not allow is saved as a space.
* Saving such a value used to fail with WstxIOException (issue #6761).
*/
```
##########
src/core/src/test/java/org/apache/jmeter/save/SaveServiceInvalidXmlCharTest.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jmeter.save;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.ByteArrayOutputStream;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.jmeter.junit.JMeterTestCase;
+import org.apache.jmeter.samplers.SampleEvent;
+import org.apache.jmeter.samplers.SampleResult;
+import org.apache.jmeter.samplers.SampleSaveConfiguration;
+import org.apache.jmeter.testelement.property.StringProperty;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for <a href="https://github.com/apache/jmeter/issues/6761">#6761</a>:
+ * Woodstox rejects characters that are illegal in XML 1.0 when saving JTL/JMX.
+ */
+public class SaveServiceInvalidXmlCharTest extends JMeterTestCase {
+
+ private static final String ILLEGAL_XML_CHARS = "pre\u0000mid\u001fsuf";
+
+ @Test
+ void saveSampleResultAllowsIllegalXmlCharsInSamplerData() {
+ SampleSaveConfiguration saveConfig = new SampleSaveConfiguration();
+ saveConfig.setSamplerData(true);
+
+ SampleResult result = new SampleResult();
+ result.setSaveConfig(saveConfig);
+ result.setSampleLabel("binary-body");
+ result.setSamplerData(ILLEGAL_XML_CHARS);
+
+ StringWriter writer = new StringWriter();
+ assertDoesNotThrow(() -> SaveService.saveSampleResult(new SampleEvent(result, "tg"), writer));
+
+ String xml = writer.toString();
+ assertFalse(xml.isEmpty(), "JTL output should not be empty");
+ assertFalse(xml.indexOf('\u0000') >= 0, "NUL must not appear in well-formed XML");
+ assertFalse(xml.indexOf('\u001f') >= 0, "C1 control 0x1F must not appear in XML 1.0");
Review Comment:
These assertions still pass if the value is dropped entirely: the output is non-empty and contains no NUL. Please assert the literal that is expected in the output, e.g. `<samplerData class="java.lang.String">pre mid suf</samplerData>`, so the test fails when the written value is wrong.
`assertFalse(xml.indexOf(..) >= 0)` prints only `expected: <false> but was: <true>` on failure. An `assertEquals` against the expected string prints both values. Once the test asserts the output, `assertDoesNotThrow` adds nothing.
##########
xdocs/changes.xml:
##########
@@ -126,6 +126,7 @@ Summary
<li><issue>5937</issue>Remove deprecated Log4j package scanning and configure plugin metadata processing to improve startup time and avoid deprecation warnings. Contributed by Piotr P. Karwasz (github.com/piotrgithub)</li>
<li><pr>6620</pr>Fix report generation paths so dashboard output files are created in the correct location after internal refactoring.</li>
<li><bug>6456</bug>Handle malformed percent-encoded URLs gracefully when recording HTTP traffic, logging a warning instead of failing the recording.</li>
+ <li><issue>6761</issue>Allow XML save of sample results and test plans that contain characters illegal in XML 1.0 (NUL and C0 controls) after the Woodstox migration.</li>
Review Comment:
The Woodstox migration has not been released, so users upgrading from 5.6.3 never saw this failure, and "after the Woodstox migration" describes internal history. For those users, the visible change is what happens to these characters on save: replaced with a space, or kept, depending on the rework. Please describe that instead.
##########
src/core/src/test/java/org/apache/jmeter/save/SaveServiceInvalidXmlCharTest.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jmeter.save;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.ByteArrayOutputStream;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.jmeter.junit.JMeterTestCase;
+import org.apache.jmeter.samplers.SampleEvent;
+import org.apache.jmeter.samplers.SampleResult;
+import org.apache.jmeter.samplers.SampleSaveConfiguration;
+import org.apache.jmeter.testelement.property.StringProperty;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for <a href="https://github.com/apache/jmeter/issues/6761">#6761</a>:
+ * Woodstox rejects characters that are illegal in XML 1.0 when saving JTL/JMX.
+ */
+public class SaveServiceInvalidXmlCharTest extends JMeterTestCase {
+
+ private static final String ILLEGAL_XML_CHARS = "pre\u0000mid\u001fsuf";
+
+ @Test
+ void saveSampleResultAllowsIllegalXmlCharsInSamplerData() {
+ SampleSaveConfiguration saveConfig = new SampleSaveConfiguration();
+ saveConfig.setSamplerData(true);
+
+ SampleResult result = new SampleResult();
+ result.setSaveConfig(saveConfig);
+ result.setSampleLabel("binary-body");
+ result.setSamplerData(ILLEGAL_XML_CHARS);
+
+ StringWriter writer = new StringWriter();
+ assertDoesNotThrow(() -> SaveService.saveSampleResult(new SampleEvent(result, "tg"), writer));
+
+ String xml = writer.toString();
+ assertFalse(xml.isEmpty(), "JTL output should not be empty");
+ assertFalse(xml.indexOf('\u0000') >= 0, "NUL must not appear in well-formed XML");
+ assertFalse(xml.indexOf('\u001f') >= 0, "C1 control 0x1F must not appear in XML 1.0");
+ }
+
+ @Test
+ void saveElementAllowsNulInStringProperty() {
+ StringProperty property = new StringProperty("bin", ILLEGAL_XML_CHARS);
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ assertDoesNotThrow(() -> SaveService.saveElement(property, out));
+
+ String xml = out.toString(StandardCharsets.UTF_8);
+ assertFalse(xml.isEmpty(), "JMX fragment should not be empty");
+ assertFalse(xml.indexOf('\u0000') >= 0, "NUL must not appear in well-formed XML");
+ assertFalse(xml.indexOf('\u001f') >= 0, "C1 control 0x1F must not appear in XML 1.0");
+ }
Review Comment:
What users need is that a plan saved with such a value opens again with a known value. Please add:
- a round-trip test: `saveElement`, then `SaveService.loadElement`, asserting the loaded value;
- a test that loads a 5.6.3-style fragment containing `�` and ``, which fails on the current code.
The name says `Nul`, but the input also contains 0x1F, and with both characters in one string no test checks 0x1F on its own, although it is the character in the first stack trace of the issue. A parameterized test with one case per character, plus a case for a character in an attribute (the sample label), would cover each separately.
##########
src/core/src/test/java/org/apache/jmeter/save/SaveServiceInvalidXmlCharTest.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jmeter.save;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.ByteArrayOutputStream;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.jmeter.junit.JMeterTestCase;
+import org.apache.jmeter.samplers.SampleEvent;
+import org.apache.jmeter.samplers.SampleResult;
+import org.apache.jmeter.samplers.SampleSaveConfiguration;
+import org.apache.jmeter.testelement.property.StringProperty;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for <a href="https://github.com/apache/jmeter/issues/6761">#6761</a>:
+ * Woodstox rejects characters that are illegal in XML 1.0 when saving JTL/JMX.
+ */
+public class SaveServiceInvalidXmlCharTest extends JMeterTestCase {
+
+ private static final String ILLEGAL_XML_CHARS = "pre\u0000mid\u001fsuf";
+
+ @Test
+ void saveSampleResultAllowsIllegalXmlCharsInSamplerData() {
+ SampleSaveConfiguration saveConfig = new SampleSaveConfiguration();
+ saveConfig.setSamplerData(true);
+
+ SampleResult result = new SampleResult();
+ result.setSaveConfig(saveConfig);
+ result.setSampleLabel("binary-body");
+ result.setSamplerData(ILLEGAL_XML_CHARS);
+
+ StringWriter writer = new StringWriter();
+ assertDoesNotThrow(() -> SaveService.saveSampleResult(new SampleEvent(result, "tg"), writer));
+
+ String xml = writer.toString();
+ assertFalse(xml.isEmpty(), "JTL output should not be empty");
+ assertFalse(xml.indexOf('\u0000') >= 0, "NUL must not appear in well-formed XML");
+ assertFalse(xml.indexOf('\u001f') >= 0, "C1 control 0x1F must not appear in XML 1.0");
Review Comment:
0x1F is a C0 control character, not C1 (C1 is 0x80–0x9F). The same message is on line 70.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Preserve illegal XML characters as character references [jmeter]
Posted by "kalayciburak (via GitHub)" <gi...@apache.org>.
kalayciburak commented on PR #6767:
URL: https://github.com/apache/jmeter/pull/6767#issuecomment-6469172968
My earlier reply was premature: I found a regression in 57377f0 where valid U+E000 followed by U+E100 can be read as NUL. I have moved this back to draft while I investigate a fix that preserves both valid XML and the legacy character references.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Preserve illegal XML characters as character references [jmeter]
Posted by "kalayciburak (via GitHub)" <gi...@apache.org>.
kalayciburak commented on PR #6767:
URL: https://github.com/apache/jmeter/pull/6767#issuecomment-6080118616
Thanks, addressed all four points in 57377f0:
- illegal XML characters are written as character references and round-trip through save/load, including attributes and U+FFFE/U+FFFF
- JMeter 5.6.3 character references are accepted on read
- added round-trip, legacy, decimal/padded-reference and attribute coverage
- updated the user manual note
Focused `./gradlew :src:core:test --tests org.apache.jmeter.save.SaveServiceInvalidXmlCharTest --rerun-tasks` passes: 17 completed, 0 failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Preserve illegal XML characters as character references [jmeter]
Posted by "kalayciburak (via GitHub)" <gi...@apache.org>.
kalayciburak commented on PR #6767:
URL: https://github.com/apache/jmeter/pull/6767#issuecomment-5822706557
switched to character references, same as 5.6.3, so save and reload keep the value. 5.6.3 files with � and  load again. pushed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org