From a0c1b52b64336ab6fa4c22432b12865b5e88de71 Mon Sep 17 00:00:00 2001 From: Chris Davenport Date: Mon, 10 Aug 2020 09:26:20 -0700 Subject: [PATCH 1/2] Resp Bound Fixes --- .../main/scala/io/chrisdavenport/rediculous/Resp.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala b/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala index 25fe7cc..db5f189 100644 --- a/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala +++ b/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala @@ -49,7 +49,7 @@ object Resp { } def parse(arr: SArray[Byte]): RespParserResult[Resp] = { - if (arr.size >= 0) { + if (arr.size > 0) { val switchVal = arr(0) switchVal match { case Plus => SimpleString.parse(arr) @@ -65,6 +65,7 @@ object Resp { } // First Byte is + + // +foo/r/n case class SimpleString(value: String) extends Resp object SimpleString { def encode(s: SimpleString): SArray[Byte] = { @@ -139,6 +140,7 @@ object Resp { } } // First Byte is $ + // $3/r/n/foo/r/n case class BulkString(value: Option[String]) extends Resp object BulkString { def encode(b: BulkString): SArray[Byte] = { @@ -162,10 +164,10 @@ object Resp { idx += 2 } if (length == -1) ParseComplete(BulkString(None), arr.drop(idx)) - else { + else if (idx + length + 2 < arr.size) { val out = new String(arr, idx, length) ParseComplete(BulkString(Some(out)), arr.drop(idx + length + 2)) - } + } else ParseIncomplete(arr) } catch { case scala.util.control.NonFatal(e) => ParseError(s"Error in BulkString Processing: ${e.getMessage}", Some(e)) From 8cdcbb88dceae15f7cecec84731ca3eca6f69629 Mon Sep 17 00:00:00 2001 From: Chris Davenport Date: Mon, 10 Aug 2020 09:55:29 -0700 Subject: [PATCH 2/2] Charset UTF8 and Size Fixes --- .../io/chrisdavenport/rediculous/Resp.scala | 41 +++++++++++-------- .../chrisdavenport/rediculous/RespSpec.scala | 2 +- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala b/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala index db5f189..6c42f79 100644 --- a/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala +++ b/core/src/main/scala/io/chrisdavenport/rediculous/Resp.scala @@ -2,6 +2,8 @@ package io.chrisdavenport.rediculous import scala.collection.mutable import cats.implicits._ +import scala.util.control.NonFatal +import java.nio.charset.StandardCharsets sealed trait Resp @@ -69,7 +71,7 @@ object Resp { case class SimpleString(value: String) extends Resp object SimpleString { def encode(s: SimpleString): SArray[Byte] = { - SArray(Plus) ++ s.value.getBytes() ++ CRLF + SArray(Plus) ++ s.value.getBytes(StandardCharsets.UTF_8) ++ CRLF } def parse(arr: SArray[Byte]): RespParserResult[SimpleString] = { var idx = 1 @@ -79,13 +81,13 @@ object Resp { idx += 1 } if (idx < arr.size && (idx +1 < arr.size) && arr(idx +1) == LF){ - val out = new String(arr, 1, idx - 1) + val out = new String(arr, 1, idx - 1, StandardCharsets.UTF_8) ParseComplete(SimpleString(out), arr.drop(idx + 2)) } else { ParseIncomplete(arr) } } catch { - case scala.util.control.NonFatal(e) => + case NonFatal(e) => ParseError(s"Error in RespSimpleString Processing: ${e.getMessage}", Some(e)) } } @@ -94,7 +96,7 @@ object Resp { case class Error(value: String) extends Throwable(s"Resp Error- $value") with Resp object Error { def encode(error: Error): SArray[Byte] = - SArray(Minus) ++ error.value.getBytes() ++ CRLF + SArray(Minus) ++ error.value.getBytes(StandardCharsets.UTF_8) ++ CRLF def parse(arr: SArray[Byte]): RespParserResult[Error] = { var idx = 1 try { @@ -103,13 +105,13 @@ object Resp { idx += 1 } if (idx < arr.size && (idx +1 < arr.size) && arr(idx +1) == LF){ - val out = new String(arr, 1, idx - 1) + val out = new String(arr, 1, idx - 1, StandardCharsets.UTF_8) ParseComplete(Error(out), arr.drop(idx + 2)) } else { ParseIncomplete(arr) } } catch { - case scala.util.control.NonFatal(e) => + case NonFatal(e) => ParseError(s"Error in Resp Error Processing: ${e.getMessage}", Some(e)) } } @@ -118,7 +120,7 @@ object Resp { case class Integer(long: Long) extends Resp object Integer { def encode(i: Integer): SArray[Byte] = { - SArray(Colon) ++ i.long.toString().getBytes() ++ CRLF + SArray(Colon) ++ i.long.toString().getBytes(StandardCharsets.UTF_8) ++ CRLF } def parse(arr: SArray[Byte]): RespParserResult[Integer] = { var idx = 1 @@ -128,13 +130,13 @@ object Resp { idx += 1 } if (idx < arr.size && (idx +1 < arr.size) && arr(idx +1) == LF){ - val out = new String(arr, 1, idx - 1).toLong + val out = new String(arr, 1, idx - 1, StandardCharsets.UTF_8).toLong ParseComplete(Integer(out), arr.drop(idx + 2)) } else { ParseIncomplete(arr) } } catch { - case scala.util.control.NonFatal(e) => + case NonFatal(e) => ParseError(s"Error in RespInteger Processing: ${e.getMessage}", Some(e)) } } @@ -146,8 +148,11 @@ object Resp { def encode(b: BulkString): SArray[Byte] = { b.value match { case None => SArray(Dollar) ++ MinusOne ++ CRLF - case Some(s) => SArray(Dollar) ++ s.length().toString().getBytes() ++ CRLF ++ - s.getBytes() ++ CRLF + case Some(s) => { + val bytes = s.getBytes(StandardCharsets.UTF_8) + SArray(Dollar) ++ bytes.size.toString.getBytes(StandardCharsets.UTF_8) ++ CRLF ++ + bytes ++ CRLF + } } } def parse(arr: SArray[Byte]): RespParserResult[BulkString] = { @@ -159,17 +164,17 @@ object Resp { idx += 1 } if (idx < arr.size && (idx +1 < arr.size) && arr(idx +1) == LF){ - val out = new String(arr, 1, idx - 1).toInt + val out = new String(arr, 1, idx - 1, StandardCharsets.UTF_8).toInt length = out idx += 2 } if (length == -1) ParseComplete(BulkString(None), arr.drop(idx)) - else if (idx + length + 2 < arr.size) { - val out = new String(arr, idx, length) + else if (idx + length + 2 <= arr.size) { + val out = new String(arr, idx, length, StandardCharsets.UTF_8) ParseComplete(BulkString(Some(out)), arr.drop(idx + length + 2)) } else ParseIncomplete(arr) } catch { - case scala.util.control.NonFatal(e) => + case NonFatal(e) => ParseError(s"Error in BulkString Processing: ${e.getMessage}", Some(e)) } } @@ -186,7 +191,7 @@ object Resp { buffer ++= MinusOne buffer ++= CRLF case Some(value) => - buffer ++= value.size.toString().getBytes() + buffer ++= value.size.toString().getBytes(StandardCharsets.UTF_8) buffer ++= CRLF value.foreach(resp => buffer ++= Resp.encode(resp) @@ -203,7 +208,7 @@ object Resp { idx += 1 } if (idx < arr.size && (idx +1 <= arr.size) && arr(idx +1) == LF){ - val out = new String(arr, 1, idx - 1).toInt + val out = new String(arr, 1, idx - 1, StandardCharsets.UTF_8).toInt length = out idx += 2 } @@ -224,7 +229,7 @@ object Resp { repeatParse(next, length, List.empty) } } catch { - case scala.util.control.NonFatal(e) => + case NonFatal(e) => ParseError(s"Error in RespArray Processing: ${e.getMessage}", Some(e)) } } diff --git a/core/src/test/scala/io/chrisdavenport/rediculous/RespSpec.scala b/core/src/test/scala/io/chrisdavenport/rediculous/RespSpec.scala index ee23c5e..1c16c26 100644 --- a/core/src/test/scala/io/chrisdavenport/rediculous/RespSpec.scala +++ b/core/src/test/scala/io/chrisdavenport/rediculous/RespSpec.scala @@ -120,7 +120,7 @@ class RespSpec extends Specification with ScalaCheck { case p@ParseIncomplete(_) => ko(s"Got Incomplete Result $p") case e@ParseError(_,_) => ko(s"Got ParseError $e") } - }.setGen(org.scalacheck.Gen.asciiStr) // Get this to all strings? + } "parse an empty array" in { val init = "*0\r\n"