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/10/14 19:45:04 UTC
[GitHub] [geode] sabbey37 opened a new pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
sabbey37 opened a new pull request #5627:
URL: https://github.com/apache/geode/pull/5627
Add integration tests and functionality to match native Redis.
----------------------------------------------------------------
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] lgtm-com[bot] commented on pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #5627:
URL: https://github.com/apache/geode/pull/5627#issuecomment-710095163
This pull request **introduces 2 alerts** when merging 681e73630254e9db9800d86663411e06b28749a0 into 4c4820207e263f1a9d11f34df5cd6cd2f9daf973 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-65a447ce28c79e4440b13fc5873863b002516dd3)
**new alerts:**
* 2 for Dereferenced variable may be null
----------------------------------------------------------------
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] sabbey37 merged pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
Posted by GitBox <gi...@apache.org>.
sabbey37 merged pull request #5627:
URL: https://github.com/apache/geode/pull/5627
----------------------------------------------------------------
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] sabbey37 commented on a change in pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
Posted by GitBox <gi...@apache.org>.
sabbey37 commented on a change in pull request #5627:
URL: https://github.com/apache/geode/pull/5627#discussion_r505871824
##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/ScanExecutor.java
##########
@@ -36,91 +39,87 @@
public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
List<byte[]> commandElems = command.getProcessedCommand();
- if (commandElems.size() < 2) {
- return RedisResponse.error(ArityDef.SCAN);
- }
-
String cursorString = command.getStringKey();
- int cursor = 0;
- Pattern matchPattern = null;
- String globMatchString = null;
+ BigInteger cursor;
+ Pattern matchPattern;
+ String globPattern = null;
int count = DEFAULT_COUNT;
+
try {
- cursor = Integer.parseInt(cursorString);
+ cursor = new BigInteger(cursorString).abs();
} catch (NumberFormatException e) {
return RedisResponse.error(ERROR_CURSOR);
}
- if (cursor < 0) {
+
+ if (cursor.compareTo(UNSIGNED_LONG_CAPACITY) > 0) {
return RedisResponse.error(ERROR_CURSOR);
}
- if (commandElems.size() > 3) {
- try {
- byte[] bytes = commandElems.get(2);
- String tmp = Coder.bytesToString(bytes);
- if (tmp.equalsIgnoreCase("MATCH")) {
- bytes = commandElems.get(3);
- globMatchString = Coder.bytesToString(bytes);
- } else if (tmp.equalsIgnoreCase("COUNT")) {
- bytes = commandElems.get(3);
- count = Coder.bytesToInt(bytes);
- }
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_COUNT);
- }
+ if (!cursor.equals(context.getScanCursor())) {
+ cursor = new BigInteger("0");
}
- if (commandElems.size() > 5) {
- try {
- byte[] bytes = commandElems.get(4);
- String tmp = Coder.bytesToString(bytes);
- if (tmp.equalsIgnoreCase("COUNT")) {
- bytes = commandElems.get(5);
- count = Coder.bytesToInt(bytes);
+ for (int i = 2; i < commandElems.size(); i = i + 2) {
+ byte[] commandElemBytes = commandElems.get(i);
+ String keyword = Coder.bytesToString(commandElemBytes);
+ if (keyword.equalsIgnoreCase("MATCH")) {
+ commandElemBytes = commandElems.get(i + 1);
+ globPattern = Coder.bytesToString(commandElemBytes);
+
+ } else if (keyword.equalsIgnoreCase("COUNT")) {
+ commandElemBytes = commandElems.get(i + 1);
+ try {
+ count = Coder.bytesToInt(commandElemBytes);
+ } catch (NumberFormatException e) {
+ return RedisResponse.error(ERROR_NOT_INTEGER);
+ }
+
+ if (count < 1) {
+ return RedisResponse.error(ERROR_SYNTAX);
}
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_COUNT);
- }
- }
- if (count < 0) {
- return RedisResponse.error(ERROR_COUNT);
+ } else {
+ return RedisResponse.error(ERROR_SYNTAX);
+ }
}
try {
- matchPattern = convertGlobToRegex(globMatchString);
+ matchPattern = convertGlobToRegex(globPattern);
} catch (PatternSyntaxException e) {
- return RedisResponse.error(ERROR_ILLEGAL_GLOB);
+ LogService.getLogger().warn(
+ "Could not compile the pattern: '{}' due to the following exception: '{}'. SCAN will return an empty list.",
+ globPattern, e.getMessage());
+ return RedisResponse.emptyScan();
}
- List<String> returnList = getIteration(getDataRegion(context).keySet(), matchPattern, count,
+ List<Object> returnList = scan(getDataRegion(context).keySet(), matchPattern, count,
cursor);
+ context.setScanCursor(new BigInteger((String) returnList.get(0)));
return RedisResponse.scan(returnList);
}
- private List<String> getIteration(Collection<ByteArrayWrapper> list, Pattern matchPattern,
- int count, int cursor) {
- List<String> returnList = new ArrayList<>();
+ private List<Object> scan(Collection<ByteArrayWrapper> list, Pattern matchPattern,
Review comment:
Good idea! I just implemented it. Let me know what you think.
----------------------------------------------------------------
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] sabbey37 commented on a change in pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
Posted by GitBox <gi...@apache.org>.
sabbey37 commented on a change in pull request #5627:
URL: https://github.com/apache/geode/pull/5627#discussion_r505871824
##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/ScanExecutor.java
##########
@@ -36,91 +39,87 @@
public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
List<byte[]> commandElems = command.getProcessedCommand();
- if (commandElems.size() < 2) {
- return RedisResponse.error(ArityDef.SCAN);
- }
-
String cursorString = command.getStringKey();
- int cursor = 0;
- Pattern matchPattern = null;
- String globMatchString = null;
+ BigInteger cursor;
+ Pattern matchPattern;
+ String globPattern = null;
int count = DEFAULT_COUNT;
+
try {
- cursor = Integer.parseInt(cursorString);
+ cursor = new BigInteger(cursorString).abs();
} catch (NumberFormatException e) {
return RedisResponse.error(ERROR_CURSOR);
}
- if (cursor < 0) {
+
+ if (cursor.compareTo(UNSIGNED_LONG_CAPACITY) > 0) {
return RedisResponse.error(ERROR_CURSOR);
}
- if (commandElems.size() > 3) {
- try {
- byte[] bytes = commandElems.get(2);
- String tmp = Coder.bytesToString(bytes);
- if (tmp.equalsIgnoreCase("MATCH")) {
- bytes = commandElems.get(3);
- globMatchString = Coder.bytesToString(bytes);
- } else if (tmp.equalsIgnoreCase("COUNT")) {
- bytes = commandElems.get(3);
- count = Coder.bytesToInt(bytes);
- }
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_COUNT);
- }
+ if (!cursor.equals(context.getScanCursor())) {
+ cursor = new BigInteger("0");
}
- if (commandElems.size() > 5) {
- try {
- byte[] bytes = commandElems.get(4);
- String tmp = Coder.bytesToString(bytes);
- if (tmp.equalsIgnoreCase("COUNT")) {
- bytes = commandElems.get(5);
- count = Coder.bytesToInt(bytes);
+ for (int i = 2; i < commandElems.size(); i = i + 2) {
+ byte[] commandElemBytes = commandElems.get(i);
+ String keyword = Coder.bytesToString(commandElemBytes);
+ if (keyword.equalsIgnoreCase("MATCH")) {
+ commandElemBytes = commandElems.get(i + 1);
+ globPattern = Coder.bytesToString(commandElemBytes);
+
+ } else if (keyword.equalsIgnoreCase("COUNT")) {
+ commandElemBytes = commandElems.get(i + 1);
+ try {
+ count = Coder.bytesToInt(commandElemBytes);
+ } catch (NumberFormatException e) {
+ return RedisResponse.error(ERROR_NOT_INTEGER);
+ }
+
+ if (count < 1) {
+ return RedisResponse.error(ERROR_SYNTAX);
}
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_COUNT);
- }
- }
- if (count < 0) {
- return RedisResponse.error(ERROR_COUNT);
+ } else {
+ return RedisResponse.error(ERROR_SYNTAX);
+ }
}
try {
- matchPattern = convertGlobToRegex(globMatchString);
+ matchPattern = convertGlobToRegex(globPattern);
} catch (PatternSyntaxException e) {
- return RedisResponse.error(ERROR_ILLEGAL_GLOB);
+ LogService.getLogger().warn(
+ "Could not compile the pattern: '{}' due to the following exception: '{}'. SCAN will return an empty list.",
+ globPattern, e.getMessage());
+ return RedisResponse.emptyScan();
}
- List<String> returnList = getIteration(getDataRegion(context).keySet(), matchPattern, count,
+ List<Object> returnList = scan(getDataRegion(context).keySet(), matchPattern, count,
cursor);
+ context.setScanCursor(new BigInteger((String) returnList.get(0)));
return RedisResponse.scan(returnList);
}
- private List<String> getIteration(Collection<ByteArrayWrapper> list, Pattern matchPattern,
- int count, int cursor) {
- List<String> returnList = new ArrayList<>();
+ private List<Object> scan(Collection<ByteArrayWrapper> list, Pattern matchPattern,
Review comment:
Good idea! I just changed implemented it. Let me know what you think.
----------------------------------------------------------------
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] lgtm-com[bot] removed a comment on pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] removed a comment on pull request #5627:
URL: https://github.com/apache/geode/pull/5627#issuecomment-710095163
This pull request **introduces 2 alerts** when merging 681e73630254e9db9800d86663411e06b28749a0 into 4c4820207e263f1a9d11f34df5cd6cd2f9daf973 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-65a447ce28c79e4440b13fc5873863b002516dd3)
**new alerts:**
* 2 for Dereferenced variable may be null
----------------------------------------------------------------
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] jdeppe-pivotal commented on a change in pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
Posted by GitBox <gi...@apache.org>.
jdeppe-pivotal commented on a change in pull request #5627:
URL: https://github.com/apache/geode/pull/5627#discussion_r505827527
##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/ScanExecutor.java
##########
@@ -36,91 +39,87 @@
public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
List<byte[]> commandElems = command.getProcessedCommand();
- if (commandElems.size() < 2) {
- return RedisResponse.error(ArityDef.SCAN);
- }
-
String cursorString = command.getStringKey();
- int cursor = 0;
- Pattern matchPattern = null;
- String globMatchString = null;
+ BigInteger cursor;
+ Pattern matchPattern;
+ String globPattern = null;
int count = DEFAULT_COUNT;
+
try {
- cursor = Integer.parseInt(cursorString);
+ cursor = new BigInteger(cursorString).abs();
} catch (NumberFormatException e) {
return RedisResponse.error(ERROR_CURSOR);
}
- if (cursor < 0) {
+
+ if (cursor.compareTo(UNSIGNED_LONG_CAPACITY) > 0) {
return RedisResponse.error(ERROR_CURSOR);
}
- if (commandElems.size() > 3) {
- try {
- byte[] bytes = commandElems.get(2);
- String tmp = Coder.bytesToString(bytes);
- if (tmp.equalsIgnoreCase("MATCH")) {
- bytes = commandElems.get(3);
- globMatchString = Coder.bytesToString(bytes);
- } else if (tmp.equalsIgnoreCase("COUNT")) {
- bytes = commandElems.get(3);
- count = Coder.bytesToInt(bytes);
- }
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_COUNT);
- }
+ if (!cursor.equals(context.getScanCursor())) {
+ cursor = new BigInteger("0");
}
- if (commandElems.size() > 5) {
- try {
- byte[] bytes = commandElems.get(4);
- String tmp = Coder.bytesToString(bytes);
- if (tmp.equalsIgnoreCase("COUNT")) {
- bytes = commandElems.get(5);
- count = Coder.bytesToInt(bytes);
+ for (int i = 2; i < commandElems.size(); i = i + 2) {
+ byte[] commandElemBytes = commandElems.get(i);
+ String keyword = Coder.bytesToString(commandElemBytes);
+ if (keyword.equalsIgnoreCase("MATCH")) {
+ commandElemBytes = commandElems.get(i + 1);
+ globPattern = Coder.bytesToString(commandElemBytes);
+
+ } else if (keyword.equalsIgnoreCase("COUNT")) {
+ commandElemBytes = commandElems.get(i + 1);
+ try {
+ count = Coder.bytesToInt(commandElemBytes);
+ } catch (NumberFormatException e) {
+ return RedisResponse.error(ERROR_NOT_INTEGER);
+ }
+
+ if (count < 1) {
+ return RedisResponse.error(ERROR_SYNTAX);
}
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_COUNT);
- }
- }
- if (count < 0) {
- return RedisResponse.error(ERROR_COUNT);
+ } else {
+ return RedisResponse.error(ERROR_SYNTAX);
+ }
}
try {
- matchPattern = convertGlobToRegex(globMatchString);
+ matchPattern = convertGlobToRegex(globPattern);
} catch (PatternSyntaxException e) {
- return RedisResponse.error(ERROR_ILLEGAL_GLOB);
+ LogService.getLogger().warn(
+ "Could not compile the pattern: '{}' due to the following exception: '{}'. SCAN will return an empty list.",
+ globPattern, e.getMessage());
+ return RedisResponse.emptyScan();
}
- List<String> returnList = getIteration(getDataRegion(context).keySet(), matchPattern, count,
+ List<Object> returnList = scan(getDataRegion(context).keySet(), matchPattern, count,
cursor);
+ context.setScanCursor(new BigInteger((String) returnList.get(0)));
return RedisResponse.scan(returnList);
}
- private List<String> getIteration(Collection<ByteArrayWrapper> list, Pattern matchPattern,
- int count, int cursor) {
- List<String> returnList = new ArrayList<>();
+ private List<Object> scan(Collection<ByteArrayWrapper> list, Pattern matchPattern,
Review comment:
I think it would be better to have this return a `Pair<BigInteger, List<Object>>` - that way it's not hiding the fact that the zero-th element is special.
----------------------------------------------------------------
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