Fixed issue where escaped characters in data of curl causes unbalanced quotes by ruthst00 · Pull Request #6773 · apache/jmeter

ruthst00 · 2026-09-24T08:37:55Z

Description

Summary of all changes to BasicCurlParser.java:

Javadoc for translateCommandline: Replaced the one-line "Crack a command line." description with a full <ul> block documenting all four quoting rules: unquoted-space tokenisation, single-quote verbatim semantics, POSIX double-quote escape rules, and outside-quotes backslash behaviour (including the \<LF> line-continuation special case and the \<CR> literal-escape behaviour), plus an explicit note that ANSI-C quoting ($'...') is not supported.
Tokenizer rewritten from StringTokenizer to a character-level loop: The original implementation used new StringTokenizer(toProcess, "\"\' ", true) which split on ", ', and space as single-character delimiters. This made lookahead impossible, so backslash handling required a regex post-processing hack (nextTok.replaceAll("^\\\\[\\r\\n]", "")). The new implementation iterates character by character with an explicit index, enabling correct lookahead for all backslash sequences.
inQuote state (single-quoted strings): The old StringTokenizer-based code had no backslash handling inside single quotes. The new code appends each character literally and exits only on ' — correctly implementing POSIX single-quote semantics where backslash has no special meaning.
inDoubleQuote state (double-quoted strings): The old code had no backslash handling inside double quotes at all; \" would have ended the double-quoted region prematurely. The new code implements full POSIX double-quote escape rules: \ escapes only ", \, $, `, and \n (the escaped character is appended); before \r it is a line-continuation (both discarded); before any other character the \ is kept as a literal.
normal state (outside quotes): The old code used nextTok.replaceAll("^\\\\[\\r\\n]", "") to strip a leading \<CR> or \<LF> — a fragile regex that only worked when the backslash happened to be at the start of a StringTokenizer chunk. The new code adds an explicit \ branch: \<LF> is the sole line-continuation (both characters discarded); any other character after \ — including \<CR> — has the backslash dropped and the character appended literally. A $' guard was also added to throw IllegalArgumentException and explicitly reject ANSI-C quoting.
Changed handling of \ followed by CRLF (on master it produced an extra "\n" argument).

Motivation and Context

Root cause: BasicCurlParser.translateCommandline used StringTokenizer with quote characters as delimiters. A backslash-escaped quote inside a quoted token (e.g., 'tes\'t' or "tes\"t") caused the tokenizer to treat the escaped quote as a closing delimiter, leaving the remainder unbalanced and throwing IllegalArgumentException: unbalanced quotes.

Fixes #6374

How Has This Been Tested?

Here is the complete list of tests added to BasicCurlParserTest:

Created a single @ParameterizedTest testTranslateCommandline with 11 named cases:

"plain unquoted tokens" — curl -X POST http://example.com → 4 tokens
"single-quoted token with space" — curl 'hello world' → 2 tokens
"double-quoted token with space" — curl "hello world" → 2 tokens
"single quote via POSIX idiom 'tes'\''t'" — yields tes't
"escaped double-quote inside double quotes" — "tes\"t" → tes"t
"multiple single quotes via POSIX idiom" — 'it'\''s a test'\''s value' → it's a test's value
"backslash-LF line continuation" — curl <LF>-d 'hey' → 3 tokens
(new) "backslash-CRLF: only LF continuation is supported; CR and LF are appended" — curl <CR><LF>-d 'hey' → ["curl", "\r\n-d", "hey"]
(new) "backslash before closing single quote is literal inside single quotes" — curl -d 'C:\dir\' → ["curl", "-d", "C:\dir\"] (not unbalanced)
(new) "escaped backslashes inside double quotes" — curl -d "C:\\dir\\" → ["curl", "-d", "C:\dir\", "http://x"]
(new) "double-quoted escaped backslash yields single backslash" — "a\\b" → a\b

New standalone @Test methods:

testTranslateCommandlineUnbalancedDoubleQuotesThrows — unbalanced " throws IllegalArgumentException
testTranslateCommandlineUnbalancedSingleQuotesThrows — unbalanced ' throws IllegalArgumentException (previously only double-quote unbalance was tested)

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code follows the [code style][style-guide] of this project.
I have updated the documentation accordingly.

Generated with Claude Sonnet via Cline API Provider

…d quotes.

…is not allowed in apache/jmeter because all actions must be from a repository owned by your enterprise, created by GitHub, or match one of the patterns.

vlsi

The change fixes two inputs from #6374, but it breaks commands that parse correctly on master. It also adds an escaping rule that neither bash nor any browser's "Copy as cURL" uses. I compared translateCommandline from master and from this PR against bash (eval "set -- $cmd"; printf '[%s]' "$@"):

Input	bash	this PR
`curl -d 'C:\dir\' -H 'X: y' http://x`	`C:\dir\`, `X: y`	`IllegalArgumentException: unbalanced quotes` (`master` parses it correctly)
`curl -d "C:\\dir\\" -H "X: y" http://x`	`C:\dir\`, `X: y`	`IllegalArgumentException: unbalanced quotes` (`master` parses it)
`curl -d "a\\b" http://x`	`a\b`	`a\\b`
`curl -d $'tes\'t' http://x`	`tes't`	`$tes't`: the `$` ends up in the request body without any error
`curl -d 'tes'\''t' http://x`	`tes't`	`IllegalArgumentException`
`curl -d tes\'t http://x`	`tes't`	`IllegalArgumentException`
`curl --data 'tes\'t'` (the first example in #6374)	syntax error: unterminated `'`	`tes't`

Please implement the POSIX quoting rules instead of the two special cases:

inside '…', a backslash is a literal character and nothing escapes the closing quote;
inside "…", a backslash escapes only ", \, $, ` and a newline, and stays literal before any other character;
outside quotes, a backslash escapes the next character, and a backslash followed by a newline is a line continuation.

Please also:

update the Javadoc of translateCommandline (Crack a command line.) so that it states which quoting and escaping rules the method supports; it is public API and the contract is currently not written anywhere;
add an entry to xdocs/changes.xml; the checklist says the documentation was updated, but the PR does not touch any documentation;
fix the PR title, which is cut off at "unbalance…" (the rest of it opens the description), and mention in the description the workflow change and the changed handling of \ followed by CRLF (on master it produced an extra "\n" argument);
move the workflow change into a separate PR (see the inline comment).

Tests owed by the change, each with the expected tokens taken from bash rather than from the parser's output:

a backslash before the closing single quote: -d 'C:\dir\' -H 'X: y';
an escaped backslash at the end of a double-quoted string: -d "C:\\dir\\" -H "X: y";
\\ inside double quotes: "a\\b" gives a\b;
a backslash before a character that "…" does not escape: "a\b" keeps the backslash;
an escaped quote outside quotes: tes\'t;
\ followed by CRLF as a line continuation;
'tes'\''t', the way a POSIX shell writes a single quote inside a single-quoted string.

vlsi · 2026-09-24T20:38:44Z

            switch (state) {
                case inQuote -> {
-                    if ("'".equals(nextTok)) {
+                    if (c == '\\' && i + 1 < len && toProcess.charAt(i + 1) == '\'') {


In a POSIX shell, a backslash inside single quotes is a literal character: nothing can escape the closing '. This branch therefore breaks valid commands. curl -d 'C:\dir\' -H 'X: y' http://x passes C:\dir\ in bash and on master, and now throws unbalanced quotes, because the \' swallows the closing quote. The same happens to any regex or JSON value that ends in a backslash.

'tes\'t' from #6374 is not a valid shell word either: bash reports unexpected EOF while looking for matching '. Shells and browsers write a single quote inside single quotes as 'tes'\''t' or as $'tes\'t', and this PR supports neither form. Please drop this branch and follow the POSIX rule; 'tes'\''t' will then parse through the existing states.

vlsi · 2026-09-24T20:38:44Z

                }
                case inDoubleQuote -> {
-                    if ("\"".equals(nextTok)) {
+                    if (c == '\\' && i + 1 < len && toProcess.charAt(i + 1) == '"') {


Inside double quotes, the shell treats a backslash as an escape before ", \, $, ` and a newline, and keeps it as a literal character before anything else. Handling only \" makes a string that ends in an escaped backslash impossible to close: -d "C:\\dir\\" -H "X: y" parsed on master and now throws unbalanced quotes. "a\\b" still yields a\\b instead of a\b. Please implement the full POSIX set for double quotes.

vlsi · 2026-09-24T20:38:44Z

                }
                default -> {
-                    if ("'".equals(nextTok)) {
+                    if (c == '\'') {


$'…' (ANSI-C quoting) is what Chrome's "Copy as cURL (bash)" emits whenever the body contains a single quote or a non-printable character, e.g. --data-raw $'{"name":"O\'Brien"}'. Before this PR such input failed with unbalanced quotes. Now it parses without an error, and the $ becomes part of the request body ($'tes\'t' gives $tes't). Please either support $'…' or keep rejecting it; sending a corrupted body is worse than an error message.

vlsi · 2026-09-24T20:38:44Z

                        }
+                        lastTokenHasBeenQuoted = false;
+                        i++;
+                    } else if (c == '\\' && i + 1 < len


Outside quotes, a POSIX shell treats a backslash as an escape for any character, not only for a line break: tes\'t is tes't. With this change tes\'t still opens a quote and throws.

This branch also changes how \ followed by CRLF is handled: master produced an extra argument "\n", and this code drops it. Please add a test for CRLF and mention the change in the PR description.

vlsi · 2026-09-24T20:38:45Z

            return new String[0];
        }
-        // parse with a simple finite state machine
+        // parse with a character-level finite state machine so that


This comment describes the change (what is now handled "correctly") and uses 'tes\'t', which is not valid shell syntax, as its example. Please describe the supported quoting rules in the method's Javadoc and remove this comment. The comments // escaped single-quote inside single-quoted string, // escaped double-quote inside double-quoted string and // also skip a following \n if we consumed \r only repeat the next line of code and can be removed as well.

vlsi · 2026-09-24T20:38:45Z

+    }
+
+    // -----------------------------------------------------------------------
+    // Direct unit tests for translateCommandline (tokenizer-level coverage)


Please remove the banner comment. The tokenizer cases differ only in the input and the expected tokens, so they fit one @ParameterizedTest with a name per case (for example @MethodSource with Arguments.of("backslash before closing single quote", "-d 'C:\\dir\\'", new String[]{"-d", "C:\\dir\\"})). The expected tokens for each case should come from bash.

vlsi · 2026-09-24T20:38:45Z

+    @Test
+    public void testTranslateCommandlineSimpleTokens() {
+        String[] result = BasicCurlParser.translateCommandline("curl -X POST http://example.com");
+        assertEquals(4, result.length);


assertEquals(4, result.length) followed by per-element assertEquals stops at the first mismatch and never prints the actual tokens. assertArrayEquals(new String[]{"curl", "-X", "POST", "http://example.com"}, result) prints both arrays when it fails. The same pattern appears in every testTranslateCommandline* test.

vlsi · 2026-09-24T20:38:45Z

+
+    /**
+     * Backslash-escaped single-quote inside a single-quoted token must be
+     * treated as a literal single-quote (fix for issue #6374).


(fix for issue #6374) gives the issue number instead of the defect. Please state the rule the test checks and, if needed, the old failure in one sentence (used to throw IllegalArgumentException: unbalanced quotes), with the issue number after it. The same applies to line 853.

vlsi · 2026-09-24T20:38:45Z

+
+    /** Backslash + newline (line continuation) outside quotes is consumed silently. */
+    @Test
+    public void testTranslateCommandlineBackslashLineContinuation() {


This duplicates the existing testBackslashAtLineEnding ("curl \\\n-d 'hey' …"). The case this PR changes is \ followed by \r\n, which has no test.

vlsi · 2026-09-24T20:38:45Z

+        assertEquals(0, BasicCurlParser.translateCommandline("").length);
+    }
+
+    /** Genuinely unbalanced quotes still throw IllegalArgumentException. */


Genuinely adds nothing here or in the assertion message on line 893: Unbalanced quotes throw IllegalArgumentException. The message should give the input (translateCommandline("curl \"unclosed")) rather than repeat the test name.

…ack of PR.

vlsi · 2026-09-25T06:55:53Z

+            // Plain unquoted tokens split on spaces
+            // bash: printf "%s\n" curl -X POST http://example.com
+            //   → curl / -X / POST / http://example.com
+            org.junit.jupiter.params.provider.Arguments.of(


Can this be a static import of Arguments.arguments?

ruthst00 added 2 commits September 24, 2026 04:35

Fixed issue where escaped characters in data of curl causes unbalance…

5a3c228

…d quotes.

Fixed build break where the action gradle/actions/wrapper-validation …

9a673b4

…is not allowed in apache/jmeter because all actions must be from a repository owned by your enterprise, created by GitHub, or match one of the patterns.

vlsi requested changes Sep 24, 2026

View reviewed changes

vlsi mentioned this pull request Sep 24, 2026

Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped #6770

Open

2 tasks

ruthst00 changed the title ~~Fixed issue where escaped characters in data of curl causes unbalance…~~ Fixed issue where escaped characters in data of curl causes unbalanced quotes Sep 25, 2026

Implemented changes to code and tests in response to maintainer feedb…

cfc9338

…ack of PR.

ruthst00 requested a review from vlsi September 25, 2026 06:46

vlsi reviewed Sep 25, 2026

View reviewed changes

Converted org.junit.jupiter.params.provider.Arguments to static import.

a56e515

Conversation

ruthst00 commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

vlsi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ruthst00 commented Sep 24, 2026 •

edited

Loading