From 10142fd9567fbe71b008dfd89e86aac63ddd4095 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Mon, 5 Jun 2023 12:53:08 -0400 Subject: [PATCH 1/4] using ->end() instead of exit in controllers --- controllers/GroupsController.php | 6 ++---- controllers/ItemsController.php | 12 ++++-------- controllers/KeysController.php | 18 ++++-------------- controllers/MappingsController.php | 25 ++++++++++++------------- controllers/StorageController.php | 12 ++++++------ 5 files changed, 28 insertions(+), 45 deletions(-) diff --git a/controllers/GroupsController.php b/controllers/GroupsController.php index 7cb54f38..3670c10e 100644 --- a/controllers/GroupsController.php +++ b/controllers/GroupsController.php @@ -174,8 +174,7 @@ public function groups() { $group->erase(); Zotero_DB::commit(); - header("HTTP/1.1 204 No Content"); - exit; + $this->e204(); } @@ -486,8 +485,7 @@ public function groupUsers() { Zotero_DB::commit(); - header("HTTP/1.1 204 No Content"); - exit; + $this->e204(); } // Single user diff --git a/controllers/ItemsController.php b/controllers/ItemsController.php index d3a6e489..4a0ecb9f 100644 --- a/controllers/ItemsController.php +++ b/controllers/ItemsController.php @@ -787,7 +787,6 @@ private function _handleFileRequest($item) { } StatsD::increment("storage.view", 1); $this->redirect($url); - exit; } // File download @@ -804,7 +803,6 @@ private function _handleFileRequest($item) { StatsD::increment("storage.download", 1); $this->redirect($url); - exit; } else if ($this->method == 'POST' || $this->method == 'PATCH') { @@ -1066,7 +1064,7 @@ private function _handleFileRequest($item) { header('Content-Type: application/json'); echo json_encode($params); } - exit; + $this->end(); } // @@ -1128,9 +1126,8 @@ private function _handleFileRequest($item) { Zotero_DB::commit(); - header("HTTP/1.1 204 No Content"); header("Last-Modified-Version: " . $item->version); - exit; + $this->e204(); } @@ -1181,12 +1178,11 @@ private function _handleFileRequest($item) { Zotero_DB::commit(); - header("HTTP/1.1 204 No Content"); - exit; + $this->e204(); } throw new Exception("Invalid request", Z_ERROR_INVALID_INPUT); } - exit; + $this->end(); } } diff --git a/controllers/KeysController.php b/controllers/KeysController.php index f7357bc7..5c6d0fa4 100644 --- a/controllers/KeysController.php +++ b/controllers/KeysController.php @@ -148,8 +148,7 @@ public function keys() { $keyObj->erase(); Zotero_DB::commit(); - header("HTTP/1.1 204 No Content"); - exit; + $this->e204(); } else { @@ -316,20 +315,11 @@ public function keys() { } } - if ($this->apiVersion >= 3) { - $this->end(); - } - else { + if ($this->apiVersion < 3) { + unset($this->queryParams['format']); header('Content-Type: application/xml'); - $xmlstr = $this->responseXML->asXML(); - - $doc = new DOMDocument('1.0'); - - $doc->loadXML($xmlstr); - $doc->formatOutput = true; - echo $doc->saveXML(); - exit; } + $this->end(); } diff --git a/controllers/MappingsController.php b/controllers/MappingsController.php index 7cbb93c6..b13d501a 100644 --- a/controllers/MappingsController.php +++ b/controllers/MappingsController.php @@ -34,7 +34,9 @@ public function mappings() { $locale = !empty($_GET['locale']) ? \Zotero\Schema::resolveLocale($_GET['locale']) : 'en-US'; - + unset($this->queryParams['format']); + header("Content-Type: application/json"); + if ($this->subset == 'itemTypeFields') { if (empty($_GET['itemType'])) { $this->e400("'itemType' not provided"); @@ -63,7 +65,7 @@ public function mappings() { // Notes and attachments don't have creators if ($itemType == 'note' || $itemType == 'attachment') { echo "[]"; - exit; + $this->end(); } } @@ -77,9 +79,8 @@ public function mappings() { $ttl = 60; $json = Z_Core::$MC->get($cacheKey); if ($json) { - header("Content-Type: application/json"); echo $json; - exit; + $this->end(); } switch ($this->subset) { @@ -132,12 +133,10 @@ public function mappings() { ); } - header("Content-Type: application/json"); $json = Zotero_Utilities::formatJSON($json); Z_Core::$MC->set($cacheKey, $json, $ttl); - echo $json; - exit; + $this->end(); } @@ -145,7 +144,10 @@ public function newItem() { if (empty($_GET['itemType'])) { $this->e400("'itemType' not provided"); } - + + unset($this->queryParams['format']); + header("Content-Type: application/json"); + $itemType = $_GET['itemType']; if ($itemType == 'attachment') { if (empty($_GET['linkMode'])) { @@ -200,9 +202,8 @@ public function newItem() { $ttl = 60; $json = Z_Core::$MC->get($cacheKey); if ($json) { - header("Content-Type: application/json"); echo $json; - exit; + $this->end(); } // Generate template @@ -325,12 +326,10 @@ public function newItem() { unset($json['relations']); } - header("Content-Type: application/json"); $json = Zotero_Utilities::formatJSON($json); Z_Core::$MC->set($cacheKey, $json, $ttl); - echo $json; - exit; + $this->end(); } } diff --git a/controllers/StorageController.php b/controllers/StorageController.php index 2a8e33df..edbfe180 100644 --- a/controllers/StorageController.php +++ b/controllers/StorageController.php @@ -54,7 +54,7 @@ public function laststoragesync() { } echo $lastSync; - exit; + $this->end(); } @@ -66,8 +66,7 @@ public function removestoragefiles() { $sql = "DELETE FROM storageFileLibraries WHERE libraryID = ?"; Zotero_DB::query($sql, $this->objectLibraryID); - header("HTTP/1.1 204 No Content"); - exit; + $this->e204(); } @@ -146,9 +145,10 @@ public function storageadmin() { Zotero_DB::commit(); - header('application/xml'); - echo $xml->asXML(); - exit; + $this->responseXML = $xml; + unset($this->queryParams['format']); + header('Content-Type: application/xml'); + $this->end(); } From 3efb9c377061bbdabeb72994cb270886793df0f1 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Mon, 5 Jun 2023 15:10:43 -0400 Subject: [PATCH 2/4] on 429 error, make client wait till full capacity is reached before next 200 --- include/RequestLimiter.inc.php | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/include/RequestLimiter.inc.php b/include/RequestLimiter.inc.php index a8902a83..49c52ca0 100644 --- a/include/RequestLimiter.inc.php +++ b/include/RequestLimiter.inc.php @@ -104,6 +104,12 @@ public static function checkBucketRate($params) { return null; } $prefix = self::REDIS_PREFIX .'rrl:' . $params['bucket']; + + // If bucket should wait for full capacity, reject + if (self::$redis->get($prefix . ".fc")) { + return false; + } + $keys = [$prefix . '.tk', $prefix . '.ts']; $args = [$params['replenishRate'], $params['capacity'], time()]; try { @@ -113,7 +119,14 @@ public static function checkBucketRate($params) { Z_Core::logError("Warning: " . $e); return null; } - return !!$res[0]; + // If capacity was reached, save how soon it will be re-filled. + // Requests before that will be rejected. + $allowed = !!$res[0]; + if (!$allowed) { + self::addFullCapacityTimeout($prefix, $params['replenishRate'], $params['capacity']); + return false; + } + return true; } /** @@ -186,4 +199,10 @@ private static function evalLua($lua, $keys, $args) { } return $res; } + + //Save how soon a given bucket will be at full capacity + private static function addFullCapacityTimeout($prefix, $rate, $capacity) { + $full_capacity_ts = $capacity / $rate; + self::$redis->setex($prefix . ".fc", (int) $full_capacity_ts, 1); + } } From 1758fb9e0521c008db616ca106888a6a795fbd1d Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Thu, 8 Jun 2023 10:42:08 -0400 Subject: [PATCH 3/4] removed unecessary header setting --- controllers/MappingsController.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/controllers/MappingsController.php b/controllers/MappingsController.php index b13d501a..d7462e3c 100644 --- a/controllers/MappingsController.php +++ b/controllers/MappingsController.php @@ -34,8 +34,6 @@ public function mappings() { $locale = !empty($_GET['locale']) ? \Zotero\Schema::resolveLocale($_GET['locale']) : 'en-US'; - unset($this->queryParams['format']); - header("Content-Type: application/json"); if ($this->subset == 'itemTypeFields') { if (empty($_GET['itemType'])) { From 3cbf2465593d2ad5785d752b32e1f1c853acb408 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Sat, 10 Jun 2023 20:19:57 -0400 Subject: [PATCH 4/4] default format for mapping action v1/v2 as json --- controllers/MappingsController.php | 3 --- model/API.inc.php | 6 ++++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/controllers/MappingsController.php b/controllers/MappingsController.php index d7462e3c..1637f7eb 100644 --- a/controllers/MappingsController.php +++ b/controllers/MappingsController.php @@ -143,9 +143,6 @@ public function newItem() { $this->e400("'itemType' not provided"); } - unset($this->queryParams['format']); - header("Content-Type: application/json"); - $itemType = $_GET['itemType']; if ($itemType == 'attachment') { if (empty($_GET['linkMode'])) { diff --git a/model/API.inc.php b/model/API.inc.php index 863104a3..c20f34e6 100644 --- a/model/API.inc.php +++ b/model/API.inc.php @@ -54,7 +54,8 @@ class Zotero_API { 'action' => [ 'default' => 'atom', 'fulltext' => 'versions', - 'itemContent' => 'json' + 'itemContent' => 'json', + 'newItem' => 'json' ] ], 2 => [ @@ -63,7 +64,8 @@ class Zotero_API { 'fulltext' => 'versions', 'itemContent' => 'json', 'deleted' => 'json', - 'settings' => 'json' + 'settings' => 'json', + 'newItem' => 'json' ] ] ]