From 25f5eed56faa81969a0e0cd6a732f789deaddb0c Mon Sep 17 00:00:00 2001
From: bhagyashris <bhagyashri.shewale@nttdata.com>
Date: Wed, 18 May 2016 18:04:30 +0530
Subject: [PATCH] Add check to limit maximum value of age_in_days

If you pass age_in_days value greater than unix epoch
then it raises OverflowError. Added check to ensure
age_in_days is within unix epoch time to fix this problem.

Removed the redundant check for age_in_days value from
cinder.db.sqlalchemy.api module which is already checked
at cinder.cmd.manage module.

Closes-Bug: #1543937
Change-Id: Ib418627fb8527a1275c2656d1451f1e1dfeb72ef
---
 cinder/cmd/manage.py               |  4 ++++
 cinder/db/sqlalchemy/api.py        |  4 ----
 cinder/tests/unit/db/test_purge.py |  4 ----
 cinder/tests/unit/test_cmd.py      | 36 ++++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/cinder/cmd/manage.py b/cinder/cmd/manage.py
index 6280a3421..3e6d3a3da 100644
--- a/cinder/cmd/manage.py
+++ b/cinder/cmd/manage.py
@@ -58,6 +58,7 @@ from __future__ import print_function
 import logging as python_logging
 import os
 import sys
+import time
 
 from oslo_config import cfg
 from oslo_db import exception as db_exc
@@ -229,6 +230,9 @@ class DbCommands(object):
         if age_in_days <= 0:
             print(_("Must supply a positive, non-zero value for age"))
             sys.exit(1)
+        if age_in_days >= (int(time.time()) / 86400):
+            print(_("Maximum age is count of days since epoch."))
+            sys.exit(1)
         ctxt = context.get_admin_context()
 
         try:
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index aa9258420..6493c4b42 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -4402,10 +4402,6 @@ def purge_deleted_rows(context, age_in_days):
         msg = _('Invalid value for age, %(age)s') % {'age': age_in_days}
         LOG.exception(msg)
         raise exception.InvalidParameterValue(msg)
-    if age_in_days <= 0:
-        msg = _('Must supply a positive value for age')
-        LOG.error(msg)
-        raise exception.InvalidParameterValue(msg)
 
     engine = get_engine()
     session = get_session()
diff --git a/cinder/tests/unit/db/test_purge.py b/cinder/tests/unit/db/test_purge.py
index c3975352c..7e45459ce 100644
--- a/cinder/tests/unit/db/test_purge.py
+++ b/cinder/tests/unit/db/test_purge.py
@@ -98,10 +98,6 @@ class PurgeDeletedTest(test.TestCase):
         self.assertRaises(exception.InvalidParameterValue,
                           db.purge_deleted_rows, self.context,
                           age_in_days='ten')
-        # Test with negative value
-        self.assertRaises(exception.InvalidParameterValue,
-                          db.purge_deleted_rows, self.context,
-                          age_in_days=-1)
 
     def test_purge_deleted_rows_integrity_failure(self):
         dialect = self.engine.url.get_dialect()
diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py
index a24385e37..f8220334f 100644
--- a/cinder/tests/unit/test_cmd.py
+++ b/cinder/tests/unit/test_cmd.py
@@ -12,6 +12,7 @@
 
 import datetime
 import sys
+import time
 
 import mock
 from oslo_config import cfg
@@ -374,6 +375,11 @@ class TestCinderManageCmd(test.TestCase):
     def tearDown(self):
         super(TestCinderManageCmd, self).tearDown()
 
+    def _test_purge_invalid_age_in_days(self, age_in_days):
+        db_cmds = cinder_manage.DbCommands()
+        ex = self.assertRaises(SystemExit, db_cmds.purge, age_in_days)
+        self.assertEqual(1, ex.code)
+
     @mock.patch('cinder.db.migration.db_sync')
     def test_db_commands_sync(self, db_sync):
         version = mock.MagicMock()
@@ -409,6 +415,36 @@ class TestCinderManageCmd(test.TestCase):
             version_cmds.__call__()
             version_string.assert_called_once_with()
 
+    def test_purge_age_in_days_value_equal_to_zero(self):
+        age_in_days = 0
+        self._test_purge_invalid_age_in_days(age_in_days)
+
+    def test_purge_with_negative_age_in_days(self):
+        age_in_days = -1
+        self._test_purge_invalid_age_in_days(age_in_days)
+
+    def test_purge_exceeded_age_in_days_limit(self):
+        age_in_days = int(time.time() / 86400) + 1
+        self._test_purge_invalid_age_in_days(age_in_days)
+
+    @mock.patch('cinder.db.sqlalchemy.api.purge_deleted_rows')
+    @mock.patch('cinder.context.get_admin_context')
+    def test_purge_less_than_age_in_days_limit(self, get_admin_context,
+                                               purge_deleted_rows):
+        age_in_days = int(time.time() / 86400) - 1
+        ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
+                                      is_admin=True)
+        get_admin_context.return_value = ctxt
+
+        purge_deleted_rows.return_value = None
+
+        db_cmds = cinder_manage.DbCommands()
+        db_cmds.purge(age_in_days)
+
+        get_admin_context.assert_called_once_with()
+        purge_deleted_rows.assert_called_once_with(
+            ctxt, age_in_days=age_in_days)
+
     @mock.patch('cinder.db.service_get_all')
     @mock.patch('cinder.context.get_admin_context')
     def test_host_commands_list(self, get_admin_context, service_get_all):