You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/05/09 22:38:36 UTC

[GitHub] csantanapr closed pull request #3615: Terminate route for invalid list skip values

csantanapr closed pull request #3615: Terminate route for invalid list skip values
URL: https://github.com/apache/incubator-openwhisk/pull/3615
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index 40685c0994..5a31855cd7 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -151,9 +151,12 @@ object Messages {
   }
 
   def listLimitOutOfRange(collection: String, value: Int, max: Int) = {
-    s"The value $value is not in the range of 0 to $max for $collection."
+    s"The value '$value' is not in the range of 0 to $max for $collection."
   }
-  def listLimitIsNotAString = s"The API expects the 'limit' value to be an integer but the given value is not."
+  def listSkipOutOfRange(collection: String, value: Int) = {
+    s"The value '$value' is not greater than or equal to 0 for $collection."
+  }
+  def argumentNotInteger(collection: String, value: String) = s"The value '$value' is not an integer for $collection."
 
   def truncateLogs(limit: ByteSize) = {
     s"Logs were truncated because the total bytes size exceeds the limit of ${limit.toBytes} bytes."
diff --git a/core/controller/src/main/scala/whisk/core/controller/Actions.scala b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
index fd5f94340d..25be8fa4ee 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Actions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
@@ -34,7 +34,7 @@ import spray.json._
 import spray.json.DefaultJsonProtocol._
 import whisk.common.TransactionId
 import whisk.core.WhiskConfig
-import whisk.core.controller.RestApiCommons.ListLimit
+import whisk.core.controller.RestApiCommons.{ListLimit, ListSkip}
 import whisk.core.controller.actions.PostActionActivation
 import whisk.core.database.CacheChangeNotification
 import whisk.core.database.NoDocumentException
@@ -339,20 +339,22 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
    * - 500 Internal Server Error
    */
   override def list(user: Identity, namespace: EntityPath)(implicit transid: TransactionId) = {
-    parameter('skip ? 0, 'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit), 'count ? false) {
-      (skip, limit, count) =>
-        if (!count) {
-          listEntities {
-            WhiskAction.listCollectionInNamespace(entityStore, namespace, skip, limit.n, includeDocs = false) map {
-              list =>
-                list.fold((js) => js, (as) => as.map(WhiskAction.serdes.write(_)))
-            }
-          }
-        } else {
-          countEntities {
-            WhiskAction.countCollectionInNamespace(entityStore, namespace, skip)
+    parameter(
+      'skip.as[ListSkip] ? ListSkip(collection.defaultListSkip),
+      'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit),
+      'count ? false) { (skip, limit, count) =>
+      if (!count) {
+        listEntities {
+          WhiskAction.listCollectionInNamespace(entityStore, namespace, skip.n, limit.n, includeDocs = false) map {
+            list =>
+              list.fold((js) => js, (as) => as.map(WhiskAction.serdes.write(_)))
           }
         }
+      } else {
+        countEntities {
+          WhiskAction.countCollectionInNamespace(entityStore, namespace, skip.n)
+        }
+      }
     }
   }
 
@@ -697,6 +699,10 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
 
   /** Custom unmarshaller for query parameters "limit" for "list" operations. */
   private implicit val stringToListLimit: Unmarshaller[String, ListLimit] = RestApiCommons.stringToListLimit(collection)
+
+  /** Custom unmarshaller for query parameters "skip" for "list" operations. */
+  private implicit val stringToListSkip: Unmarshaller[String, ListSkip] = RestApiCommons.stringToListSkip(collection)
+
 }
 
 private case class TooManyActionsInSequence() extends IllegalArgumentException
diff --git a/core/controller/src/main/scala/whisk/core/controller/Activations.scala b/core/controller/src/main/scala/whisk/core/controller/Activations.scala
index ff31e4fbf2..4a141fc7ff 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Activations.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Activations.scala
@@ -30,7 +30,7 @@ import akka.http.scaladsl.unmarshalling._
 import spray.json.DefaultJsonProtocol.RootJsObjectFormat
 import whisk.common.TransactionId
 import whisk.core.containerpool.logging.LogStore
-import whisk.core.controller.RestApiCommons.ListLimit
+import whisk.core.controller.RestApiCommons.{ListLimit, ListSkip}
 import whisk.core.database.StaleParameter
 import whisk.core.entitlement.Privilege.READ
 import whisk.core.entitlement.{Collection, Privilege, Resource}
@@ -64,6 +64,10 @@ object WhiskActivationsApi {
   private implicit val stringToListLimit: Unmarshaller[String, ListLimit] =
     RestApiCommons.stringToListLimit(Collection(Collection.ACTIVATIONS))
 
+  /** Custom unmarshaller for query parameters "skip" for "list" operations. */
+  private implicit val stringToListSkip: Unmarshaller[String, ListSkip] =
+    RestApiCommons.stringToListSkip(Collection(Collection.ACTIVATIONS))
+
 }
 
 /** A trait implementing the activations API. */
@@ -143,9 +147,10 @@ trait WhiskActivationsApi extends Directives with AuthenticatedRouteProvider wit
     import WhiskActivationsApi.stringToRestrictedEntityPath
     import WhiskActivationsApi.stringToInstantDeserializer
     import WhiskActivationsApi.stringToListLimit
+    import WhiskActivationsApi.stringToListSkip
 
     parameter(
-      'skip ? 0,
+      'skip.as[ListSkip] ? ListSkip(collection.defaultListSkip),
       'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit),
       'count ? false,
       'docs ? false,
@@ -157,7 +162,7 @@ trait WhiskActivationsApi extends Directives with AuthenticatedRouteProvider wit
           WhiskActivation.countCollectionInNamespace(
             activationStore,
             name.flatten.map(p => namespace.addPath(p)).getOrElse(namespace),
-            skip,
+            skip.n,
             since,
             upto,
             StaleParameter.UpdateAfter,
@@ -172,7 +177,7 @@ trait WhiskActivationsApi extends Directives with AuthenticatedRouteProvider wit
               activationStore,
               namespace,
               action,
-              skip,
+              skip.n,
               limit.n,
               docs,
               since,
@@ -182,7 +187,7 @@ trait WhiskActivationsApi extends Directives with AuthenticatedRouteProvider wit
             WhiskActivation.listCollectionInNamespace(
               activationStore,
               namespace,
-              skip,
+              skip.n,
               limit.n,
               docs,
               since,
diff --git a/core/controller/src/main/scala/whisk/core/controller/Packages.scala b/core/controller/src/main/scala/whisk/core/controller/Packages.scala
index 77000a859f..509cc7623f 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Packages.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Packages.scala
@@ -26,7 +26,7 @@ import akka.http.scaladsl.server.{RequestContext, RouteResult}
 import akka.http.scaladsl.unmarshalling.Unmarshaller
 
 import whisk.common.TransactionId
-import whisk.core.controller.RestApiCommons.ListLimit
+import whisk.core.controller.RestApiCommons.{ListLimit, ListSkip}
 import whisk.core.database.{CacheChangeNotification, DocumentTypeMismatchException, NoDocumentException}
 import whisk.core.entitlement._
 import whisk.core.entity._
@@ -168,26 +168,28 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
    * - 500 Internal Server Error
    */
   override def list(user: Identity, namespace: EntityPath)(implicit transid: TransactionId) = {
-    parameter('skip ? 0, 'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit), 'count ? false) {
-      (skip, limit, count) =>
-        val viewName = if (user.namespace.toPath == namespace) WhiskPackage.view else WhiskPackage.publicPackagesView
-        if (!count) {
-          listEntities {
-            WhiskPackage
-              .listCollectionInNamespace(
-                entityStore,
-                namespace,
-                skip,
-                limit.n,
-                includeDocs = false,
-                viewName = viewName)
-              .map(_.fold((js) => js, (ps) => ps.map(WhiskPackage.serdes.write(_))))
-          }
-        } else {
-          countEntities {
-            WhiskPackage.countCollectionInNamespace(entityStore, namespace, skip, viewName = viewName)
-          }
+    parameter(
+      'skip.as[ListSkip] ? ListSkip(collection.defaultListSkip),
+      'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit),
+      'count ? false) { (skip, limit, count) =>
+      val viewName = if (user.namespace.toPath == namespace) WhiskPackage.view else WhiskPackage.publicPackagesView
+      if (!count) {
+        listEntities {
+          WhiskPackage
+            .listCollectionInNamespace(
+              entityStore,
+              namespace,
+              skip.n,
+              limit.n,
+              includeDocs = false,
+              viewName = viewName)
+            .map(_.fold((js) => js, (ps) => ps.map(WhiskPackage.serdes.write(_))))
+        }
+      } else {
+        countEntities {
+          WhiskPackage.countCollectionInNamespace(entityStore, namespace, skip.n, viewName = viewName)
         }
+      }
     }
   }
 
@@ -330,4 +332,8 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
 
   /** Custom unmarshaller for query parameters "limit" for "list" operations. */
   private implicit val stringToListLimit: Unmarshaller[String, ListLimit] = RestApiCommons.stringToListLimit(collection)
+
+  /** Custom unmarshaller for query parameters "skip" for "list" operations. */
+  private implicit val stringToListSkip: Unmarshaller[String, ListSkip] = RestApiCommons.stringToListSkip(collection)
+
 }
diff --git a/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala b/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
index bc1e0b5923..9791fa8bb8 100644
--- a/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
@@ -126,7 +126,21 @@ protected[controller] object RestApiCommons {
         case Success(n) =>
           throw new IllegalArgumentException(
             Messages.listLimitOutOfRange(collection.path, n, Collection.MAX_LIST_LIMIT))
-        case Failure(t) => throw new IllegalArgumentException(Messages.listLimitIsNotAString)
+        case Failure(t) => throw new IllegalArgumentException(Messages.argumentNotInteger(collection.path, value))
+      }
+    }
+  }
+
+  /** Custom unmarshaller for query parameters "skip" for "list" operations. */
+  case class ListSkip(n: Int)
+
+  def stringToListSkip(collection: Collection): Unmarshaller[String, ListSkip] = {
+    Unmarshaller.strict[String, ListSkip] { value =>
+      Try { value.toInt } match {
+        case Success(n) if (n >= 0) => ListSkip(n)
+        case Success(n) =>
+          throw new IllegalArgumentException(Messages.listSkipOutOfRange(collection.path, n))
+        case Failure(t) => throw new IllegalArgumentException(Messages.argumentNotInteger(collection.path, value))
       }
     }
   }
diff --git a/core/controller/src/main/scala/whisk/core/controller/Rules.scala b/core/controller/src/main/scala/whisk/core/controller/Rules.scala
index dc511be672..5e99ff9894 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Rules.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Rules.scala
@@ -24,7 +24,7 @@ import akka.http.scaladsl.server.StandardRoute
 import akka.http.scaladsl.unmarshalling.Unmarshaller
 import spray.json.DeserializationException
 import whisk.common.TransactionId
-import whisk.core.controller.RestApiCommons.ListLimit
+import whisk.core.controller.RestApiCommons.{ListLimit, ListSkip}
 import whisk.core.database.{CacheChangeNotification, DocumentConflictException, NoDocumentException}
 import whisk.core.entitlement.{Collection, Privilege, ReferencedEntities}
 import whisk.core.entity._
@@ -253,19 +253,21 @@ trait WhiskRulesApi extends WhiskCollectionAPI with ReferencedEntities {
    * - 500 Internal Server Error
    */
   override def list(user: Identity, namespace: EntityPath)(implicit transid: TransactionId) = {
-    parameter('skip ? 0, 'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit), 'count ? false) {
-      (skip, limit, count) =>
-        if (!count) {
-          listEntities {
-            WhiskRule.listCollectionInNamespace(entityStore, namespace, skip, limit.n, includeDocs = true) map { list =>
-              list.fold((js) => js, (rls) => rls.map(WhiskRule.serdes.write(_)))
-            }
-          }
-        } else {
-          countEntities {
-            WhiskRule.countCollectionInNamespace(entityStore, namespace, skip)
+    parameter(
+      'skip.as[ListSkip] ? ListSkip(collection.defaultListSkip),
+      'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit),
+      'count ? false) { (skip, limit, count) =>
+      if (!count) {
+        listEntities {
+          WhiskRule.listCollectionInNamespace(entityStore, namespace, skip.n, limit.n, includeDocs = true) map { list =>
+            list.fold((js) => js, (rls) => rls.map(WhiskRule.serdes.write(_)))
           }
         }
+      } else {
+        countEntities {
+          WhiskRule.countCollectionInNamespace(entityStore, namespace, skip.n)
+        }
+      }
     }
   }
 
@@ -428,6 +430,10 @@ trait WhiskRulesApi extends WhiskCollectionAPI with ReferencedEntities {
 
   /** Custom unmarshaller for query parameters "limit" for "list" operations. */
   private implicit val stringToListLimit: Unmarshaller[String, ListLimit] = RestApiCommons.stringToListLimit(collection)
+
+  /** Custom unmarshaller for query parameters "skip" for "list" operations. */
+  private implicit val stringToListSkip: Unmarshaller[String, ListSkip] = RestApiCommons.stringToListSkip(collection)
+
 }
 
 private case class IgnoredRuleActivation(noop: Boolean) extends Throwable
diff --git a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
index 760ae04f67..b7f0438a62 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
@@ -38,7 +38,7 @@ import com.typesafe.sslconfig.akka.AkkaSSLConfig
 import pureconfig.loadConfigOrThrow
 import spray.json._
 import whisk.common.{Https, TransactionId}
-import whisk.core.controller.RestApiCommons.ListLimit
+import whisk.core.controller.RestApiCommons.{ListLimit, ListSkip}
 import whisk.core.database.CacheChangeNotification
 import whisk.core.entitlement.Collection
 import whisk.core.entity._
@@ -227,20 +227,22 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
    * - 500 Internal Server Error
    */
   override def list(user: Identity, namespace: EntityPath)(implicit transid: TransactionId) = {
-    parameter('skip ? 0, 'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit), 'count ? false) {
-      (skip, limit, count) =>
-        if (!count) {
-          listEntities {
-            WhiskTrigger.listCollectionInNamespace(entityStore, namespace, skip, limit.n, includeDocs = false) map {
-              list =>
-                list.fold((js) => js, (ts) => ts.map(WhiskTrigger.serdes.write(_)))
-            }
-          }
-        } else {
-          countEntities {
-            WhiskTrigger.countCollectionInNamespace(entityStore, namespace, skip)
+    parameter(
+      'skip.as[ListSkip] ? ListSkip(collection.defaultListSkip),
+      'limit.as[ListLimit] ? ListLimit(collection.defaultListLimit),
+      'count ? false) { (skip, limit, count) =>
+      if (!count) {
+        listEntities {
+          WhiskTrigger.listCollectionInNamespace(entityStore, namespace, skip.n, limit.n, includeDocs = false) map {
+            list =>
+              list.fold((js) => js, (ts) => ts.map(WhiskTrigger.serdes.write(_)))
           }
         }
+      } else {
+        countEntities {
+          WhiskTrigger.countCollectionInNamespace(entityStore, namespace, skip.n)
+        }
+      }
     }
   }
 
@@ -424,4 +426,7 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
   /** Custom unmarshaller for query parameters "limit" for "list" operations. */
   private implicit val stringToListLimit: Unmarshaller[String, ListLimit] = RestApiCommons.stringToListLimit(collection)
 
+  /** Custom unmarshaller for query parameters "skip" for "list" operations. */
+  private implicit val stringToListSkip: Unmarshaller[String, ListSkip] = RestApiCommons.stringToListSkip(collection)
+
 }
diff --git a/core/controller/src/main/scala/whisk/core/entitlement/Collection.scala b/core/controller/src/main/scala/whisk/core/entitlement/Collection.scala
index b5063f2960..5f658768e6 100644
--- a/core/controller/src/main/scala/whisk/core/entitlement/Collection.scala
+++ b/core/controller/src/main/scala/whisk/core/entitlement/Collection.scala
@@ -45,9 +45,11 @@ import whisk.core.entity.types.EntityStore
  * @param path the name of the collection (the resource path in URI and the view name in the datastore)
  * @param activate the privilege for an activate (may be ACTIVATE or REJECT for example)
  * @param listLimit the default limit on number of entities returned from a collection on a list operation
+ * @param skipLimit the default skip on number of entities returned from a collection on a list operation
  */
 protected[core] case class Collection protected (val path: String,
-                                                 val defaultListLimit: Int = Collection.DEFAULT_LIST_LIMIT) {
+                                                 val defaultListLimit: Int = Collection.DEFAULT_LIST_LIMIT,
+                                                 val defaultListSkip: Int = Collection.DEFAULT_SKIP_LIMIT) {
   override def toString = path
 
   /** Determines the right to request for the resources and context. */
@@ -110,6 +112,7 @@ protected[core] object Collection {
   /** Number of records allowed per query. */
   protected[core] val DEFAULT_LIST_LIMIT = 30
   protected[core] val MAX_LIST_LIMIT = 200
+  protected[core] val DEFAULT_SKIP_LIMIT = 0
 
   protected[core] val ACTIONS = WhiskAction.collectionName
   protected[core] val TRIGGERS = WhiskTrigger.collectionName
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
index 380c5f0b76..e20936afeb 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
@@ -100,6 +100,39 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
     }
   }
 
+  it should "reject list when limit is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?limit=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.ACTIONS, notAnInteger)
+      }
+    }
+  }
+
+  it should "reject list when skip is negative" in {
+    implicit val tid = transid()
+    val negativeSkip = -1
+    val response = Get(s"$collectionPath?skip=$negativeSkip") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.listSkipOutOfRange(Collection.ACTIONS, negativeSkip)
+      }
+    }
+  }
+
+  it should "reject list when skip is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?skip=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.ACTIONS, notAnInteger)
+      }
+    }
+  }
+
   // ?docs disabled
   ignore should "list action by default namespace with full docs" in {
     implicit val tid = transid()
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala
index 1e4e684b1d..b9a8915296 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActivationsApiTests.scala
@@ -385,7 +385,7 @@ class ActivationsApiTests extends ControllerTestCommon with WhiskActivationsApi
     }
   }
 
-  it should "reject activation list when limit is greater than maximum allowed value" in {
+  it should "reject list when limit is greater than maximum allowed value" in {
     implicit val tid = transid()
     val exceededMaxLimit = Collection.MAX_LIST_LIMIT + 1
     val response = Get(s"$collectionPath?limit=$exceededMaxLimit") ~> Route.seal(routes(creds)) ~> check {
@@ -396,6 +396,39 @@ class ActivationsApiTests extends ControllerTestCommon with WhiskActivationsApi
     }
   }
 
+  it should "reject list when limit is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?limit=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.ACTIVATIONS, notAnInteger)
+      }
+    }
+  }
+
+  it should "reject list when skip is negative" in {
+    implicit val tid = transid()
+    val negativeSkip = -1
+    val response = Get(s"$collectionPath?skip=$negativeSkip") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.listSkipOutOfRange(Collection.ACTIVATIONS, negativeSkip)
+      }
+    }
+  }
+
+  it should "reject list when skip is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?skip=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.ACTIVATIONS, notAnInteger)
+      }
+    }
+  }
+
   it should "reject get activation by namespace and action name when action name is not a valid name" in {
     implicit val tid = transid()
     Get(s"$collectionPath?name=0%20") ~> Route.seal(routes(creds)) ~> check {
diff --git a/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala
index 706ca9cb66..9e48eed760 100644
--- a/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala
@@ -162,6 +162,39 @@ class PackagesApiTests extends ControllerTestCommon with WhiskPackagesApi {
     }
   }
 
+  it should "reject list when limit is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?limit=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.PACKAGES, notAnInteger)
+      }
+    }
+  }
+
+  it should "reject list when skip is negative" in {
+    implicit val tid = transid()
+    val negativeSkip = -1
+    val response = Get(s"$collectionPath?skip=$negativeSkip") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.listSkipOutOfRange(Collection.PACKAGES, negativeSkip)
+      }
+    }
+  }
+
+  it should "reject list when skip is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?skip=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.PACKAGES, notAnInteger)
+      }
+    }
+  }
+
   ignore should "list all public packages excluding bindings" in {
     implicit val tid = transid()
     // create packages and package bindings, set some public and confirm API lists only public packages
diff --git a/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala
index 78cea3c23a..f5e7449b89 100644
--- a/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala
@@ -104,6 +104,39 @@ class RulesApiTests extends ControllerTestCommon with WhiskRulesApi {
     }
   }
 
+  it should "reject list when limit is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?limit=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.RULES, notAnInteger)
+      }
+    }
+  }
+
+  it should "reject list when skip is negative" in {
+    implicit val tid = transid()
+    val negativeSkip = -1
+    val response = Get(s"$collectionPath?skip=$negativeSkip") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.listSkipOutOfRange(Collection.RULES, negativeSkip)
+      }
+    }
+  }
+
+  it should "reject list when skip is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?skip=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.RULES, notAnInteger)
+      }
+    }
+  }
+
   //?docs disabled
   ignore should "list rules by default namespace with full docs" in {
     implicit val tid = transid()
diff --git a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
index 9f795409a9..aceae91910 100644
--- a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
@@ -108,6 +108,39 @@ class TriggersApiTests extends ControllerTestCommon with WhiskTriggersApi {
     }
   }
 
+  it should "reject list when limit is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?limit=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.TRIGGERS, notAnInteger)
+      }
+    }
+  }
+
+  it should "reject list when skip is negative" in {
+    implicit val tid = transid()
+    val negativeSkip = -1
+    val response = Get(s"$collectionPath?skip=$negativeSkip") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.listSkipOutOfRange(Collection.TRIGGERS, negativeSkip)
+      }
+    }
+  }
+
+  it should "reject list when skip is not an integer" in {
+    implicit val tid = transid()
+    val notAnInteger = "string"
+    val response = Get(s"$collectionPath?skip=$notAnInteger") ~> Route.seal(routes(creds)) ~> check {
+      status should be(BadRequest)
+      responseAs[String] should include {
+        Messages.argumentNotInteger(Collection.TRIGGERS, notAnInteger)
+      }
+    }
+  }
+
   // ?docs disabled
   ignore should "list triggers by default namespace with full docs" in {
     implicit val tid = transid()


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services