You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/05/29 23:41:50 UTC
[GitHub] [geode] alb3rtobr opened a new pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
alb3rtobr opened a new pull request #5187:
URL: https://github.com/apache/geode/pull/5187
When parsing the user input in gfsh, the first `=` character position was used to split strings which were expected to follow the pattern `--parameter=value`. But if the `=` sign is not introduced, the `=` is deleted in `--parameter` and `value` (if exists).
In the case of `--query` parameter, the value of the parameter (the query itself) is very likely to contain a `=`, which could be removed from the query.
With this modification, if a query is introduced without the `=` between `--query` and the query statement, the `=` will not be removed from the query.
I also have included the `--J` parameter in this check, because its value is always containing a `=`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao merged pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
Posted by GitBox <gi...@apache.org>.
jinmeiliao merged pull request #5187:
URL: https://github.com/apache/geode/pull/5187
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao commented on a change in pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
Posted by GitBox <gi...@apache.org>.
jinmeiliao commented on a change in pull request #5187:
URL: https://github.com/apache/geode/pull/5187#discussion_r436991385
##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
##########
@@ -122,21 +122,30 @@ else if (insideQuoteOf == c) {
List<String> splitWithWhiteSpaces = splitWithWhiteSpace(userInput);
List<String> furtherSplitWithEquals = new ArrayList<>();
+ boolean doNotRemoveEqualsInNextToken = false;
for (String token : splitWithWhiteSpaces) {
- // if this token has equal sign, split around the first occurrence of it
- int indexOfFirstEqual = token.indexOf('=');
- if (indexOfFirstEqual < 0) {
+ if (doNotRemoveEqualsInNextToken) {
furtherSplitWithEquals.add(token);
+ doNotRemoveEqualsInNextToken = false;
} else {
- String left = token.substring(0, indexOfFirstEqual);
- String right = token.substring(indexOfFirstEqual + 1);
- if (left.length() > 0) {
- furtherSplitWithEquals.add(left);
- }
- if (right.length() > 0) {
- furtherSplitWithEquals.add(right);
+ // if this token has equal sign, split around the first occurrence of it
+ int indexOfFirstEqual = token.indexOf('=');
+ if (indexOfFirstEqual < 0) {
+ furtherSplitWithEquals.add(token);
+ } else {
+ String left = token.substring(0, indexOfFirstEqual);
+ String right = token.substring(indexOfFirstEqual + 1);
+ if (left.length() > 0) {
+ furtherSplitWithEquals.add(left);
+ }
+ if (right.length() > 0) {
+ furtherSplitWithEquals.add(right);
+ }
}
}
+ if (token.equals("--query") || token.equals("--J")) {
Review comment:
anyway to not hardcode these two conditions? maybe based the decision on "do not remove = sign if it's in the quotes"?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] alb3rtobr commented on pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
Posted by GitBox <gi...@apache.org>.
alb3rtobr commented on pull request #5187:
URL: https://github.com/apache/geode/pull/5187#issuecomment-637811188
I have rebased the PR now that LGTM Java Analysis seems working again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao commented on a change in pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
Posted by GitBox <gi...@apache.org>.
jinmeiliao commented on a change in pull request #5187:
URL: https://github.com/apache/geode/pull/5187#discussion_r439097235
##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
##########
@@ -123,18 +123,25 @@ else if (insideQuoteOf == c) {
List<String> furtherSplitWithEquals = new ArrayList<>();
for (String token : splitWithWhiteSpaces) {
- // if this token has equal sign, split around the first occurrence of it
int indexOfFirstEqual = token.indexOf('=');
- if (indexOfFirstEqual < 0) {
+ int indexOfFirstDoubleQuote = token.indexOf('"');
+ int indexOfFirstSingleQuote = token.indexOf('\'');
+ if (indexOfFirstEqual < 0 || token.startsWith("-D")) {
furtherSplitWithEquals.add(token);
} else {
- String left = token.substring(0, indexOfFirstEqual);
- String right = token.substring(indexOfFirstEqual + 1);
- if (left.length() > 0) {
- furtherSplitWithEquals.add(left);
- }
- if (right.length() > 0) {
- furtherSplitWithEquals.add(right);
+ if (indexOfFirstDoubleQuote == 0 || indexOfFirstSingleQuote == 0) {
+ furtherSplitWithEquals.add(token);
+ } else if ((indexOfFirstDoubleQuote < 0 && indexOfFirstSingleQuote < 0)
+ || (indexOfFirstEqual < indexOfFirstDoubleQuote)
+ || (indexOfFirstEqual < indexOfFirstSingleQuote)) {
+ String left = token.substring(0, indexOfFirstEqual);
+ String right = token.substring(indexOfFirstEqual + 1);
+ if (left.length() > 0) {
+ furtherSplitWithEquals.add(left);
+ }
+ if (right.length() > 0) {
+ furtherSplitWithEquals.add(right);
+ }
}
}
}
Review comment:
this entire for loop can be simplified to:
```
for (String token : splitWithWhiteSpaces) {
// do not split with "=" if this part starts with quotes or is part of -D
if(token.startsWith("'") || token.startsWith("\"") || token.startsWith("-D")) {
furtherSplitWithEquals.add(token);
continue;
}
// if this token has equal sign, split around the first occurrence of it
int indexOfFirstEqual = token.indexOf('=');
if (indexOfFirstEqual < 0) {
furtherSplitWithEquals.add(token);
continue;
}
String left = token.substring(0, indexOfFirstEqual);
String right = token.substring(indexOfFirstEqual + 1);
if (left.length() > 0) {
furtherSplitWithEquals.add(left);
}
if (right.length() > 0) {
furtherSplitWithEquals.add(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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] alb3rtobr commented on a change in pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
Posted by GitBox <gi...@apache.org>.
alb3rtobr commented on a change in pull request #5187:
URL: https://github.com/apache/geode/pull/5187#discussion_r437255536
##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
##########
@@ -122,21 +122,30 @@ else if (insideQuoteOf == c) {
List<String> splitWithWhiteSpaces = splitWithWhiteSpace(userInput);
List<String> furtherSplitWithEquals = new ArrayList<>();
+ boolean doNotRemoveEqualsInNextToken = false;
for (String token : splitWithWhiteSpaces) {
- // if this token has equal sign, split around the first occurrence of it
- int indexOfFirstEqual = token.indexOf('=');
- if (indexOfFirstEqual < 0) {
+ if (doNotRemoveEqualsInNextToken) {
furtherSplitWithEquals.add(token);
+ doNotRemoveEqualsInNextToken = false;
} else {
- String left = token.substring(0, indexOfFirstEqual);
- String right = token.substring(indexOfFirstEqual + 1);
- if (left.length() > 0) {
- furtherSplitWithEquals.add(left);
- }
- if (right.length() > 0) {
- furtherSplitWithEquals.add(right);
+ // if this token has equal sign, split around the first occurrence of it
+ int indexOfFirstEqual = token.indexOf('=');
+ if (indexOfFirstEqual < 0) {
+ furtherSplitWithEquals.add(token);
+ } else {
+ String left = token.substring(0, indexOfFirstEqual);
+ String right = token.substring(indexOfFirstEqual + 1);
+ if (left.length() > 0) {
+ furtherSplitWithEquals.add(left);
+ }
+ if (right.length() > 0) {
+ furtherSplitWithEquals.add(right);
+ }
}
}
+ if (token.equals("--query") || token.equals("--J")) {
Review comment:
good point, I have changed the solution. Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] alb3rtobr commented on a change in pull request #5187: GEODE-8179: gfsh query cmd returns incorrect results if '=' sign is missing
Posted by GitBox <gi...@apache.org>.
alb3rtobr commented on a change in pull request #5187:
URL: https://github.com/apache/geode/pull/5187#discussion_r439298703
##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
##########
@@ -123,18 +123,25 @@ else if (insideQuoteOf == c) {
List<String> furtherSplitWithEquals = new ArrayList<>();
for (String token : splitWithWhiteSpaces) {
- // if this token has equal sign, split around the first occurrence of it
int indexOfFirstEqual = token.indexOf('=');
- if (indexOfFirstEqual < 0) {
+ int indexOfFirstDoubleQuote = token.indexOf('"');
+ int indexOfFirstSingleQuote = token.indexOf('\'');
+ if (indexOfFirstEqual < 0 || token.startsWith("-D")) {
furtherSplitWithEquals.add(token);
} else {
- String left = token.substring(0, indexOfFirstEqual);
- String right = token.substring(indexOfFirstEqual + 1);
- if (left.length() > 0) {
- furtherSplitWithEquals.add(left);
- }
- if (right.length() > 0) {
- furtherSplitWithEquals.add(right);
+ if (indexOfFirstDoubleQuote == 0 || indexOfFirstSingleQuote == 0) {
+ furtherSplitWithEquals.add(token);
+ } else if ((indexOfFirstDoubleQuote < 0 && indexOfFirstSingleQuote < 0)
+ || (indexOfFirstEqual < indexOfFirstDoubleQuote)
+ || (indexOfFirstEqual < indexOfFirstSingleQuote)) {
+ String left = token.substring(0, indexOfFirstEqual);
+ String right = token.substring(indexOfFirstEqual + 1);
+ if (left.length() > 0) {
+ furtherSplitWithEquals.add(left);
+ }
+ if (right.length() > 0) {
+ furtherSplitWithEquals.add(right);
+ }
}
}
}
Review comment:
thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org