Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ispn794 master and 4.2.x #68

Closed
wants to merge 1 commit into from

2 participants

@galderz
Owner

https://jira.jboss.org/browse/ISPN-794

Branch 4.2.x as well: ispn794_42x

@galderz galderz ISPN-794 - Memcached server should treat unsigned numeric values as such
Check for unsigned values, such as flags and deltas for incr/decr added.
6e33e61
@maniksurtani
Collaborator

Done

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 26, 2010
  1. @galderz

    ISPN-794 - Memcached server should treat unsigned numeric values as such

    galderz authored
    Check for unsigned values, such as flags and deltas for incr/decr added.
This page is out of date. Refresh to see the latest.
View
54 server/memcached/src/main/scala/org/infinispan/server/memcached/MemcachedDecoder.scala
@@ -48,7 +48,7 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
if (op.get == StatsRequest && !endOfOp) {
val line = readLine(buffer).trim
if (!line.isEmpty)
- throw new IOException("Stats command does not accept arguments: " + line)
+ throw new StreamCorruptedException("Stats command does not accept arguments: " + line)
}
Some(new RequestHeader(op.get))
}
@@ -108,6 +108,7 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
}
case _ => {
val flags = getFlags(args(index))
+ if (flags < 0) throw new StreamCorruptedException("Flags cannot be negative: " + flags)
index += 1
val lifespan = {
val streamLifespan = getLifespan(args(index))
@@ -143,9 +144,13 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
new MemcachedValue(p.data, nextVersion, p.flags)
}
- private def getFlags(flags: String): Int = {
+ private def getFlags(flags: String): Long = {
if (flags == null) throw new EOFException("No flags passed")
- flags.toInt
+ try {
+ numericLimitCheck(flags, 4294967295L, "Flags")
+ } catch {
+ case n: NumberFormatException => numericLimitCheck(flags, 4294967295L, "Flags", n)
+ }
}
private def getLifespan(lifespan: String): Int = {
@@ -212,14 +217,15 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
val prev = cache.get(k)
if (prev != null) {
val prevCounter = BigInt(new String(prev.data))
+ val delta = validateDelta(params.get.delta)
val newCounter =
h.op match {
case IncrementRequest => {
- val candidateCounter = prevCounter + BigInt(params.get.delta)
- if (candidateCounter > BigInt("18446744073709551615")) 0 else candidateCounter
+ val candidateCounter = prevCounter + delta
+ if (candidateCounter > MAX_UNSIGNED_LONG) 0 else candidateCounter
}
case DecrementRequest => {
- val candidateCounter = prevCounter - BigInt(params.get.delta)
+ val candidateCounter = prevCounter - delta
if (candidateCounter < 0) 0 else candidateCounter
}
}
@@ -251,6 +257,15 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
}
}
+ private def validateDelta(delta: String): BigInt = {
+ val bigIntDelta = BigInt(delta)
+ if (bigIntDelta > MAX_UNSIGNED_LONG)
+ throw new StreamCorruptedException("Increment or decrement delta sent (" + delta + ") exceeds unsigned limit (" + MAX_UNSIGNED_LONG + ")")
+ else if (bigIntDelta < MIN_UNSIGNED)
+ throw new StreamCorruptedException("Increment or decrement delta cannot be negative: " + delta)
+ bigIntDelta
+ }
+
override def createSuccessResponse(h: RequestHeader, params: Option[MemcachedParameters], prev: MemcachedValue): AnyRef = {
if (isStatsEnabled) {
h.op match {
@@ -389,7 +404,7 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
buffer
}
- private def createValue(data: Array[Byte], nextVersion: Long, flags: Int): MemcachedValue = {
+ private def createValue(data: Array[Byte], nextVersion: Long, flags: Long): MemcachedValue = {
new MemcachedValue(data, nextVersion, flags)
}
@@ -411,20 +426,29 @@ class MemcachedDecoder(cache: Cache[String, MemcachedValue], scheduler: Schedule
try {
number.toInt
} catch {
- case n: NumberFormatException => {
- if (number.toLong > Int.MaxValue)
- throw new NumberFormatException(message + " sent (" + number
- + ") exceeds the limit (" + Int.MaxValue + ")")
- else
- throw n
- }
+ case n: NumberFormatException => numericLimitCheck(number, Int.MaxValue, message, n)
}
}
+
+ private def numericLimitCheck(number: String, maxValue: Long, message: String, n: NumberFormatException): Int = {
+ if (number.toLong > maxValue)
+ throw new NumberFormatException(message + " sent (" + number
+ + ") exceeds the limit (" + maxValue + ")")
+ else throw n
+ }
+
+ private def numericLimitCheck(number: String, maxValue: Long, message: String): Long = {
+ val numeric = number.toLong
+ if (number.toLong > maxValue)
+ throw new NumberFormatException(message + " sent (" + number
+ + ") exceeds the limit (" + maxValue + ")")
+ numeric
+ }
}
class MemcachedParameters(override val data: Array[Byte], override val lifespan: Int,
override val maxIdle: Int, override val streamVersion: Long,
- val noReply: Boolean, val flags: Int, val delta: String,
+ val noReply: Boolean, val flags: Long, val delta: String,
val flushDelay: Int) extends RequestParameters(data, lifespan, maxIdle, streamVersion) {
override def toString = {
new StringBuilder().append("MemcachedParameters").append("{")
View
6 server/memcached/src/main/scala/org/infinispan/server/memcached/MemcachedValue.scala
@@ -13,7 +13,7 @@ import org.infinispan.marshall.Marshallable
*/
// TODO: putting Ids.MEMCACHED_CACHE_VALUE fails compilation in 2.8 - https://lampsvn.epfl.ch/trac/scala/ticket/2764
@Marshallable(externalizer = classOf[MemcachedValue.Externalizer], id = 56)
-class MemcachedValue(override val data: Array[Byte], override val version: Long, val flags: Int)
+class MemcachedValue(override val data: Array[Byte], override val version: Long, val flags: Long)
extends CacheValue(data, version) {
override def toString = {
@@ -33,14 +33,14 @@ object MemcachedValue {
output.write(cacheValue.data.length)
output.write(cacheValue.data)
output.writeLong(cacheValue.version)
- output.writeInt(cacheValue.flags)
+ output.writeLong(cacheValue.flags)
}
override def readObject(input: ObjectInput): AnyRef = {
val data = new Array[Byte](input.read())
input.readFully(data)
val version = input.readLong
- val flags = input.readInt
+ val flags = input.readLong
new MemcachedValue(data, version, flags)
}
}
View
3  server/memcached/src/main/scala/org/infinispan/server/memcached/TextProtocolUtil.scala
@@ -27,6 +27,9 @@ trait TextProtocolUtil {
val CR = 13
val LF = 10
+ val MAX_UNSIGNED_LONG = BigInt("18446744073709551615")
+ val MIN_UNSIGNED = BigInt("0")
+
/**
* In the particular case of Memcached, the end of operation/command
* is signaled by "\r\n" characters. So, if end of operation is
View
25 server/memcached/src/test/scala/org/infinispan/server/memcached/MemcachedFunctionalTest.scala
@@ -406,6 +406,31 @@ class MemcachedFunctionalTest extends MemcachedSingleNodeTest {
assertClientError(send("add boo2 0 0 -1\r\n"))
}
+ def testFlagsIsUnsigned(m: Method) {
+ val k = m.getName
+ assertClientError(send("set boo1 -1 0 0\r\n"))
+ assertStored(send("set " + k + " 4294967295 0 0\r\n"))
+ assertClientError(send("set boo2 4294967296 0 0\r\n"))
+ assertClientError(send("set boo2 18446744073709551615 0 0\r\n"))
+ }
+
+ def testIncrDecrIsUnsigned(m: Method) {
+ var k = m.getName
+ var f = client.set(k, 0, "0")
+ assertTrue(f.get(timeout, TimeUnit.SECONDS).booleanValue)
+ assertClientError(send("incr " + k + " -1\r\n"))
+ assertClientError(send("decr " + k + " -1\r\n"))
+ k = k + "-1"
+ f = client.set(k, 0, "0")
+ assertTrue(f.get(timeout, TimeUnit.SECONDS).booleanValue)
+ assertExpectedResponse(send("incr " + k + " 18446744073709551615\r\n"), "18446744073709551615", true)
+ k = k + "-1"
+ f = client.set(k, 0, "0")
+ assertTrue(f.get(timeout, TimeUnit.SECONDS).booleanValue)
+ assertClientError(send("incr " + k + " 18446744073709551616\r\n"))
+ assertClientError(send("decr " + k + " 18446744073709551616\r\n"))
+ }
+
// def testRegex {
// val notFoundRegex = new Regex("""\bNOT_FOUND\b""")
// assertEquals(notFoundRegex.findAllIn("NOT_FOUND\r\nNOT_FOUND\r\n").length, 2)
View
3  server/memcached/src/test/scala/org/infinispan/server/memcached/MemcachedSingleNodeTest.scala
@@ -89,6 +89,9 @@ abstract class MemcachedSingleNodeTest extends SingleCacheManagerTest with Memca
protected def assertError(resp: String) = assertExpectedResponse(resp, "ERROR", true)
@Test(enabled = false) // Disable explicitly to avoid TestNG thinking this is a test!!
+ protected def assertStored(resp: String) = assertExpectedResponse(resp, "STORED", true)
+
+ @Test(enabled = false) // Disable explicitly to avoid TestNG thinking this is a test!!
protected def assertExpectedResponse(resp: String, expectedResp: String, strictComparison: Boolean) {
if (strictComparison)
assertEquals(resp, expectedResp, "Instead response is: " + resp)
Something went wrong with that request. Please try again.