[GitHub] [geode] sabbey37 commented on a change in pull request #5627: GEODE-8585: Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters

[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