dmagda commented on a change in pull request #7808:
URL: https://github.com/apache/ignite/pull/7808#discussion_r428315712
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/rest/protocols/tcp/GridTcpRestParser.java
##########
@@ -246,6 +246,45 @@ else if (msg instanceof GridRouterRequest) {
}
}
+ /**
+ * Parses redis protocol message.
+ *
+ * @param ses Session.
+ * @param buf Buffer containing not parsed bytes.
+ * @param state Current parser state.
+ * @return Parsed packet.s
+ * @throws IOException If packet cannot be parsed.
+ * @throws IgniteCheckedException If deserialization error occurred.
+ */
+ private GridClientMessage parseRedisPacket(GridNioSession ses, ByteBuffer
buf, ParserState state)
Review comment:
Wouldn't it be better to move this method to
GridRedisProtocolParser.java class and renaming it to "readPacket"? This would
also allow us to use "private" visibility level for the "validate.." set of the
new methods
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/rest/protocols/tcp/redis/GridRedisProtocolParser.java
##########
@@ -112,6 +115,60 @@ public static String readBulkStr(ByteBuffer buf) throws
IgniteCheckedException {
return new String(bulkStr);
}
+ /*
+ * A validation method to check packet completeness.
+ * return true if and only if
+ * 1. First byte is ARRAY (43)
+ * 2. Last two bytes are CR(13) LF(10)
+ *
+ * Otherwise, return false representing this is an incomplete packet with
three possible scenarios:
+ * 1. A beginning packet with leading ARRAY byte
+ * 2. A continual packet with ending CRLF bytes.
+ * 3. A continual packet with neither conditions above.
+ */
+ public static boolean validatePacket(ByteBuffer buf) {
Review comment:
I would suggest naming the method "isCompletePacket"
##########
File path:
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/protocols/tcp/redis/RedisProtocolConnectSelfTest.java
##########
@@ -74,4 +75,21 @@ public void testSelect() throws Exception {
Assert.assertEquals("v0", jedis.get("k0"));
}
}
+
+ //IGNITE-7153
Review comment:
Please remove the mentioning of "IGNITE-7153". Instead, use the text
below as a method description
```
/**
* @throws Exception If failed.
*/
@Test
```
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/rest/protocols/tcp/redis/GridRedisProtocolParser.java
##########
@@ -112,6 +115,60 @@ public static String readBulkStr(ByteBuffer buf) throws
IgniteCheckedException {
return new String(bulkStr);
}
+ /*
+ * A validation method to check packet completeness.
+ * return true if and only if
+ * 1. First byte is ARRAY (43)
+ * 2. Last two bytes are CR(13) LF(10)
+ *
+ * Otherwise, return false representing this is an incomplete packet with
three possible scenarios:
+ * 1. A beginning packet with leading ARRAY byte
+ * 2. A continual packet with ending CRLF bytes.
+ * 3. A continual packet with neither conditions above.
+ */
+ public static boolean validatePacket(ByteBuffer buf) {
+ return validatePacketHeader(buf) && validatePacketFooter(buf);
+ }
+
+ public static boolean validatePacketHeader(ByteBuffer buf) {
+ boolean result = true;
+
+ //mark at initial position
+ buf.mark();
+
+ if(buf.get() != ARRAY) {
+ result = false;
+ }
+
+ //reset to initial position
+ buf.reset();
+
+ return result;
+ }
+
+ public static boolean validatePacketFooter(ByteBuffer buf) {
+ boolean result = true;
+
+ //mark at initial position
+ buf.mark();
+
Review comment:
Extra space, please remove. Check guidelines for reference:
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-Whitespacesandemptylines
----------------------------------------------------------------
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:
[email protected]
