diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php index 37c49d7d72..c740ff7ddc 100644 --- a/phpBB/includes/acp/acp_groups.php +++ b/phpBB/includes/acp/acp_groups.php @@ -386,13 +386,21 @@ class acp_groups $error = array_merge($error, $phpbb_avatar_manager->localize_errors($user, $avatar_error)); } - // Validate the length of "Maximum number of allowed recipients per private message" setting. - // We use 16777215 as a maximum because it matches MySQL unsigned mediumint maximum value - // which is the lowest amongst DBMSes supported by phpBB3 - if ($max_recipients_error = validate_data($submit_ary, array('max_recipients' => array('num', false, 0, 16777215)))) + /* + * Validate the length of "Maximum number of allowed recipients per + * private message" setting. We use 16777215 as a maximum because it matches + * MySQL unsigned mediumint maximum value which is the lowest amongst DBMSes + * supported by phpBB3. Also validate the submitted colour value. + */ + $validation_checks = array( + 'max_recipients' => array('num', false, 0, 16777215), + 'colour' => array('hex_colour', true), + ); + + if ($validation_error = validate_data($submit_ary, $validation_checks)) { // Replace "error" string with its real, localised form - $error = array_merge($error, array_map(array(&$user, 'lang'), $max_recipients_error)); + $error = array_merge($error, array_map(array(&$user, 'lang'), $validation_error)); } if (!sizeof($error)) diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 599cb24f75..9a2ad4c25f 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -1330,22 +1330,12 @@ function validate_data($data, $val_ary) { $function = array_shift($validate); array_unshift($validate, $data[$var]); + $function_prefix = (function_exists('phpbb_validate_' . $function)) ? 'phpbb_validate_' : 'validate'; - if (function_exists('phpbb_validate_' . $function)) + if ($result = call_user_func_array($function_prefix . $function, $validate)) { - if ($result = call_user_func_array('phpbb_validate_' . $function, $validate)) - { - // Since errors are checked later for their language file existence, we need to make sure custom errors are not adjusted. - $error[] = (empty($user->lang[$result . '_' . strtoupper($var)])) ? $result : $result . '_' . strtoupper($var); - } - } - else - { - if ($result = call_user_func_array('validate_' . $function, $validate)) - { - // Since errors are checked later for their language file existence, we need to make sure custom errors are not adjusted. - $error[] = (empty($user->lang[$result . '_' . strtoupper($var)])) ? $result : $result . '_' . strtoupper($var); - } + // Since errors are checked later for their language file existence, we need to make sure custom errors are not adjusted. + $error[] = (empty($user->lang[$result . '_' . strtoupper($var)])) ? $result : $result . '_' . strtoupper($var); } } } @@ -2008,6 +1998,30 @@ function validate_jabber($jid) return false; } +/** +* Validate hex colour value +* +* @param string $colour The hex colour value +* @param bool $optional Whether the colour value is optional. True if an empty +* string will be accepted as correct input, false if not. +* @return bool|string Error message if colour value is incorrect, false if it +* fits the hex colour code +*/ +function phpbb_validate_hex_colour($colour, $optional = false) +{ + if (empty($colour)) + { + return (($optional) ? false : 'WRONG_DATA'); + } + + if (!preg_match('/^([0-9a-fA-F]{6}|[0-9a-fA-F]{3})$/', $colour)) + { + return 'WRONG_DATA'; + } + + return false; +} + /** * Verifies whether a style ID corresponds to an active style. * diff --git a/phpBB/includes/ucp/ucp_groups.php b/phpBB/includes/ucp/ucp_groups.php index a633ce448c..efc88e6e37 100644 --- a/phpBB/includes/ucp/ucp_groups.php +++ b/phpBB/includes/ucp/ucp_groups.php @@ -557,6 +557,13 @@ class ucp_groups $error[] = $user->lang['FORM_INVALID']; } + // Validate submitted colour value + if ($colour_error = validate_data($submit_ary, array('colour' => array('hex_colour', true)))) + { + // Replace "error" string with its real, localised form + $error = array_merge($error, array_map(array(&$user, 'lang'), $colour_error)); + } + if (!sizeof($error)) { // Only set the rank, colour, etc. if it's changed or if we're adding a new diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index 6e1782d551..cdfd3416f5 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -819,6 +819,7 @@ $lang = array_merge($lang, array( 'WHO_IS_ONLINE' => 'Who is online', 'WRONG_PASSWORD' => 'You entered an incorrect password.', + 'WRONG_DATA_COLOUR' => 'The colour value you entered is invalid.', 'WRONG_DATA_ICQ' => 'The number you entered is not a valid ICQ number.', 'WRONG_DATA_JABBER' => 'The name you entered is not a valid Jabber account name.', 'WRONG_DATA_LANG' => 'The language you specified is not valid.', diff --git a/phpBB/styles/prosilver/template/ucp_groups_manage.html b/phpBB/styles/prosilver/template/ucp_groups_manage.html index 74cb9f4579..a785d18082 100644 --- a/phpBB/styles/prosilver/template/ucp_groups_manage.html +++ b/phpBB/styles/prosilver/template/ucp_groups_manage.html @@ -54,7 +54,7 @@

{L_GROUP_COLOR_EXPLAIN}
-
    [ {L_COLOUR_SWATCH} ]
+
    [ {L_COLOUR_SWATCH} ]
diff --git a/tests/functional/acp_groups_test.php b/tests/functional/acp_groups_test.php new file mode 100644 index 0000000000..3d8cabb086 --- /dev/null +++ b/tests/functional/acp_groups_test.php @@ -0,0 +1,21 @@ +markTestIncomplete( + 'Test fails on develop due to another test deleting the Administrators group.' + ); + // See https://github.com/phpbb/phpbb3/pull/1407#issuecomment-18465480 + // and https://gist.github.com/bantu/22dc4f6c6c0b8f9e0fa1 + + $this->login(); + $this->admin_login(); + $this->add_lang(array('ucp', 'acp/groups')); + + // Manage Administrators group + $crawler = $this->request('GET', $this->get_url() . '&g=5&sid=' . $this->sid); + $this->assert_response_success(); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $form['group_colour']->setValue($input); + $crawler = $this->client->submit($form); + $this->assertContains($this->lang($expected), $crawler->text()); + } +} diff --git a/tests/functional/ucp_groups_test.php b/tests/functional/ucp_groups_test.php new file mode 100644 index 0000000000..9c6b1edc5e --- /dev/null +++ b/tests/functional/ucp_groups_test.php @@ -0,0 +1,21 @@ +