From b92a03e0bca4296ea720f294242a55f702485021 Mon Sep 17 00:00:00 2001 From: Tomi Virtanen Date: Mon, 25 May 2026 11:41:47 +0300 Subject: [PATCH 1/2] fix: ignore parens inside think-time sleeps K6TestRefactorer counted ( and ) with no string-awareness, so a literal ) inside a header value (e.g. Chrome sec-ch-ua "Not/A)Brand";v="99") made parenCount drop to zero mid-headers. The think-time sleep() was then injected inside the headers object, breaking JS parsing in combined-scenarios.js when combineScenarios=true. Same fragility applied to brace-counting in JSESSION/Vaadin extraction insertion. Replace countOccurrences with countUnquotedOccurrences that tracks '..', ".." and `..` string state and honors backslash escapes. Fixes: #2253 --- .../loadtest/util/K6TestRefactorer.java | 41 ++++++-- .../loadtest/util/K6TestRefactorerTest.java | 96 +++++++++++++++++++ 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/vaadin-testbench-loadtest/testbench-converter-plugin/src/main/java/com/vaadin/testbench/loadtest/util/K6TestRefactorer.java b/vaadin-testbench-loadtest/testbench-converter-plugin/src/main/java/com/vaadin/testbench/loadtest/util/K6TestRefactorer.java index e0ad261dc..59a743c17 100644 --- a/vaadin-testbench-loadtest/testbench-converter-plugin/src/main/java/com/vaadin/testbench/loadtest/util/K6TestRefactorer.java +++ b/vaadin-testbench-loadtest/testbench-converter-plugin/src/main/java/com/vaadin/testbench/loadtest/util/K6TestRefactorer.java @@ -499,9 +499,9 @@ private List parseRequestsWithTiming(List lines) { } if (inRequest) { - int opens = countOccurrences(line, '('); + int opens = countUnquotedOccurrences(line, '('); parenCount += opens; - parenCount -= countOccurrences(line, ')'); + parenCount -= countUnquotedOccurrences(line, ')'); if (opens > 0) { seenOpenParen = true; } @@ -647,8 +647,8 @@ private String insertJsessionExtraction(String content) { } if (inFirstAppRequest) { - braceCount += countOccurrences(line, '{'); - braceCount -= countOccurrences(line, '}'); + braceCount += countUnquotedOccurrences(line, '{'); + braceCount -= countUnquotedOccurrences(line, '}'); if (braceCount == 0 && line.contains(")")) { insertLineIndex = i; @@ -687,8 +687,8 @@ private String insertVaadinExtraction(String content) { } if (inInitRequest) { - braceCount += countOccurrences(line, '{'); - braceCount -= countOccurrences(line, '}'); + braceCount += countUnquotedOccurrences(line, '{'); + braceCount -= countUnquotedOccurrences(line, '}'); if (braceCount == 0 && line.contains(")")) { insertLineIndex = i; @@ -705,10 +705,35 @@ private String insertVaadinExtraction(String content) { return content; } - private int countOccurrences(String str, char c) { + /** + * Counts occurrences of {@code target} in {@code str}, ignoring any + * occurrences that fall inside a JavaScript string literal delimited by + * {@code '}, {@code "}, or {@code `}. Backslash escapes inside strings are + * honored. This is what keeps a {@code )} inside a header value like + * {@code 'sec-ch-ua': '"Not/A)Brand";v="99"'} from being mistaken for the + * closing paren of an {@code http.get(...)} call. + *

+ * The scan is per-line: {@link HarToK6Converter} escapes + * {@code \n}/{@code \r}/ {@code \t} in URLs, headers, and bodies, so + * generated string literals never span multiple lines. Template-literal + * interpolations ({@code ${...}}) are treated as opaque string content — + * the converter only emits interpolations of bare identifiers, none of + * which contain {@code (}, {@code )}, {@code {}, or {@code }}. + */ + private int countUnquotedOccurrences(String str, char target) { int count = 0; + char stringDelim = 0; for (int i = 0; i < str.length(); i++) { - if (str.charAt(i) == c) { + char c = str.charAt(i); + if (stringDelim != 0) { + if (c == '\\' && i + 1 < str.length()) { + i++; + } else if (c == stringDelim) { + stringDelim = 0; + } + } else if (c == '\'' || c == '"' || c == '`') { + stringDelim = c; + } else if (c == target) { count++; } } diff --git a/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java b/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java index 82a4edee1..718553320 100644 --- a/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java +++ b/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java @@ -144,6 +144,102 @@ export default function() { + refactored); } + @Test + void refactorContent_thinkTimesEnabled_headerValueContainsCloseParen_sleepStaysAtStatementScope() { + K6TestRefactorer refactorer = new K6TestRefactorer( + ThinkTimeConfig.DEFAULT); + + // Reproduces the combineScenarios=true breakage: the Chrome sec-ch-ua + // header value contains a literal `)` inside a single-quoted JS + // string. A non-string-aware paren counter dropped to zero mid-headers + // and the sleep() got injected inside the headers object, producing + // "Unexpected number" parse errors in k6. + String script = """ + import http from 'k6/http' + import { sleep } from 'k6' + export default function() { + // HAR_DELTA_MS: 0 + // Request 1: GET http://localhost:8080/?v-r=init + let response = http.get( + 'http://localhost:8080/?v-r=init', + { + headers: { + 'sec-ch-ua': '"Chromium";v="148", "Not/A)Brand";v="99"', + 'User-Agent': 'Mozilla/5.0' + }, + tags: { name: 'init' } + } + ) + // HAR_DELTA_MS: 200 + // Request 2: POST http://localhost:8080/?v-r=uidl + response = http.post('http://localhost:8080/?v-r=uidl', '{"event":"click"}') + } + """; + + String refactored = refactorer.refactorContent(script); + + // Walk the function body and check that every `sleep(` lands at + // statement scope — paren-balance 0 and brace-balance 1 (the open + // brace of `export default function() { ... }` itself). The walker + // is string- and line-comment-aware so it isn't fooled by `)` inside + // header values or `(` inside `// Request N:` comments. + int functionBodyOpen = refactored + .indexOf("export default function() {"); + assertTrue(functionBodyOpen >= 0, + "Function body not found in refactored output:\n" + refactored); + int scanFrom = functionBodyOpen + + "export default function() {".length(); + + int parenBalance = 0; + int braceBalance = 1; + char stringDelim = 0; + int sleepCount = 0; + + for (int i = scanFrom; i < refactored.length(); i++) { + char c = refactored.charAt(i); + if (stringDelim != 0) { + if (c == '\\' && i + 1 < refactored.length()) { + i++; + } else if (c == stringDelim) { + stringDelim = 0; + } + continue; + } + if (c == '/' && i + 1 < refactored.length() + && refactored.charAt(i + 1) == '/') { + int eol = refactored.indexOf('\n', i); + i = (eol == -1) ? refactored.length() - 1 : eol; + continue; + } + if (c == '\'' || c == '"' || c == '`') { + stringDelim = c; + } else if (c == '(') { + parenBalance++; + } else if (c == ')') { + parenBalance--; + } else if (c == '{') { + braceBalance++; + } else if (c == '}') { + braceBalance--; + } else if (c == 's' && refactored.startsWith("sleep(", i)) { + sleepCount++; + assertEquals(0, parenBalance, "sleep(...) at offset " + i + + " is nested inside open parens (depth " + parenBalance + + ") — likely injected inside an http.get/post(...) call. Output:\n" + + refactored); + assertEquals(1, braceBalance, "sleep(...) at offset " + i + + " is nested inside unbalanced braces (depth " + + braceBalance + + ") — likely injected inside a `headers` or options object. Output:\n" + + refactored); + } + } + + assertTrue(sleepCount > 0, + "Expected at least one sleep(...) call in refactored output (page-read delay after init block):\n" + + refactored); + } + @Test void refactor_readsInputAndWritesOutputFile() throws IOException { Path input = tempDir.resolve("in.js"); From 0eacf29da80ed6f387879c70bfda45c6fc6125ef Mon Sep 17 00:00:00 2001 From: Tomi Virtanen Date: Mon, 25 May 2026 14:55:28 +0300 Subject: [PATCH 2/2] refactor: simplify test --- .../loadtest/util/K6TestRefactorerTest.java | 78 +++++-------------- 1 file changed, 20 insertions(+), 58 deletions(-) diff --git a/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java b/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java index 718553320..f81ff7a7d 100644 --- a/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java +++ b/vaadin-testbench-loadtest/testbench-converter-plugin/src/test/java/com/vaadin/testbench/loadtest/util/K6TestRefactorerTest.java @@ -145,7 +145,7 @@ export default function() { } @Test - void refactorContent_thinkTimesEnabled_headerValueContainsCloseParen_sleepStaysAtStatementScope() { + void refactorContent_thinkTimesEnabled_headerValueContainsCloseParen_sleepIsNotInjectedInsideHeaders() { K6TestRefactorer refactorer = new K6TestRefactorer( ThinkTimeConfig.DEFAULT); @@ -178,65 +178,27 @@ export default function() { String refactored = refactorer.refactorContent(script); - // Walk the function body and check that every `sleep(` lands at - // statement scope — paren-balance 0 and brace-balance 1 (the open - // brace of `export default function() { ... }` itself). The walker - // is string- and line-comment-aware so it isn't fooled by `)` inside - // header values or `(` inside `// Request N:` comments. - int functionBodyOpen = refactored - .indexOf("export default function() {"); - assertTrue(functionBodyOpen >= 0, - "Function body not found in refactored output:\n" + refactored); - int scanFrom = functionBodyOpen - + "export default function() {".length(); - - int parenBalance = 0; - int braceBalance = 1; - char stringDelim = 0; - int sleepCount = 0; - - for (int i = scanFrom; i < refactored.length(); i++) { - char c = refactored.charAt(i); - if (stringDelim != 0) { - if (c == '\\' && i + 1 < refactored.length()) { - i++; - } else if (c == stringDelim) { - stringDelim = 0; - } - continue; - } - if (c == '/' && i + 1 < refactored.length() - && refactored.charAt(i + 1) == '/') { - int eol = refactored.indexOf('\n', i); - i = (eol == -1) ? refactored.length() - 1 : eol; - continue; - } - if (c == '\'' || c == '"' || c == '`') { - stringDelim = c; - } else if (c == '(') { - parenBalance++; - } else if (c == ')') { - parenBalance--; - } else if (c == '{') { - braceBalance++; - } else if (c == '}') { - braceBalance--; - } else if (c == 's' && refactored.startsWith("sleep(", i)) { - sleepCount++; - assertEquals(0, parenBalance, "sleep(...) at offset " + i - + " is nested inside open parens (depth " + parenBalance - + ") — likely injected inside an http.get/post(...) call. Output:\n" + // A sleep() call should be emitted (page-read delay at the init + // block boundary). + assertTrue(refactored.contains("sleep("), + "Expected at least one sleep() call in refactored output:\n" + refactored); - assertEquals(1, braceBalance, "sleep(...) at offset " + i - + " is nested inside unbalanced braces (depth " - + braceBalance - + ") — likely injected inside a `headers` or options object. Output:\n" - + refactored); - } - } - assertTrue(sleepCount > 0, - "Expected at least one sleep(...) call in refactored output (page-read delay after init block):\n" + // The `Not/A)Brand` sec-ch-ua line is the bug trigger. After + // refactoring it must still be immediately followed by the next + // header property — not by a // Think time comment or a sleep() + // call, which would mean the sleep was injected inside the + // headers object. + String triggerLine = "'sec-ch-ua': '\"Chromium\";v=\"148\", \"Not/A)Brand\";v=\"99\"',"; + int triggerIdx = refactored.indexOf(triggerLine); + assertTrue(triggerIdx >= 0, + "Expected sec-ch-ua header line to survive refactoring intact:\n" + + refactored); + String afterTrigger = refactored + .substring(triggerIdx + triggerLine.length()).stripLeading(); + assertTrue(afterTrigger.startsWith("'User-Agent'"), + "sec-ch-ua header was not followed by the User-Agent header — " + + "a sleep() was likely injected inside the headers object:\n" + refactored); }