Skip to content

Don't create exceptions in RegexExtractor on failed extractions #6759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ashrafiucse wants to merge 1 commit into
base: master
Choose a base branch
from
+115 −26
Open

Don't create exceptions in RegexExtractor on failed extractions #6759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 46 additions & 26 deletions src/components/src/main/java/org/apache/jmeter/extractor/RegexExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,7 @@ private void extractWithOroRegex(SampleResult previousResult, JMeterVariables va
String prevString = vars.get(refName + REF_MATCH_NR);
if (prevString != null) {
vars.remove(refName + REF_MATCH_NR);// ensure old value is not left defined
try {
prevCount = Integer.parseInt(prevString);
} catch (NumberFormatException nfe) {
log.warn("Could not parse number: '{}'", prevString);
}
prevCount = parseIntOrDefault(prevString, 0);
}
int matchCount=0;// Number of refName_n variable sets to keep
try {
Expand Down Expand Up @@ -188,11 +184,7 @@ private void extractWithJavaRegex(SampleResult previousResult, JMeterVariables v
String prevString = vars.get(refName + REF_MATCH_NR);
if (prevString != null) {
vars.remove(refName + REF_MATCH_NR);// ensure old value is not left defined
try {
prevCount = Integer.parseInt(prevString);
} catch (NumberFormatException nfe) {
log.warn("Could not parse number: '{}'", prevString);
}
prevCount = parseIntOrDefault(prevString, 0);
}
int matchCount=0;// Number of refName_n variable sets to keep
try {
Expand Down Expand Up @@ -355,11 +347,7 @@ private static void saveGroups(JMeterVariables vars, String basename, MatchResul
String prevString=vars.get(buf.toString());
int previous=0;
if (prevString!=null){
try {
previous=Integer.parseInt(prevString);
} catch (NumberFormatException nfe) {
log.warn("Could not parse number: '{}'.", prevString);
}
previous=parseIntOrDefault(prevString, 0);
}
//Note: match.groups() includes group 0
final int groups = match.groups();
Expand All @@ -384,11 +372,7 @@ private static void saveGroups(JMeterVariables vars, String basename, java.util.
String prevString=vars.get(buf.toString());
int previous=0;
if (prevString!=null){
try {
previous=Integer.parseInt(prevString);
} catch (NumberFormatException nfe) {
log.warn("Could not parse number: '{}'.", prevString);
}
previous=parseIntOrDefault(prevString, 0);
}
//Note: match.groups() includes group 0, groupCount() not
final int groups = match.groupCount() + 1;
Expand All @@ -410,18 +394,54 @@ private static void saveGroups(JMeterVariables vars, String basename, java.util.
* basename_gn, where n=0...# of groups<br/>
* basename_g = number of groups (apart from g0)
*/
/**
* Parses the given string as a signed decimal integer without relying on
* exceptions, so the hot path of failing extractions does not pay for
* exception creation (see issue #6240).
*
* @param s string to parse, may be null, empty or not a number
* @param defaultValue value to return when the string cannot be parsed
* @return the parsed value, or {@code defaultValue} if the string is null,
* empty, not a signed decimal number, or overflows an {@code int}
*/
private static int parseIntOrDefault(String s, int defaultValue) {
if (s != null && !s.isEmpty() && isSignedDigits(s)) {
try {
return Integer.parseInt(s);
} catch (NumberFormatException overflow) {
// Digit string longer than the int range, fall through to the default
}
}
if (s != null && !s.isEmpty()) {
log.warn("Could not parse number: '{}'", s);
}
return defaultValue;
}

private static boolean isSignedDigits(String s) {
char first = s.charAt(0);
int start = first == '-' || first == '+' ? 1 : 0;
if (start == s.length()) {
return false;
}
for (int i = start; i < s.length(); i++) {
char c = s.charAt(i);
if (c < '0' || c > '9') {
return false;
}
}
return true;
}

private static void removeGroups(JMeterVariables vars, String basename) {
StringBuilder buf = new StringBuilder();
buf.append(basename);
buf.append("_g"); // $NON-NLS-1$
int pfxlen=buf.length();
// How many groups are there?
int groups;
try {
groups=Integer.parseInt(vars.get(buf.toString()));
} catch (NumberFormatException e) {
groups=0;
}
// The group-count variable is absent whenever no match has succeeded
// yet, so parsing must cope with null without creating exceptions (#6240)
int groups = parseIntOrDefault(vars.get(buf.toString()), 0);
vars.remove(buf.toString());// Remove the group count
for (int i = 0; i <= groups; i++) {
buf.append(i);
Expand Down
68 changes: 68 additions & 0 deletions src/components/src/test/java/org/apache/jmeter/extractor/TestRegexExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,4 +479,72 @@ public void testScope2() {
final String found = vars.get("regVal");
assertTrue(found.equals("ONE") || found.equals("TWO"));
}

// Tests for #6240: failing extractions must not create exceptions while
// cleaning up the group variables

@Test
public void testNoMatchOnFreshVariablesAppliesDefault() {
extractor.setRegex("nonexistent-(\\d+)");
extractor.setTemplate("$1$");
extractor.setDefaultValue("NOTFOUND");
extractor.setMatchNumber(1);
extractor.process();
assertEquals("NOTFOUND", vars.get("regVal"));
// No group variables may be left behind
assertNull(vars.get("regVal_g"));
assertNull(vars.get("regVal_g0"));
assertNull(vars.get("regVal_g1"));
assertNull(vars.get("regVal_matchNr"));
}

@Test
public void testNoMatchCleansUpPreviousGroupVariables() {
extractor.setRegex("RetCode\">(\\w+)<");
extractor.setTemplate("$1$");
extractor.setMatchNumber(1);
extractor.process();
assertEquals("LIS_OK", vars.get("regVal"));
assertEquals("1", vars.get("regVal_g"));
assertEquals("LIS_OK", vars.get("regVal_g1"));

// Now fail the extraction: previous group variables must be cleaned up
extractor.setRegex("nonexistent-(\\d+)");
extractor.setDefaultValue("NOTFOUND");
extractor.process();
assertEquals("NOTFOUND", vars.get("regVal"));
assertNull(vars.get("regVal_g"));
assertNull(vars.get("regVal_g0"));
assertNull(vars.get("regVal_g1"));
}

@Test
public void testNoMatchWithTamperedGroupCountVariable() {
vars.put("regVal_g", "not-a-number");
vars.put("regVal_g0", "stale");
extractor.setRegex("nonexistent-(\\d+)");
extractor.setTemplate("$1$");
extractor.setDefaultValue("NOTFOUND");
extractor.setMatchNumber(1);
extractor.process();
assertEquals("NOTFOUND", vars.get("regVal"));
assertNull(vars.get("regVal_g"));
assertNull(vars.get("regVal_g0"));
}

@Test
public void testAllMatchesWithTamperedMatchNumberVariable() {
vars.put("regVal_matchNr", "not-a-number");
vars.put("content", "one, two, 3, 45");
extractor.setRegex("(\\d+)");
extractor.setTemplate("$1$");
extractor.setMatchNumber(-1);
extractor.setScopeVariable("content");
extractor.process();
// The tampered counter must not break the extraction
assertEquals("2", vars.get("regVal_matchNr"));
assertEquals("3", vars.get("regVal_1"));
assertEquals("45", vars.get("regVal_2"));
assertNull(vars.get("regVal_3"));
}
}
1 change: 1 addition & 0 deletions xdocs/changes.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Summary
<ch_section>Bug fixes</ch_section>
<h3>General</h3>
<ul>
<li><issue>6240</issue>Regular Expression Extractor created a <code>NumberFormatException</code> (caught and discarded) on every failed extraction. Failed extractions no longer create exceptions when cleaning up the group variables.</li>
<li><pr>6654</pr><issue>6611</issue>Support JDK 25 and above for result collectors with empty file names</li>
<li>Trim whitespace when parsing numeric JMeter properties so accidental spaces do not silently change configuration values.</li>
<li><pr>6372</pr>Fix KeyManager logging when using CLI mode so keystore passwords are not incorrectly reported as missing. Contributed by Patrick Uiterwijk (patrick at puiterwijk.org)</li>
Expand Down