diff --git a/phpBB/config/default/container/services_content.yml b/phpBB/config/default/container/services_content.yml index d290974ff7..53592a4d0c 100644 --- a/phpBB/config/default/container/services_content.yml +++ b/phpBB/config/default/container/services_content.yml @@ -65,6 +65,13 @@ services: - '@controller.helper' - '@event_dispatcher' + posting.lock: + class: phpbb\lock\posting + shared: false + arguments: + - '@cache.driver' + - '@config' + viewonline_helper: class: phpbb\viewonline_helper arguments: diff --git a/phpBB/phpbb/lock/posting.php b/phpBB/phpbb/lock/posting.php new file mode 100644 index 0000000000..569cad9971 --- /dev/null +++ b/phpBB/phpbb/lock/posting.php @@ -0,0 +1,77 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\lock; + +use phpbb\cache\driver\driver_interface as cache_interface; +use phpbb\config\config; + +class posting +{ + /** @var cache_interface */ + private $cache; + + /** @var config */ + private $config; + + /** @var string */ + private $lock_name = ''; + + /** + * Constructor for posting lock + * + * @param cache_interface $cache + * @param config $config + */ + public function __construct(cache_interface $cache, config $config) + { + $this->cache = $cache; + $this->config = $config; + } + + /** + * Set lock name + * + * @param int $creation_time Creation time of form, must be checked already + * @param string $form_token Form token used for form, must be checked already + * + * @return void + */ + private function set_lock_name(int $creation_time, string $form_token): void + { + $this->lock_name = sha1(((string) $creation_time) . $form_token) . '_posting_lock'; + } + + /** + * Acquire lock for current posting form submission + * + * @param int $creation_time Creation time of form, must be checked already + * @param string $form_token Form token used for form, must be checked already + * + * @return bool True if lock could be acquired, false if not + */ + public function acquire(int $creation_time, string $form_token): bool + { + $this->set_lock_name($creation_time, $form_token); + + // Lock is held for session, cannot acquire it unless special flag for testing is set + if ($this->cache->_exists($this->lock_name) && !$this->config->offsetExists('ci_tests_no_lock_posting')) + { + return false; + } + + $this->cache->put($this->lock_name, true, $this->config['flood_interval']); + + return true; + } +} diff --git a/phpBB/posting.php b/phpBB/posting.php index 79ba5585e9..515e9e35ad 100644 --- a/phpBB/posting.php +++ b/phpBB/posting.php @@ -1427,7 +1427,14 @@ if ($submit || $preview || $refresh) // Store message, sync counters if (!count($error) && $submit) { - if ($submit) + /** @var \phpbb\lock\posting $posting_lock */ + $posting_lock = $phpbb_container->get('posting.lock'); + + // Get creation time and form token, must be already checked at this point + $creation_time = abs($request->variable('creation_time', 0)); + $form_token = $request->variable('form_token', ''); + + if ($posting_lock->acquire($creation_time, $form_token)) { // Lock/Unlock Topic $change_topic_status = $post_data['topic_status']; @@ -1618,6 +1625,11 @@ if ($submit || $preview || $refresh) redirect($redirect_url); } + else + { + // Posting was already locked before, hence form submission was already attempted once and is now invalid + $error[] = $language->lang('FORM_INVALID'); + } } } diff --git a/phpBB/styles/prosilver/template/ajax.js b/phpBB/styles/prosilver/template/ajax.js index 5b1e08b4de..421c43f767 100644 --- a/phpBB/styles/prosilver/template/ajax.js +++ b/phpBB/styles/prosilver/template/ajax.js @@ -337,6 +337,29 @@ $('[data-ajax]').each(function() { } }); +// Prevent accidental double submission of form +$('[data-prevent-flood] input[type=submit]').click(function(event) { + const $submitButton = $(this); // Store the button element + const $form = $submitButton.closest('form'); + + // Always add the disabled class for visual feedback + $submitButton.addClass('disabled'); + + // Submit form if it hasn't been submitted yet + if (!$form.prop('data-form-submitted')) { + $form.prop('data-form-submitted', true); + + return; + } + + // Prevent default submission for subsequent clicks within 5 seconds + event.preventDefault(); + + setTimeout(() => { + $form.prop('removeProp', 'data-form-submitted'); + $submitButton.removeClass('disabled'); // Re-enable after 5 seconds + }, 5000); +}); /** * This simply appends #preview to the action of the diff --git a/phpBB/styles/prosilver/template/posting_editor.html b/phpBB/styles/prosilver/template/posting_editor.html index eebc96623e..02390bf619 100644 --- a/phpBB/styles/prosilver/template/posting_editor.html +++ b/phpBB/styles/prosilver/template/posting_editor.html @@ -97,7 +97,7 @@
-
+
{S_HIDDEN_ADDRESS_FIELD} {S_HIDDEN_FIELDS} diff --git a/tests/lock/posting_test.php b/tests/lock/posting_test.php new file mode 100644 index 0000000000..75716dd2ba --- /dev/null +++ b/tests/lock/posting_test.php @@ -0,0 +1,56 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use phpbb\cache\driver\file as file_cache; +use phpbb\config\config; +use phpbb\lock\posting; + +class phpbb_lock_posting_test extends phpbb_test_case +{ + /** @var \phpbb\cache\driver\file */ + protected $cache; + + /** @var config */ + protected $config; + + /** @var posting */ + protected $lock; + + public function setUp(): void + { + $this->cache = new file_cache(__DIR__ . '/../tmp/'); + $this->cache->purge(); // ensure cache is clean + $this->config = new config([ + 'flood_interval' => 15, + ]); + $this->lock = new posting($this->cache, $this->config); + } + + public function test_lock_acquire() + { + $this->assertTrue($this->lock->acquire(100, 'foo')); + $this->assertFalse($this->lock->acquire(100, 'foo')); + + $this->assertTrue($this->cache->_exists(sha1('100foo') . '_posting_lock')); + $this->assertFalse($this->lock->acquire(100, 'foo')); + $this->cache->put(sha1('100foo') . '_posting_lock', 'foo', -30); + + $this->assertTrue($this->lock->acquire(100, 'foo')); + $this->assertTrue($this->cache->_exists(sha1('100foo') . '_posting_lock')); + $this->config->offsetSet('ci_tests_no_lock_posting', true); + $this->assertTrue($this->lock->acquire(100, 'foo')); + $this->assertTrue($this->cache->_exists(sha1('100foo') . '_posting_lock')); + // Multiple acquires possible due to special ci test flag + $this->assertTrue($this->lock->acquire(100, 'foo')); + } +} diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index c0dd68ab14..02d5911459 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -106,6 +106,14 @@ class phpbb_functional_test_case extends phpbb_test_case $db = $this->get_db(); + // Special flag for testing without possibility to run into lock scenario. + // Unset entry and add it back if lock behavior for posting should be tested. + // Unset ci_tests_no_lock_posting from config + $db->sql_return_on_error(true); + $sql = 'INSERT INTO ' . CONFIG_TABLE . " (config_name, config_value) VALUES ('ci_tests_no_lock_posting', '1')"; + $this->db->sql_query($sql); + $db->sql_return_on_error(false); + foreach (static::setup_extensions() as $extension) { $this->purge_cache(); @@ -130,6 +138,11 @@ class phpbb_functional_test_case extends phpbb_test_case if ($this->db instanceof \phpbb\db\driver\driver_interface) { + // Unset ci_tests_no_lock_posting from config + $sql = 'DELETE FROM ' . CONFIG_TABLE . " + WHERE config_name = 'ci_tests_no_lock_posting'"; + $this->db->sql_query($sql); + // Close the database connections again this test $this->db->sql_close(); } @@ -212,6 +225,9 @@ class phpbb_functional_test_case extends phpbb_test_case ]; } + /** + * @return \phpbb\db\driver\driver_interface + */ protected function get_db() { global $phpbb_root_path, $phpEx; diff --git a/tests/test_framework/phpbb_test_case_helpers.php b/tests/test_framework/phpbb_test_case_helpers.php index bed719c6bb..466be97d4c 100644 --- a/tests/test_framework/phpbb_test_case_helpers.php +++ b/tests/test_framework/phpbb_test_case_helpers.php @@ -136,7 +136,6 @@ class phpbb_test_case_helpers { $config = array(); - if (extension_loaded('sqlite3')) { $config = array_merge($config, array(