Skip to content

Commit

Permalink
Issue #905: Cleaning up validation on Redirect forms. Consistency fix…
Browse files Browse the repository at this point in the history 
…es between aliases and redirects.
  • Loading branch information
@quicksketch
quicksketch committed Apr 21, 2016
1 parent 80b4da0 commit 7302e27
Show file tree
Hide file tree
Showing 8 changed files with 197 additions and 198 deletions.
6 changes: 6 additions & 0 deletions core/modules/path/path.admin.inc
View file
Expand Up @@ -180,8 +180,14 @@ function path_admin_form($form, &$form_state, $path = array('source' => '', 'ali
'#type' => 'submit',
'#value' => t('Delete'),
'#submit' => array('path_admin_form_delete_submit'),
'#limit_validation_errors' => array(array('actions'), array('pid')),
);
}
$form['actions']['cancel'] = array(
'#type' => 'link',
'#title' => t('Cancel'),
'#href' => isset($_GET['destination']) ? $_GET['destination'] : 'admin/config/search/path',
);

return $form;
}
Expand Down
7 changes: 1 addition & 6 deletions core/modules/redirect/config/redirect.settings.json
View file
Expand Up @@ -5,10 +5,5 @@
"passthrough_querystring": true,
"warning": false,
"additional_statuses": false,
"purge_inactive": false,
"global_home": true,
"global_clean": true,
"global_canonical": true,
"global_deslash": false,
"global_admin_paths": false
"purge_inactive": false
}
140 changes: 64 additions & 76 deletions core/modules/redirect/redirect.admin.inc
View file
Expand Up @@ -374,7 +374,7 @@ function redirect_edit_form($form, &$form_state, $redirect = NULL) {
'#maxlength' => 560,
'#default_value' => $redirect->rid || $redirect->redirect ? redirect_url($redirect->redirect, $redirect->redirect_options, TRUE) : '',
'#required' => TRUE,
'#description' => t('Enter an internal Backdrop path, path alias, or complete external URL (like http://example.com/) to redirect to. Use %front to redirect to the front page.', array('%front' => '<front>')),
'#description' => t('Enter an internal Backdrop path, path alias, or complete external URL (like http://example.com/) to which the user will be sent. Use %front to redirect to the front page.', array('%front' => '<front>')),
'#element_validate' => array('redirect_element_validate_redirect'),
);
$form['redirect_options'] = array(
Expand Down Expand Up @@ -403,27 +403,45 @@ function redirect_edit_form($form, &$form_state, $redirect = NULL) {
);

$form['override'] = array(
'#type' => 'checkbox',
'#title' => t('I understand the following warnings and would like to proceed with saving this URL redirect'),
'#default_value' => FALSE,
'#access' => FALSE,
'#required' => FALSE,
'#weight' => -100,
'#prefix' => '<div class="messages warning">',
'#suffix' => '</div>',
'#access' => FALSE,
'#weight' => -100,
);
$form['override']['confirm'] = array(
'#type' => 'checkbox',
'#title' => t('I understand the above warnings and would like to proceed with saving this URL redirect'),
'#default_value' => FALSE,
'#parents' => array('override'),
);
if (!empty($form_state['storage']['override_messages'])) {

if (!empty($form_state['storage']['override_messages']) && !count(form_get_errors())) {
$form['override']['#access'] = TRUE;
//$form['override']['#required'] = TRUE;
$form['override']['#description'] = theme('item_list', array('items' => $form_state['storage']['override_messages']));
if (count($form_state['storage']['override_messages']) > 1) {
$message_output = theme('item_list', array('items' => $form_state['storage']['override_messages']));
}
else {
$message_output = implode('', $form_state['storage']['override_messages']);
}
$form['override']['messages'] = array(
'#markup' => $message_output,
'#weight' => -1,
);
// Reset the messages.
$form_state['storage']['override_messages'] = array();
}

$form['actions'] = array('#type' => 'actions');
$form['actions']['submit'] = array(
'#type' => 'submit',
'#value' => t('Save'),
'#value' => t('Save redirect'),
);
$form['actions']['delete'] = array(
'#type' => 'submit',
'#value' => t('Delete'),
'#submit' => array('redirect_edit_form_delete_submit'),
'#limit_validation_errors' => array(array('actions')),
'#access' => !$redirect->isNew(),
);
$form['actions']['cancel'] = array(
'#type' => 'link',
Expand All @@ -440,26 +458,33 @@ function redirect_edit_form($form, &$form_state, $redirect = NULL) {
* @see redirect_edit_form()
*/
function redirect_element_validate_source($element, &$form_state) {
$value = &$element['#value'];

// Check that the source contains no URL fragment.
if (strpos($value, '#') !== FALSE) {
if (strpos($element['#value'], '#') !== FALSE) {
form_error($element, t('The source path cannot contain an URL fragment anchor.'));
}

_redirect_extract_url_options($element, $form_state);
$parsed_value = _redirect_extract_url_options($element, $form_state);

// Disallow redirections from the frontpage.
if ($value === '<front>') {
if ($parsed_value['path'] === '<front>') {
form_error($element, t('The source path cannot be the front page.'));
}

// Cannot create redirects for valid paths.
if (empty($form_state['values']['override'])) {
$menu_item = menu_get_item($value);
if ($menu_item && $menu_item['page_callback'] != 'redirect_redirect' && $value == $menu_item['path']) {
$form_state['storage']['override_messages']['valid-path'] = t('The source path %path is likely a valid path. It is preferred to <a href="@url-alias">create URL aliases</a> for existing paths rather than redirects.', array('%path' => $value, '@url-alias' => url('admin/config/search/path/add')));
// Check if this is an alias.
$alias = path_load(array('alias' => $parsed_value['path']));
if ($alias) {
$form_state['storage']['override_messages']['existing-alias'] = t('The source path !path is an existing alias. You may prefer to <a href="@url-alias">edit this URL alias</a> rather than using a redirect.', array('!path' => '<em>' . l($parsed_value['path'], $parsed_value['path']) . '</em>', '@url-alias' => url('admin/config/search/path/edit/' . $alias['pid'])));
$form_state['rebuild'] = TRUE;
return $element;
}
$path = backdrop_get_normal_path($parsed_value['path'], $form_state['values']['langcode']);
$menu_item = menu_get_item($path);
if ($menu_item && $menu_item['page_callback'] != 'redirect_redirect') {
$form_state['storage']['override_messages']['valid-path'] = t('The source path !path is an existing path. Creating a redirect may make it inaccessible. You may prefer to <a href="@url-alias">create a URL alias</a> for this path rather than a redirect.', array('!path' => '<em>' . l($parsed_value['path'], $parsed_value['path']) . '</em>', '@url-alias' => url('admin/config/search/path/add')));
$form_state['rebuild'] = TRUE;
return $element;
}
}

Expand All @@ -472,13 +497,14 @@ function redirect_element_validate_source($element, &$form_state) {
* @see redirect_edit_form()
*/
function redirect_element_validate_redirect($element, &$form_state) {
$value = &$element['#value'];
_redirect_extract_url_options($element, $form_state);
$value = &$form_state['values']['redirect'];


// Normalize the path.
$value = backdrop_get_normal_path($value, $form_state['values']['langcode']);
$parsed = _redirect_extract_url_options($element, $form_state);
if (!url_is_external($parsed['url'])) {
$value = backdrop_get_normal_path($parsed['url'], $form_state['values']['langcode']);
}
else {
$value = $parsed['url'];
}
form_set_value($element, $value, $form_state);

if (!valid_url($value) && !valid_url($value, TRUE) && $value != '<front>' && $value != '' && !file_exists($value)) {
form_error($element, t('The redirect path %value is not valid.', array('%value' => $value)));
Expand Down Expand Up @@ -518,11 +544,6 @@ function _redirect_extract_url_options(&$element, &$form_state) {
unset($options['https']);
}

if (!url_is_external($parsed['url'])) {
$parsed['url'] = backdrop_get_normal_path($parsed['url'], $form_state['values']['langcode']);
}

form_set_value($element, $parsed['url'], $form_state);
return $parsed;
}

Expand All @@ -543,17 +564,24 @@ function redirect_edit_form_validate($form, &$form_state) {
$form_state['rebuild'] = TRUE;
}
}

if ($form['override']['#access']) {
backdrop_set_message('Did you read the warnings and click the checkbox?', 'error');
$form_state['rebuild'] = TRUE;
}
}

redirect_validate($redirect, $form, $form_state);
$form_state['redirect'] = $redirect;
}

/**
* Form submit handler; handle the 'Delete' button on redirect_edit_form().
*/
function redirect_edit_form_delete_submit($form, &$form_state) {
$destination = array();
if (isset($_GET['destination'])) {
$destination = backdrop_get_destination();
unset($_GET['destination']);
}
$form_state['redirect'] = array('admin/config/search/redirect/delete/' . $form_state['redirect']->rid, array('query' => $destination));
}

/**
* Form submit handler; insert or update an URL redirect.
*
Expand Down Expand Up @@ -639,41 +667,6 @@ function redirect_settings_form($form, &$form_state) {
'#description' => t('Only redirects managed by the redirect module itself will be deleted. Redirects managed by other modules will be left alone.'),
);

$form['globals'] = array(
'#type' => 'fieldset',
'#title' => t('Always enabled redirections'),
'#description' => t('(formerly Global Redirect features)'),
'#access' => FALSE,
);
$form['globals']['global_home'] = array(
'#type' => 'checkbox',
'#title' => t('Redirect from paths like index.php and /node to the root directory.'),
'#default_value' => $config->get('global_home'),
'#access' => FALSE,
);
$form['globals']['global_clean'] = array(
'#type' => 'checkbox',
'#title' => t('Redirect from non-clean URLs to clean URLs.'),
'#default_value' => $config->get('global_clean'),
'#disabled' => !$config->get('clean_url'),
'#access' => FALSE,
);
$form['globals']['global_canonical'] = array(
'#type' => 'checkbox',
'#title' => t('Redirect from non-canonical URLs to the canonical URLs.'),
'#default_value' => $config->get('global_canonical'),
);
$form['globals']['global_deslash'] = array(
'#type' => 'checkbox',
'#title' => t('Remove trailing slashes from paths.'),
'#default_value' => $config->get('global_deslash'),
'#access' => FALSE,
);
$form['globals']['global_admin_paths'] = array(
'#type' => 'checkbox',
'#title' => t('Allow redirections on admin paths.'),
'#default_value' => $config->get('global_admin_paths'),
);
$form['actions']['#type'] = 'actions';
$form['actions']['submit'] = array(
'#type' => 'submit',
Expand All @@ -695,11 +688,6 @@ function redirect_settings_form_submit($form, &$form_state) {
$config->set('passthrough_querystring', (bool) $values['passthrough_querystring']);
$config->set('additional_statuses', (bool) $values['additional_statuses']);
$config->set('purge_inactive', (bool) $values['purge_inactive']);
$config->set('global_home', (bool) $values['global_home']);
$config->set('global_clean', (bool) $values['global_clean']);
$config->set('global_canonical', (bool) $values['global_canonical']);
$config->set('global_deslash', (bool) $values['global_deslash']);
$config->set('global_admin_paths', (bool) $values['global_admin_paths']);
$config->save();
backdrop_set_message(t('The configuration options have been saved.'));
}
Expand Down
1 change: 1 addition & 0 deletions core/modules/redirect/redirect.info
View file
@@ -1,5 +1,6 @@
name = Redirect
description = Allows users to redirect from old URLs to new URLs.
package = System
backdrop = 1.x
version = BACKDROP_VERSION
type = module
Expand Down
14 changes: 2 additions & 12 deletions core/modules/redirect/redirect.install
View file
Expand Up @@ -40,7 +40,7 @@ function redirect_schema() {
'type' => 'varchar',
'length' => 255,
'not null' => TRUE,
'description' => 'The source path to redirect from.',
'description' => 'The source path from which to redirect.',
),
'source_options' => array(
'type' => 'text',
Expand All @@ -52,7 +52,7 @@ function redirect_schema() {
'type' => 'varchar',
'length' => 255,
'not null' => TRUE,
'description' => 'The destination path to redirect to.',
'description' => 'The destination path to which the user will be redirected.',
),
'redirect_options' => array(
'type' => 'text',
Expand Down Expand Up @@ -118,11 +118,6 @@ function redirect_update_1000() {
$config->set('passthrough_querystring', update_variable_get('redirect_passthrough_querystring', TRUE));
$config->set('default_status_code', update_variable_get('redirect_default_status_code', '301'));
$config->set('purge_inactive', update_variable_get('redirect_purge_inactive', FALSE));
$config->set('global_home', update_variable_get('redirect_global_home', TRUE));
$config->set('global_clean', update_variable_get('redirect_global_clean', TRUE));
$config->set('global_canonical', update_variable_get('redirect_global_canonical', TRUE));
$config->set('global_deslash', update_variable_get('redirect_global_deslash', FALSE));
$config->set('global_admin_paths', update_variable_get('redirect_global_admin_paths', FALSE));
$config->save();

// Remove old variables.
Expand All @@ -132,11 +127,6 @@ function redirect_update_1000() {
update_variable_del('redirect_default_status_code');
update_variable_del('redirect_page_cache');
update_variable_del('redirect_purge_inactive');
update_variable_del('redirect_global_home');
update_variable_del('redirect_global_clean');
update_variable_del('redirect_global_canonical');
update_variable_del('redirect_global_deslash');
update_variable_del('redirect_global_admin_paths');
}

/**
Expand Down
46 changes: 13 additions & 33 deletions core/modules/redirect/redirect.module
View file
Expand Up @@ -131,7 +131,7 @@ function redirect_menu() {
);

// Add an action link to the 404 page.
$site_404 = config_get('system.site_404');
$site_404 = config_get('system.c');
if (empty($path)) {
$site_404 = 'system/404';
}
Expand Down Expand Up @@ -255,36 +255,20 @@ function redirect_init() {
if ($redirect = redirect_get_current_redirect()) {
redirect_redirect($redirect);
}

$redirect_global = FALSE;
$request_uri = $original_uri = ltrim(request_uri(), '/');

// Redirect from non-clean URLs to clean URLs.
if (config_get('redirect.settings', 'global_clean') && config_get('system.core', 'clean_url') && strpos($request_uri, '?q=') !== FALSE) {
//$redirect_global = TRUE;
//$request_uri = str_replace('?q=', '', $request_uri);
}

if (strpos($request_uri, 'index.php') !== FALSE) {
//$redirect_global = TRUE;
//$request_uri = str_replace('index.php', '', $request_uri);
}

//$request_uri = ltrim($request_uri, '/');
//$parsed = parse_url($request_uri);
// TODO: gff figure out what this is trying to do, if it does it and if Not
// determine if should/can be fixed or removed.
if ($redirect_global && $request_uri != $original_uri) {
redirect_redirect(array(/*'redirect' => $request_uri,*/ 'type' => 'global'));
}
}

/**
* Implements hook_cron().
*/
function redirect_cron() {
// Purge inactive self-managed redirects from the database.
redirect_purge_inactive_redirects();
// Throttle to only once every 6 hours.
$last_run = state_get('redirect_cron_last', 0);
$interval = 21600;
if ($last_run + $interval < REQUEST_TIME) {
// Purge inactive self-managed redirects from the database.
redirect_purge_inactive_redirects();
state_set('redirect_cron_last', REQUEST_TIME);
}
}

/**
Expand Down Expand Up @@ -576,7 +560,9 @@ function redirect_load_multiple(array $rids = array(), array $conditions = array
$redirects = array();
if ($rids) {
foreach ($rids as $rid) {
$redirects[$rid] = $cached_redirects[$rid];
if (isset($cached_redirects[$rid])) {
$redirects[$rid] = $cached_redirects[$rid];
}
}
}
elseif ($conditions) {
Expand Down Expand Up @@ -1033,7 +1019,7 @@ function redirect_page_cache_clear(Redirect $redirect) {
* - If the PHP entry point is the root index.php file.
* - If PHP is not running as CLI.
* - If the site is not offline or in install/update mode.
* - If the curerent page is not an admin page (check can be disabled).
* - If the current page is not an admin page (check can be disabled).
* - If the current request does not have any POST data since a redirect
* may interrupt form submission.
*
Expand All @@ -1042,9 +1028,7 @@ function redirect_page_cache_clear(Redirect $redirect) {
*/
function redirect_can_redirect() {
$can_redirect = &backdrop_static(__FUNCTION__);
$config = config('redirect.settings');
if (!isset($can_redirect)) {
$path = current_path();
$can_redirect = TRUE;

if ($_SERVER['SCRIPT_NAME'] != $GLOBALS['base_path'] . 'index.php') {
Expand All @@ -1063,10 +1047,6 @@ function redirect_can_redirect() {
// Do not redirect in offline or maintenance mode.
$can_redirect = FALSE;
}
elseif (!$config->get('global_admin_paths') && path_is_admin($path)) {
// Do not redirect on admin paths.
$can_redirect = FALSE;
}
}

return $can_redirect;
Expand Down

0 comments on commit 7302e27

Please sign in to comment.