From 1b88eef3569f39c0596d2fb5b39d23301f65af0f Mon Sep 17 00:00:00 2001
From: Mark van Holsteijn <mark.van.holsteijn@gmail.com>
Date: Thu, 10 Jun 2021 21:39:15 +0200
Subject: [PATCH] improved implementation of GoogleStorageBucket

---
 lib/Data/GoogleCloudStorage.php     | 55 +++++++++++++++++------------
 tst/Data/GoogleCloudStorageTest.php | 43 +++++++++++++++++++---
 2 files changed, 71 insertions(+), 27 deletions(-)

diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php
index 75bb8a65..6bbea600 100644
--- a/lib/Data/GoogleCloudStorage.php
+++ b/lib/Data/GoogleCloudStorage.php
@@ -2,7 +2,6 @@
 
 namespace PrivateBin\Data;
 
-use DateTime;
 use Exception;
 use Google\Cloud\Core\Exception\NotFoundException;
 use Google\Cloud\Storage\StorageClient;
@@ -225,23 +224,22 @@ class GoogleCloudStorage extends AbstractData
      */
     public function purgeValues($namespace, $time)
     {
-        $prefix = 'config/' . $namespace . '/';
+        $path = 'config/' . $namespace;
         try {
-            foreach ($this->_bucket->objects(array('prefix' => $prefix)) as $object) {
-                $info        = $object->info();
-                $timeCreated = false;
-                if (key_exists('timeCreated', $info)) {
-                    $timeCreated = DateTime::createFromFormat(GoogleCloudStorage::DATETIME_FORMAT, $info['timeCreated']);
+            foreach ($this->_bucket->objects(array('prefix' => $path)) as $object) {
+                $name = $object->name();
+                if (strlen($name) > strlen($path) && substr($name, strlen($path), 1) !== '/') {
+                    continue;
                 }
-                if ($timeCreated && ($timeCreated->getTimestamp() < $time)) {
-                    try {
-                        $object->delete();
-                    } catch (NotFoundException $e) {
-                        // deleted by another instance.
-                    }
-                } else {
-                    if (!$timeCreated) {
-                        error_log('failed to parse create timestamp ' . $info['timeCreated'] . ' of object ' . $object->name());
+                $info        = $object->info();
+                if (key_exists('metadata', $info) && key_exists('value', $info['metadata'])) {
+                    $value = $info['metadata']['value'];
+                    if (is_numeric($value) && intval($value) < $time) {
+                        try {
+                            $object->delete();
+                        } catch (NotFoundException $e) {
+                            // deleted by another instance.
+                        }
                     }
                 }
             }
@@ -251,15 +249,24 @@ class GoogleCloudStorage extends AbstractData
     }
 
     /**
-     * This is the simplest thing that could possibly work.
-     * will be to tested for runtime performance.
+     * For GoogleCloudStorage, the value will also be stored in the metadata for the
+     * namespaces traffic_limiter and purge_limiter.
      * @inheritDoc
      */
     public function setValue($value, $namespace, $key = '')
     {
-        $key  = 'config/' . $namespace . '/' . $key;
+        if ($key === '') {
+            $key = 'config/' . $namespace;
+        } else {
+            $key = 'config/' . $namespace . '/' . $key;
+        }
+
         $data = Json::encode($value);
 
+        $metadata = array('namespace' => $namespace);
+        if ($namespace != 'salt') {
+            $metadata['value'] = strval($value);
+        }
         try {
             $this->_bucket->upload($data, array(
                 'name'          => $key,
@@ -267,7 +274,7 @@ class GoogleCloudStorage extends AbstractData
                 'predefinedAcl' => 'private',
                 'metadata'      => array(
                     'content-type' => 'application/json',
-                    'metadata'     => array('namespace' => $namespace),
+                    'metadata'     => $metadata,
                 ),
             ));
         } catch (Exception $e) {
@@ -279,13 +286,15 @@ class GoogleCloudStorage extends AbstractData
     }
 
     /**
-     * This is the simplest thing that could possibly work.
-     * will be to tested for runtime performance.
      * @inheritDoc
      */
     public function getValue($namespace, $key = '')
     {
-        $key = 'config/' . $namespace . '/' . $key;
+        if ($key === '') {
+            $key = 'config/' . $namespace;
+        } else {
+            $key = 'config/' . $namespace . '/' . $key;
+        }
         try {
             $o    = $this->_bucket->object($key);
             $data = $o->downloadAsString();
diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php
index 1c5190de..7945e2b1 100644
--- a/tst/Data/GoogleCloudStorageTest.php
+++ b/tst/Data/GoogleCloudStorageTest.php
@@ -141,8 +141,8 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase
     public function testKeyValueStore()
     {
         $salt = bin2hex(random_bytes(256));
-        $this->_model->setValue($salt, 'salt', 'master');
-        $storedSalt = $this->_model->getValue('salt', 'master');
+        $this->_model->setValue($salt, 'salt', '');
+        $storedSalt = $this->_model->getValue('salt', '');
         $this->assertEquals($salt, $storedSalt);
         $this->_model->purgeValues('salt', time() + 60);
         $this->assertFalse($this->_model->getValue('salt', 'master'));
@@ -159,12 +159,47 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase
         $this->assertFalse($this->_model->getValue('traffic_limiter', $client));
 
         $purgeAt = $expire + (15 * 60);
-        $this->_model->setValue($purgeAt, 'purge_limiter', 'at');
-        $storedPurgedAt = $this->_model->getValue('purge_limiter', 'at');
+        $this->_model->setValue($purgeAt, 'purge_limiter', '');
+        $storedPurgedAt = $this->_model->getValue('purge_limiter', '');
         $this->assertEquals($purgeAt, $storedPurgedAt);
         $this->_model->purgeValues('purge_limiter', time() + 60);
         $this->assertFalse($this->_model->getValue('purge_limiter', 'at'));
     }
+
+    /**
+     * @throws Exception
+     */
+    public function testKeyValuePurgeTrafficLimiter()
+    {
+        $salt   = bin2hex(random_bytes(256));
+        $client = hash_hmac('sha512', '127.0.0.1', $salt);
+        $expire = time();
+        $this->_model->setValue($expire, 'traffic_limiter', $client);
+        $storedExpired = $this->_model->getValue('traffic_limiter', $client);
+        $this->assertEquals($expire, $storedExpired);
+
+        $this->_model->purgeValues('traffic_limiter', time() - 60);
+        $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client));
+
+        $this->_model->purgeValues('traffic_limiter', time() + 60);
+        $this->assertFalse($this->_model->getValue('traffic_limiter', $client));
+    }
+
+    public function testKeyValuePurgeTrafficLimiterWithKey()
+    {
+        $salt   = bin2hex(random_bytes(256));
+        $client = hash_hmac('sha512', '127.0.0.1', $salt);
+        $expire = time();
+        $this->_model->setValue($expire, 'traffic_limiter', $client);
+        $storedExpired = $this->_model->getValue('traffic_limiter', $client);
+        $this->assertEquals($expire, $storedExpired);
+
+        $this->_model->purgeValues('traffic_limiter', time() - 60);
+        $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client));
+
+        $this->_model->purgeValues('traffic_limiter', time() + 60);
+        $this->assertFalse($this->_model->getValue('traffic_limiter', $client));
+    }
 }
 
 /**