From cf0a3399970eb00621e822923f17d3d52845e0a6 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Sun, 14 Jan 2018 11:47:00 +0100
Subject: [PATCH] Make OC\Security\RateLimiting strict
* Add return types
* Add scalar argument types
* Made strict
* Cleaned up phpstorm inspections
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
---
.../RateLimiting/Backend/IBackend.php | 13 +++++----
.../RateLimiting/Backend/MemoryCache.php | 28 +++++++++++--------
.../Exception/RateLimitExceededException.php | 1 +
lib/private/Security/RateLimiting/Limiter.php | 27 +++++++++---------
.../RateLimiting/Backend/MemoryCacheTest.php | 4 +--
5 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/lib/private/Security/RateLimiting/Backend/IBackend.php b/lib/private/Security/RateLimiting/Backend/IBackend.php
index b20d27af42b..88c10fbbc8d 100644
--- a/lib/private/Security/RateLimiting/Backend/IBackend.php
+++ b/lib/private/Security/RateLimiting/Backend/IBackend.php
@@ -1,4 +1,5 @@
<?php
+declare(strict_types=1);
/**
* @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
*
@@ -39,9 +40,9 @@ interface IBackend {
* @param int $seconds Seconds to look back at
* @return int
*/
- public function getAttempts($methodIdentifier,
- $userIdentifier,
- $seconds);
+ public function getAttempts(string $methodIdentifier,
+ string $userIdentifier,
+ int $seconds): int;
/**
* Registers an attempt
@@ -50,7 +51,7 @@ interface IBackend {
* @param string $userIdentifier Identifier for the user
* @param int $period Period in seconds how long this attempt should be stored
*/
- public function registerAttempt($methodIdentifier,
- $userIdentifier,
- $period);
+ public function registerAttempt(string $methodIdentifier,
+ string $userIdentifier,
+ int $period);
}
diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCache.php b/lib/private/Security/RateLimiting/Backend/MemoryCache.php
index 700fa624ed4..a8fb7b87d10 100644
--- a/lib/private/Security/RateLimiting/Backend/MemoryCache.php
+++ b/lib/private/Security/RateLimiting/Backend/MemoryCache.php
@@ -1,4 +1,5 @@
<?php
+declare(strict_types=1);
/**
* @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
*
@@ -54,8 +55,8 @@ class MemoryCache implements IBackend {
* @param string $userIdentifier
* @return string
*/
- private function hash($methodIdentifier,
- $userIdentifier) {
+ private function hash(string $methodIdentifier,
+ string $userIdentifier): string {
return hash('sha512', $methodIdentifier . $userIdentifier);
}
@@ -63,9 +64,14 @@ class MemoryCache implements IBackend {
* @param string $identifier
* @return array
*/
- private function getExistingAttempts($identifier) {
- $cachedAttempts = json_decode($this->cache->get($identifier), true);
- if(is_array($cachedAttempts)) {
+ private function getExistingAttempts(string $identifier): array {
+ $cachedAttempts = $this->cache->get($identifier);
+ if ($cachedAttempts === null) {
+ return [];
+ }
+
+ $cachedAttempts = json_decode($cachedAttempts, true);
+ if(\is_array($cachedAttempts)) {
return $cachedAttempts;
}
@@ -75,9 +81,9 @@ class MemoryCache implements IBackend {
/**
* {@inheritDoc}
*/
- public function getAttempts($methodIdentifier,
- $userIdentifier,
- $seconds) {
+ public function getAttempts(string $methodIdentifier,
+ string $userIdentifier,
+ int $seconds): int {
$identifier = $this->hash($methodIdentifier, $userIdentifier);
$existingAttempts = $this->getExistingAttempts($identifier);
@@ -96,9 +102,9 @@ class MemoryCache implements IBackend {
/**
* {@inheritDoc}
*/
- public function registerAttempt($methodIdentifier,
- $userIdentifier,
- $period) {
+ public function registerAttempt(string $methodIdentifier,
+ string $userIdentifier,
+ int $period) {
$identifier = $this->hash($methodIdentifier, $userIdentifier);
$existingAttempts = $this->getExistingAttempts($identifier);
$currentTime = $this->timeFactory->getTime();
diff --git a/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php b/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php
index ffe9b534fed..ae4fa1d6c26 100644
--- a/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php
+++ b/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php
@@ -1,4 +1,5 @@
<?php
+declare(strict_types=1);
/**
* @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
*
diff --git a/lib/private/Security/RateLimiting/Limiter.php b/lib/private/Security/RateLimiting/Limiter.php
index 6a4176a0d50..5267497f86f 100644
--- a/lib/private/Security/RateLimiting/Limiter.php
+++ b/lib/private/Security/RateLimiting/Limiter.php
@@ -1,4 +1,5 @@
<?php
+declare(strict_types=1);
/**
* @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
*
@@ -58,12 +59,12 @@ class Limiter {
* @param int $limit
* @throws RateLimitExceededException
*/
- private function register($methodIdentifier,
- $userIdentifier,
- $period,
- $limit) {
- $existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier, (int)$period);
- if ($existingAttempts >= (int)$limit) {
+ private function register(string $methodIdentifier,
+ string $userIdentifier,
+ int $period,
+ int $limit) {
+ $existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier, $period);
+ if ($existingAttempts >= $limit) {
throw new RateLimitExceededException();
}
@@ -79,10 +80,10 @@ class Limiter {
* @param string $ip
* @throws RateLimitExceededException
*/
- public function registerAnonRequest($identifier,
- $anonLimit,
- $anonPeriod,
- $ip) {
+ public function registerAnonRequest(string $identifier,
+ int $anonLimit,
+ int $anonPeriod,
+ string $ip) {
$ipSubnet = (new IpAddress($ip))->getSubnet();
$anonHashIdentifier = hash('sha512', 'anon::' . $identifier . $ipSubnet);
@@ -98,9 +99,9 @@ class Limiter {
* @param IUser $user
* @throws RateLimitExceededException
*/
- public function registerUserRequest($identifier,
- $userLimit,
- $userPeriod,
+ public function registerUserRequest(string $identifier,
+ int $userLimit,
+ int $userPeriod,
IUser $user) {
$userHashIdentifier = hash('sha512', 'user::' . $identifier . $user->getUID());
$this->register($identifier, $userHashIdentifier, $userPeriod, $userLimit);
diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php
index bacd2b7bf6f..098c40ba0e8 100644
--- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php
+++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php
@@ -61,7 +61,7 @@ class MemoryCacheTest extends TestCase {
->expects($this->once())
->method('get')
->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b')
- ->willReturn(false);
+ ->willReturn(null);
$this->assertSame(0, $this->memoryCache->getAttempts('Method', 'User', 123));
}
@@ -97,7 +97,7 @@ class MemoryCacheTest extends TestCase {
->expects($this->once())
->method('get')
->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b')
- ->willReturn(false);
+ ->willReturn(null);
$this->cache
->expects($this->once())
->method('set')
--
GitLab