[ 
https://issues.apache.org/jira/browse/GEODE-8585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17215020#comment-17215020
 ] 

ASF GitHub Bot commented on GEODE-8585:
---------------------------------------

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:
us...@infra.apache.org


> Redis SCAN, HSCAN, and SSCAN do not detect illegal parameters
> -------------------------------------------------------------
>
>                 Key: GEODE-8585
>                 URL: https://issues.apache.org/jira/browse/GEODE-8585
>             Project: Geode
>          Issue Type: Bug
>          Components: redis
>            Reporter: Sarah Abbey
>            Assignee: Sarah Abbey
>            Priority: Minor
>              Labels: pull-request-available
>
> The parameter parsing for SCAN will not detect some illegal parameters. We 
> found that if we left off the MATCH keyword it just ignored the match string 
> we gave it. Native redis throws a syntax error. We also saw that COUNT could 
> follow MATCH but MATCH could not follow COUNT. I think it also allows COUNT 
> to be specified twice. HSCAN and SSCAN probably have similar issues.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to