Skip to content

Commit

Permalink
ISPN-773 - Malformed memcached cas command should result in CLIENT_ERROR
Browse files Browse the repository at this point in the history 
Fixed by making sure corresponding exceptions are mapped to the right
type of error.
  • Loading branch information
@galderz
galderz committed Nov 16, 2010
1 parent ea53030 commit de03477
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 39 deletions.
79 changes: 42 additions & 37 deletions server/memcached/src/main/scala/org/infinispan/server/memcached/MemcachedDecoder.scala
View file
Expand Up @@ -79,46 +79,50 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
if (!line.isEmpty) {
if (isTraceEnabled) trace("Operation parameters: {0}", line)
val args = line.trim.split(" +")
var index = 0
h.op match {
case RemoveRequest => {
val delayedDeleteTime = parseDelayedDeleteTime(index, args)
val noReply = if (delayedDeleteTime == -1) parseNoReply(index, args) else false
Some(new MemcachedParameters(null, -1, -1, -1, noReply, 0, "", 0))
}
case IncrementRequest | DecrementRequest => {
val delta = args(index)
index += 1
Some(new MemcachedParameters(null, -1, -1, -1, parseNoReply(index, args), 0, delta, 0))
}
case FlushAllRequest => {
val flushDelay = args(index).toInt
index += 1
Some(new MemcachedParameters(null, -1, -1, -1, parseNoReply(index, args), 0, "", flushDelay))
}
case _ => {
val flags = getFlags(args(index))
index += 1
val lifespan = {
val streamLifespan = getLifespan(args(index))
if (streamLifespan <= 0) -1 else streamLifespan
try {
var index = 0
h.op match {
case RemoveRequest => {
val delayedDeleteTime = parseDelayedDeleteTime(index, args)
val noReply = if (delayedDeleteTime == -1) parseNoReply(index, args) else false
Some(new MemcachedParameters(null, -1, -1, -1, noReply, 0, "", 0))
}
index += 1
val length = getLength(args(index))
val streamVersion = h.op match {
case ReplaceIfUnmodifiedRequest => {
index += 1
getVersion(args(index))
case IncrementRequest | DecrementRequest => {
val delta = args(index)
index += 1
Some(new MemcachedParameters(null, -1, -1, -1, parseNoReply(index, args), 0, delta, 0))
}
case FlushAllRequest => {
val flushDelay = args(index).toInt
index += 1
Some(new MemcachedParameters(null, -1, -1, -1, parseNoReply(index, args), 0, "", flushDelay))
}
case _ => {
val flags = getFlags(args(index))
index += 1
val lifespan = {
val streamLifespan = getLifespan(args(index))
if (streamLifespan <= 0) -1 else streamLifespan
}
index += 1
val length = getLength(args(index))
val streamVersion = h.op match {
case ReplaceIfUnmodifiedRequest => {
index += 1
getVersion(args(index))
}
case _ => -1
}
case _ => -1
index += 1
val noReply = parseNoReply(index, args)
val data = new Array[Byte](length)
b.readBytes(data, 0, data.length)
readLine(b) // read the rest of line to clear CRLF after value Byte[]
Some(new MemcachedParameters(data, lifespan, -1, streamVersion, noReply, flags, "", 0))
}
index += 1
val noReply = parseNoReply(index, args)
val data = new Array[Byte](length)
b.readBytes(data, 0, data.length)
readLine(b) // read the rest of line to clear CRLF after value Byte[]
Some(new MemcachedParameters(data, lifespan, -1, streamVersion, noReply, flags, "", 0))
}
} catch {
case _: ArrayIndexOutOfBoundsException => throw new IOException("Missing content in command line " + line)
}
} else {
None // For example when delete <key> is sent without any further parameters, or flush_all without delay
Expand Down Expand Up @@ -311,7 +315,8 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
case c: ClosedChannelException => null // no-op, only log
case _ => {
cause match {
case i: IOException => sb.append("CLIENT_ERROR ")
case i: IOException => sb.append("CLIENT_ERROR bad command line format: ")
case n: NumberFormatException => sb.append("CLIENT_ERROR bad command line format: ")
case _ => sb.append("SERVER_ERROR ")
}
sb.append(t).append(CRLF)
Expand Down
30 changes: 28 additions & 2 deletions ...er/memcached/src/test/scala/org/infinispan/server/memcached/MemcachedFunctionalTest.scala
View file
Expand Up @@ -346,7 +346,30 @@ class MemcachedFunctionalTest extends MemcachedSingleNodeTest {

val keyAboveLimit = generateRandomString(251)
val resp = incr(keyAboveLimit, 1, 1024)
assertTrue(resp.contains("CLIENT_ERROR"))
assertClientError(resp)
}

def testInvalidCas(m: Method) {
var resp = send("cas bad blah 0 0 0\r\n\r\n", 1024)
assertClientError(resp)

resp = send("cas bad 0 blah 0 0\r\n\r\n", 1024)
assertClientError(resp)

resp = send("cas bad 0 0 blah 0\r\n\r\n", 1024)
assertClientError(resp)

resp = send("cas bad 0 0 0 blah\r\n\r\n", 1024)
assertClientError(resp)
}

def testInvalidCasValue(m: Method) {
val resp = send("cas foo 0 0 6 \r\nbarva2\r\n", 1024)
assertClientError(resp)
}

private def assertClientError(resp: String) {
assertTrue(resp.contains("CLIENT_ERROR"), "Instead response is: " + resp)
}

private def addAndGet(m: Method) {
Expand All @@ -360,6 +383,10 @@ class MemcachedFunctionalTest extends MemcachedSingleNodeTest {
private def incr(k: String, by: Int, expectedLength: Int): String = {
// Spymemcached expects Long so does not support 64-bit unsigned integer. Instead, do things manually.
val req = "incr " + k + " " + by + "\r\n"
send(req, expectedLength)
}

private def send(req: String, expectedLength: Int): String = {
val socket = new Socket(server.getHost, server.getPort)
try {
socket.getOutputStream.write(req.getBytes)
Expand All @@ -371,5 +398,4 @@ class MemcachedFunctionalTest extends MemcachedSingleNodeTest {
socket.close
}
}

}

0 comments on commit de03477

Please sign in to comment.