From 9063556a57e00b273a3627849388995b23274e68 Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Tue, 14 Mar 2017 00:57:28 +0530 Subject: [PATCH 1/7] [ticket/11515] Extra check after acquiring locks. Add additional check to flock.php and db.php to ensure lock aquiring. PHPBB3-11515 --- phpBB/phpbb/config/config.php | 21 +++++++++++++++++++++ phpBB/phpbb/lock/db.php | 11 ++++++++++- phpBB/phpbb/lock/flock.php | 7 ++++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/config/config.php b/phpBB/phpbb/config/config.php index aaad333006..036ae32cef 100644 --- a/phpBB/phpbb/config/config.php +++ b/phpBB/phpbb/config/config.php @@ -147,6 +147,27 @@ class config implements \ArrayAccess, \IteratorAggregate, \Countable return false; } + /** + * Checks configuration option's value only if the new_value matches the + * current configuration value and the configuration value does exist.Called + *only after set_atomic has been called. + * + * @param string $key The configuration option's name + * @param string $old_value Current configuration value + * @param string $new_value New configuration value + * @throws \phpbb\exception\http_exception when configuration value is set and not equal to *new_value. + * @return bool True if the value was changed, false otherwise. + */ + public function ensure_lock($key, $new_value) + { + if(isset($this->config[$key]) && $this->config[$key] == $new_value) + { + return true; + } else { + throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); + } + } + /** * Increments an integer configuration value. * diff --git a/phpBB/phpbb/lock/db.php b/phpBB/phpbb/lock/db.php index 85ba9a7aa3..7765619422 100644 --- a/phpBB/phpbb/lock/db.php +++ b/phpBB/phpbb/lock/db.php @@ -110,7 +110,16 @@ class db // process we failed to acquire the lock. $this->locked = $this->config->set_atomic($this->config_name, $lock_value, $this->unique_id, false); - return $this->locked; + if ($this->locked == true) + { + if ($this->config->ensure_lock($this->config_name, $this->unique_id)) + { + return true; + } + } else { + + return $this->locked; + } } /** diff --git a/phpBB/phpbb/lock/flock.php b/phpBB/phpbb/lock/flock.php index df88e1490a..89ff7cbe48 100644 --- a/phpBB/phpbb/lock/flock.php +++ b/phpBB/phpbb/lock/flock.php @@ -101,7 +101,12 @@ class flock if ($this->lock_fp) { - @flock($this->lock_fp, LOCK_EX); + if (@flock($this->lock_fp, LOCK_EX)) + { + return (bool) $this->lock_fp; + } else { + throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); + } } return (bool) $this->lock_fp; From 1ba32e1b7ad99adaab5881ca78aee949b1033910 Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Tue, 14 Mar 2017 02:16:18 +0530 Subject: [PATCH 2/7] [ticket/11515] Change If...else statement struct Modifications in if-else structure. PHPBB3-11515 --- phpBB/phpbb/config/config.php | 4 +++- phpBB/phpbb/lock/db.php | 4 +++- phpBB/phpbb/lock/flock.php | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/config/config.php b/phpBB/phpbb/config/config.php index 036ae32cef..239e4c2e55 100644 --- a/phpBB/phpbb/config/config.php +++ b/phpBB/phpbb/config/config.php @@ -163,7 +163,9 @@ class config implements \ArrayAccess, \IteratorAggregate, \Countable if(isset($this->config[$key]) && $this->config[$key] == $new_value) { return true; - } else { + } + else + { throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); } } diff --git a/phpBB/phpbb/lock/db.php b/phpBB/phpbb/lock/db.php index 7765619422..d9ad4d3b97 100644 --- a/phpBB/phpbb/lock/db.php +++ b/phpBB/phpbb/lock/db.php @@ -116,7 +116,9 @@ class db { return true; } - } else { + } + else + { return $this->locked; } diff --git a/phpBB/phpbb/lock/flock.php b/phpBB/phpbb/lock/flock.php index 89ff7cbe48..b496ed845e 100644 --- a/phpBB/phpbb/lock/flock.php +++ b/phpBB/phpbb/lock/flock.php @@ -104,7 +104,9 @@ class flock if (@flock($this->lock_fp, LOCK_EX)) { return (bool) $this->lock_fp; - } else { + } + else + { throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); } } From 4f71a75df1653c308e5f6089772abb1503f861a0 Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Tue, 14 Mar 2017 02:27:18 +0530 Subject: [PATCH 3/7] [ticket/11515] Space between if and braces Exactly one space between if and opening brace. PHPBB3-11515 --- phpBB/phpbb/config/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/config/config.php b/phpBB/phpbb/config/config.php index 239e4c2e55..0a867342f5 100644 --- a/phpBB/phpbb/config/config.php +++ b/phpBB/phpbb/config/config.php @@ -160,7 +160,7 @@ class config implements \ArrayAccess, \IteratorAggregate, \Countable */ public function ensure_lock($key, $new_value) { - if(isset($this->config[$key]) && $this->config[$key] == $new_value) + if (isset($this->config[$key]) && $this->config[$key] == $new_value) { return true; } From b98acb9409bcc3501c8244e7208bb45d29b8382d Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Tue, 14 Mar 2017 17:19:41 +0530 Subject: [PATCH 4/7] [ticket/11515] Refactoring the patch. Removing else conditions. PHPBB3-11515 --- phpBB/phpbb/config/config.php | 5 +---- phpBB/phpbb/lock/db.php | 6 +----- phpBB/phpbb/lock/flock.php | 5 +---- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/phpBB/phpbb/config/config.php b/phpBB/phpbb/config/config.php index 0a867342f5..f4b670e834 100644 --- a/phpBB/phpbb/config/config.php +++ b/phpBB/phpbb/config/config.php @@ -164,10 +164,7 @@ class config implements \ArrayAccess, \IteratorAggregate, \Countable { return true; } - else - { - throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); - } + throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); } /** diff --git a/phpBB/phpbb/lock/db.php b/phpBB/phpbb/lock/db.php index d9ad4d3b97..eea919f8f7 100644 --- a/phpBB/phpbb/lock/db.php +++ b/phpBB/phpbb/lock/db.php @@ -117,11 +117,7 @@ class db return true; } } - else - { - - return $this->locked; - } + return $this->locked; } /** diff --git a/phpBB/phpbb/lock/flock.php b/phpBB/phpbb/lock/flock.php index b496ed845e..fa4cbe3690 100644 --- a/phpBB/phpbb/lock/flock.php +++ b/phpBB/phpbb/lock/flock.php @@ -105,10 +105,7 @@ class flock { return (bool) $this->lock_fp; } - else - { - throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); - } + throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); } return (bool) $this->lock_fp; From f18743eb503f5f728e05f7ded96f9db4171739aa Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Sat, 18 Mar 2017 22:37:16 +0530 Subject: [PATCH 5/7] [ticket/11515] Refactoring changes. Refactoring the code as suggested. PHPBB3-11515 --- phpBB/phpbb/config/config.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/config/config.php b/phpBB/phpbb/config/config.php index f4b670e834..c619cae2fd 100644 --- a/phpBB/phpbb/config/config.php +++ b/phpBB/phpbb/config/config.php @@ -150,12 +150,11 @@ class config implements \ArrayAccess, \IteratorAggregate, \Countable /** * Checks configuration option's value only if the new_value matches the * current configuration value and the configuration value does exist.Called - *only after set_atomic has been called. + * only after set_atomic has been called. * * @param string $key The configuration option's name - * @param string $old_value Current configuration value * @param string $new_value New configuration value - * @throws \phpbb\exception\http_exception when configuration value is set and not equal to *new_value. + * @throws \phpbb\exception\http_exception when config value is set and not equal to new_value. * @return bool True if the value was changed, false otherwise. */ public function ensure_lock($key, $new_value) From 8897dbf9fe64c6f5988089b28d793f7bbb1a9eeb Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Sun, 14 May 2017 11:47:48 +0530 Subject: [PATCH 6/7] [ticket/11515] Inverted Logic Inverted the logic to raise exception inside if. PHPBB3-11515 --- phpBB/phpbb/lock/flock.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/lock/flock.php b/phpBB/phpbb/lock/flock.php index fa4cbe3690..6c41ceed26 100644 --- a/phpBB/phpbb/lock/flock.php +++ b/phpBB/phpbb/lock/flock.php @@ -101,11 +101,11 @@ class flock if ($this->lock_fp) { - if (@flock($this->lock_fp, LOCK_EX)) + if (!@flock($this->lock_fp, LOCK_EX)) { - return (bool) $this->lock_fp; + throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); } - throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); + return (bool) $this->lock_fp; } return (bool) $this->lock_fp; From fa108176e55ae940ce67cc7094bc2a64e1ae8cd0 Mon Sep 17 00:00:00 2001 From: Vishal Pandey Date: Sun, 14 May 2017 12:08:20 +0530 Subject: [PATCH 7/7] [ticket/11515] If condition changes Duplicate return statements removed. PHPBB3-11515 --- phpBB/phpbb/lock/flock.php | 1 - 1 file changed, 1 deletion(-) diff --git a/phpBB/phpbb/lock/flock.php b/phpBB/phpbb/lock/flock.php index 6c41ceed26..af051afb56 100644 --- a/phpBB/phpbb/lock/flock.php +++ b/phpBB/phpbb/lock/flock.php @@ -105,7 +105,6 @@ class flock { throw new \phpbb\exception\http_exception(500, 'Failure while aqcuiring locks.'); } - return (bool) $this->lock_fp; } return (bool) $this->lock_fp;