From 9a03b3aab6033b52492296730c30849f96f430bf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 24 Jan 2015 12:49:58 +0100 Subject: [PATCH 1/9] [ticket/13522] Remove empty answers from possible answers in Q&A PHPBB3-13522 --- phpBB/phpbb/captcha/plugins/qa.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index a7ba994cc3..13ff3d7f91 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -827,13 +827,22 @@ class qa function acp_get_question_input() { $answers = utf8_normalize_nfc(request_var('answers', '', true)); + + // Convert answers into array and filter if answers are set + if (strlen($answers)) + { + $answers = array_filter(explode("\n", $answers), function ($value) { + return trim($value) !== ''; + }); + } + + $question = array( 'question_text' => request_var('question_text', '', true), 'strict' => request_var('strict', false), 'lang_iso' => request_var('lang_iso', ''), - 'answers' => (strlen($answers)) ? explode("\n", $answers) : '', + 'answers' => $answers, ); - return $question; } From 874f3584e2713cd332215b159b740e53330a900c Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 24 Jan 2015 13:40:42 +0100 Subject: [PATCH 2/9] [ticket/13522] Remove unused variables and unexpected returns PHPBB3-13522 --- phpBB/phpbb/captcha/plugins/qa.php | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index 13ff3d7f91..12da64c750 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -125,7 +125,7 @@ class qa */ public function is_available() { - global $config, $db, $phpbb_root_path, $phpEx, $user; + global $config, $db, $user; // load language file for pretty display in the ACP dropdown $user->add_lang('captcha_qa'); @@ -263,7 +263,7 @@ class qa */ function garbage_collect($type = 0) { - global $db, $config; + global $db; $sql = 'SELECT c.confirm_id FROM ' . $this->table_qa_confirm . ' c @@ -310,8 +310,6 @@ class qa $db_tool = new \phpbb\db\tools($db); - $tables = array($this->table_captcha_questions, $this->table_captcha_answers, $this->table_qa_confirm); - $schemas = array( $this->table_captcha_questions => array ( 'COLUMNS' => array( @@ -366,7 +364,7 @@ class qa */ function validate() { - global $config, $db, $user; + global $user; $error = ''; @@ -414,7 +412,7 @@ class qa if (!sizeof($this->question_ids)) { - return false; + return; } $this->confirm_id = md5(unique_id($user->ip)); $this->question = (int) array_rand($this->question_ids); @@ -440,7 +438,7 @@ class qa if (!sizeof($this->question_ids)) { - return false; + return; } $this->question = (int) array_rand($this->question_ids); @@ -611,8 +609,8 @@ class qa */ function acp_page($id, &$module) { - global $db, $user, $auth, $template; - global $config, $phpbb_root_path, $phpbb_admin_path, $phpEx; + global $user, $template; + global $config; $user->add_lang('acp/board'); $user->add_lang('captcha_qa'); @@ -674,7 +672,6 @@ class qa else { // okay, show the editor - $error = false; $input_question = request_var('question_text', '', true); $input_answers = request_var('answers', '', true); $input_lang = request_var('lang_iso', '', true); @@ -819,6 +816,8 @@ class qa return $question; } + + return false; } /** From 12fe882741db91a155620d5870c6127798125720 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 24 Jan 2015 13:53:22 +0100 Subject: [PATCH 3/9] [ticket/13522] Trim array elements before processing further PHPBB3-13522 --- phpBB/phpbb/captcha/plugins/qa.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index 12da64c750..824e6db3b0 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -830,12 +830,11 @@ class qa // Convert answers into array and filter if answers are set if (strlen($answers)) { - $answers = array_filter(explode("\n", $answers), function ($value) { - return trim($value) !== ''; + $answers = array_filter(array_map('trim', explode("\n", $answers)), function ($value) { + return $value !== ''; }); } - $question = array( 'question_text' => request_var('question_text', '', true), 'strict' => request_var('strict', false), From d15e5372caf1d0811e1f2bc87d5deddba171cb09 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 25 Jan 2015 22:52:40 +0100 Subject: [PATCH 4/9] [ticket/13522] Use acp_get_question_input() for retrieving input data PHPBB3-13522 --- phpBB/phpbb/captcha/plugins/qa.php | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index 824e6db3b0..3a2652d02c 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -672,10 +672,7 @@ class qa else { // okay, show the editor - $input_question = request_var('question_text', '', true); - $input_answers = request_var('answers', '', true); - $input_lang = request_var('lang_iso', '', true); - $input_strict = request_var('strict', false); + $question_input = $this->acp_get_question_input(); $langs = $this->get_languages(); foreach ($langs as $lang => $entry) @@ -694,12 +691,12 @@ class qa { if ($question = $this->acp_get_question_data($question_id)) { - $answers = (isset($input_answers[$lang])) ? $input_answers[$lang] : implode("\n", $question['answers']); + $answers = (isset($question_input['answers'][$lang])) ? $question_input['answers'][$lang] : implode("\n", $question['answers']); $template->assign_vars(array( - 'QUESTION_TEXT' => ($input_question) ? $input_question : $question['question_text'], - 'LANG_ISO' => ($input_lang) ? $input_lang : $question['lang_iso'], - 'STRICT' => (isset($_REQUEST['strict'])) ? $input_strict : $question['strict'], + 'QUESTION_TEXT' => ($question_input['question_text']) ? $question_input['question_text'] : $question['question_text'], + 'LANG_ISO' => ($question_input['lang_iso']) ? $question_input['lang_iso'] : $question['lang_iso'], + 'STRICT' => (isset($_REQUEST['strict'])) ? $question_input['strict'] : $question['strict'], 'ANSWERS' => $answers, )); } @@ -711,10 +708,10 @@ class qa else { $template->assign_vars(array( - 'QUESTION_TEXT' => $input_question, - 'LANG_ISO' => $input_lang, - 'STRICT' => $input_strict, - 'ANSWERS' => $input_answers, + 'QUESTION_TEXT' => $question_input['question_text'], + 'LANG_ISO' => $question_input['lang_iso'], + 'STRICT' => $question_input['strict'], + 'ANSWERS' => $question_input['answers'], )); } From 5bfcc7e70780438b339731273e7793a74b24e59b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 29 Jan 2015 20:57:21 +0100 Subject: [PATCH 5/9] [ticket/13522] Add test file for Q&A captcha PHPBB3-13522 --- tests/captcha/qa_test.php | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/captcha/qa_test.php diff --git a/tests/captcha/qa_test.php b/tests/captcha/qa_test.php new file mode 100644 index 0000000000..51bc09d309 --- /dev/null +++ b/tests/captcha/qa_test.php @@ -0,0 +1,46 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class phpbb_qa_test extends \phpbb_database_test_case +{ + protected $request; + + /** @var \phpbb\captcha\plugins\qa */ + protected $qa; + + public function getDataSet() + { + return $this->createXMLDataSet(dirname(__FILE__) . '/../fixtures/empty.xml'); + } + + public function setUp() + { + global $db; + + $db = $this->new_dbal(); + + parent::setUp(); + + $this->request = new \phpbb_mock_request(); + $this->qa = new \phpbb\captcha\plugins\qa($this->request, 'phpbb_captcha_questions', 'phpbb_captcha_answers', 'phpbb_qa_confirm'); + } + + public function test_is_installed() + { + $this->assertFalse($this->qa->is_installed()); + + $this->qa->install(); + + $this->assertTrue($this->qa->is_installed()); + } +} \ No newline at end of file From 4167be1e2c5dc5ea9fcc7ed6b39f54cc68fef5b2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 31 Jan 2015 12:19:03 +0100 Subject: [PATCH 6/9] [ticket/13522] Add tests for acp_get_question_input and set/get name PHPBB3-13522 --- tests/captcha/qa_test.php | 47 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/captcha/qa_test.php b/tests/captcha/qa_test.php index 51bc09d309..fdf0c799f3 100644 --- a/tests/captcha/qa_test.php +++ b/tests/captcha/qa_test.php @@ -43,4 +43,49 @@ class phpbb_qa_test extends \phpbb_database_test_case $this->assertTrue($this->qa->is_installed()); } -} \ No newline at end of file + + public function test_set_get_name() + { + $this->assertNull($this->qa->get_service_name()); + $this->qa->set_name('foobar'); + $this->assertSame('foobar', $this->qa->get_service_name()); + } + + public function data_acp_get_question_input() + { + return array( + array("foobar\ntest\nyes", array( + 'question_text' => '', + 'strict' => false, + 'lang_iso' => '', + 'answers' => array('foobar', 'test', 'yes') + )), + array("foobar\ntest\n \nyes", array( + 'question_text' => '', + 'strict' => false, + 'lang_iso' => '', + 'answers' => array( + 0 => 'foobar', + 1 => 'test', + 3 => 'yes', + ) + )), + array('', array( + 'question_text' => '', + 'strict' => false, + 'lang_iso' => '', + 'answers' => '', + )), + ); + } + + /** + * @dataProvider data_acp_get_question_input + */ + public function test_acp_get_question_input($value, $expected) + { + $this->request->overwrite('answers', $value); + + $this->assertEquals($expected, $this->qa->acp_get_question_input()); + } +} From 8e737e483e2fbd20712bbcd1b3bb96d623ac0f24 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 31 Jan 2015 12:39:58 +0100 Subject: [PATCH 7/9] [ticket/13522] Remove unneeded variables and correctly output input on error PHPBB3-13522 --- phpBB/phpbb/captcha/plugins/qa.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index 3a2652d02c..04052b3406 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -691,13 +691,11 @@ class qa { if ($question = $this->acp_get_question_data($question_id)) { - $answers = (isset($question_input['answers'][$lang])) ? $question_input['answers'][$lang] : implode("\n", $question['answers']); - $template->assign_vars(array( 'QUESTION_TEXT' => ($question_input['question_text']) ? $question_input['question_text'] : $question['question_text'], 'LANG_ISO' => ($question_input['lang_iso']) ? $question_input['lang_iso'] : $question['lang_iso'], 'STRICT' => (isset($_REQUEST['strict'])) ? $question_input['strict'] : $question['strict'], - 'ANSWERS' => $answers, + 'ANSWERS' => implode("\n", $question['answers']), )); } else @@ -711,15 +709,13 @@ class qa 'QUESTION_TEXT' => $question_input['question_text'], 'LANG_ISO' => $question_input['lang_iso'], 'STRICT' => $question_input['strict'], - 'ANSWERS' => $question_input['answers'], + 'ANSWERS' => (is_array($question_input['answers'])) ? implode("\n", $question_input['answers']) : '', )); } if ($submit && check_form_key($form_key)) { - $data = $this->acp_get_question_input(); - - if (!$this->validate_input($data)) + if (!$this->validate_input($question_input)) { $template->assign_vars(array( 'S_ERROR' => true, @@ -729,11 +725,11 @@ class qa { if ($question_id) { - $this->acp_update_question($data, $question_id); + $this->acp_update_question($question_input, $question_id); } else { - $this->acp_add_question($data); + $this->acp_add_question($question_input); } add_log('admin', 'LOG_CONFIG_VISUAL'); From 94b72cfbdb3eae2822ba0fddb88369373ebd5b85 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 2 Feb 2015 16:19:07 +0100 Subject: [PATCH 8/9] [ticket/13522] Add captcha to test file class name PHPBB3-13522 --- tests/captcha/qa_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/captcha/qa_test.php b/tests/captcha/qa_test.php index fdf0c799f3..8d0d77d0e3 100644 --- a/tests/captcha/qa_test.php +++ b/tests/captcha/qa_test.php @@ -11,7 +11,7 @@ * */ -class phpbb_qa_test extends \phpbb_database_test_case +class phpbb_captcha_qa_test extends \phpbb_database_test_case { protected $request; From c63bc6ee93f143fbc97ddb826fb310962a8320c7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 21 Feb 2015 17:53:25 +0100 Subject: [PATCH 9/9] [ticket/13522] Fix QA tests for request_var() use PHPBB3-13522 --- tests/captcha/qa_test.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/captcha/qa_test.php b/tests/captcha/qa_test.php index 8d0d77d0e3..1f2f9f3070 100644 --- a/tests/captcha/qa_test.php +++ b/tests/captcha/qa_test.php @@ -11,6 +11,8 @@ * */ +require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; + class phpbb_captcha_qa_test extends \phpbb_database_test_case { protected $request; @@ -32,7 +34,8 @@ class phpbb_captcha_qa_test extends \phpbb_database_test_case parent::setUp(); $this->request = new \phpbb_mock_request(); - $this->qa = new \phpbb\captcha\plugins\qa($this->request, 'phpbb_captcha_questions', 'phpbb_captcha_answers', 'phpbb_qa_confirm'); + request_var(false, false, false, false, $this->request); + $this->qa = new \phpbb\captcha\plugins\qa('phpbb_captcha_questions', 'phpbb_captcha_answers', 'phpbb_qa_confirm'); } public function test_is_installed()