You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "milamberspace (via GitHub)" <gi...@apache.org> on 2023/04/20 17:47:02 UTC
[GitHub] [jmeter] milamberspace opened a new pull request, #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
milamberspace opened a new pull request, #5811:
URL: https://github.com/apache/jmeter/pull/5811
… line on special case when key and value are empty, i.e.: "k1=v1&=&k2=v2"
## Description
Fix parsing issue on special case when key and value are empty, i.e.: "k1=v1&=&k2=v2"
## Motivation and Context
Related to https://github.com/apache/jmeter/issues/5807
## How Has This Been Tested?
Run the provided test case from https://github.com/apache/jmeter/issues/5807 and followed the instructions given there.
## Screenshots (if appropriate):
## 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.
--
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
[GitHub] [jmeter] milamberspace closed pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace closed pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
URL: https://github.com/apache/jmeter/pull/5811
--
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
[GitHub] [jmeter] milamberspace closed pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace closed pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
URL: https://github.com/apache/jmeter/pull/5811
--
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
[GitHub] [jmeter] FSchumacher commented on a diff in pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "FSchumacher (via GitHub)" <gi...@apache.org>.
FSchumacher commented on code in PR #5811:
URL: https://github.com/apache/jmeter/pull/5811#discussion_r1173586470
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -316,9 +324,11 @@ public static Map<String, String[]> getQueryMap(String query) {
System.arraycopy(known, 0, tmp, 0, known.length);
known = tmp;
}
- map.put(name, known);
+ if (!noNameAndNoValue) {
Review Comment:
Double negations are hard to read.
Perhaps we could use `validNameAndValue` with a default of `true` and set it to `false`, if `name` or `value` are missing.
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -316,9 +324,11 @@ public static Map<String, String[]> getQueryMap(String query) {
System.arraycopy(known, 0, tmp, 0, known.length);
known = tmp;
}
- map.put(name, known);
+ if (!noNameAndNoValue) {
+ map.put(name, known);
+ }
}
-
+ System.out.println("i: " + i + " map size: " + map.size());
Review Comment:
Probably not needed.
##########
src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTPTest.java:
##########
@@ -21,6 +21,7 @@
import org.apache.commons.lang3.StringUtils;
import org.junit.Assert;
Review Comment:
We could change all assertions to the newer jupiter API.
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -285,9 +285,17 @@ public static Map<String, String[]> getQueryMap(String query) {
Map<String, String[]> map = new HashMap<>();
String[] params = query.split(PARAM_CONCATENATE);
+ int i = 0;
for (String param : params) {
+ i++;
String[] paramSplit = param.split("=");
- String name = decodeQuery(paramSplit[0]);
+ String name = "";
Review Comment:
Is this OK? We use `name` further down to combine key/value-pairs with the same key.
In the old implementation we would not get to the point, where name is not initialized, now, we would get there with a default value of `""`.
Maybe use a `null` value of `name` to mean *no valid key* found and use that to deduce the bool `noNameAndNoValue` later (see my other comment about double negations).
##########
src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTPTest.java:
##########
@@ -193,4 +194,29 @@ public void testGetQueryMapSoapHack() {
Assert.assertEquals(query, param1.getValue()[0]);
Assert.assertTrue(StringUtils.isBlank(param1.getKey()));
}
+
+ @Test
Review Comment:
Here a parametrized test could perhaps be used. The test cases are quite similar.
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -285,9 +285,17 @@ public static Map<String, String[]> getQueryMap(String query) {
Map<String, String[]> map = new HashMap<>();
String[] params = query.split(PARAM_CONCATENATE);
+ int i = 0;
for (String param : params) {
+ i++;
Review Comment:
Not needed apart from the println, right?
--
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
[GitHub] [jmeter] milamberspace commented on pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on PR #5811:
URL: https://github.com/apache/jmeter/pull/5811#issuecomment-1516719988
@FSchumacher should be better to fix the issue
In attach a new manual test file
--
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
[GitHub] [jmeter] milamberspace commented on pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on PR #5811:
URL: https://github.com/apache/jmeter/pull/5811#issuecomment-1516721087
JMX file
--
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
[GitHub] [jmeter] milamberspace commented on pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on PR #5811:
URL: https://github.com/apache/jmeter/pull/5811#issuecomment-1516722700
[github_issuer_5807.zip](https://github.com/apache/jmeter/files/11289074/github_issuer_5807.zip)
--
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
[GitHub] [jmeter] milamberspace commented on pull request #5811: Bug #5807 - Fix an ArrayIndexOutOfBoundsException on HTTP parameters …
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on PR #5811:
URL: https://github.com/apache/jmeter/pull/5811#issuecomment-1518591402
Thanks @FSchumacher for review, and good catch for Syso debug line.
I create a new PR #5812
--
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