Skip to content

Commit

Permalink
[imp] Add ability to set redirect headers in com_redirect. Fixes #4292
Browse files Browse the repository at this point in the history
Change language string names

Add extra large class to field

Don't check for empty new url if in advanced mode

Remove not null constraint in db

Remove not null constraint in db (2/2)

Add extra check for a 3xx code

Port to all databases

Add missing update files
  • Loading branch information
wilsonge authored and phproberto committed Oct 23, 2014
1 parent 8f2f8e6 commit b979c6b
Show file tree
Hide file tree
Showing 17 changed files with 262 additions and 47 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE `#__redirect_links` ADD header smallint(3) NOT NULL DEFAULT 301;
ALTER TABLE `#__redirect_links` MODIFY new_url varchar(255);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE "#__redirect_links" ADD COLUMN "header"INTEGER DEFAULT 301 NOT NULL;
ALTER TABLE "#__redirect_links" ALTER COLUMN "new_url" DROP NOT NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE [#__redirect_links] ADD [header] [smallint] NOT NULL DEFAULT 301;
ALTER TABLE [#__redirect_links] ALTER COLUMN [new_url] [nvarchar](255) NULL;
15 changes: 15 additions & 0 deletions administrator/components/com_redirect/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,19 @@
component="com_redirect"
section="component" />
</fieldset>
<fieldset name="redirect"
label="COM_REDIRECT_ADVANCED_OPTIONS"
>
<field
name="mode"
type="radio"
class="btn-group btn-group-yesno"
default="0"
label="COM_REDIRECT_MODE_LABEL"
description="COM_REDIRECT_MODE_DESC"
>
<option value="1">JYES</option>
<option value="0">JNO</option>
</field>
</fieldset>
</config>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!DOCTYPE html><title></title>
121 changes: 121 additions & 0 deletions administrator/components/com_redirect/models/fields/redirect.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<?php
/**
* @package Joomla.Administrator
* @subpackage com_redirect
*
* @copyright Copyright (C) 2005 - 2014 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/

defined('JPATH_BASE') or die;

JFormHelper::loadFieldClass('list');

/**
* A drop down containing all valid HTTP 1.1 response codes.
*
* @package Joomla.Administrator
* @subpackage com_redirect
* @since 3.4
*/
class JFormFieldRedirect extends JFormFieldList
{
/**
* The form field type.
*
* @var string
* @since 3.4
*/
protected $type = 'Redirect';

/**
* A map of integer HTTP 1.1 response codes to the full HTTP Status for the headers.
*
* @var object
* @since 3.4
* @see http://www.iana.org/assignments/http-status-codes/
*/
protected $responseMap = array(
100 => 'HTTP/1.1 100 Continue',
101 => 'HTTP/1.1 101 Switching Protocols',
102 => 'HTTP/1.1 102 Processing',
200 => 'HTTP/1.1 200 OK',
201 => 'HTTP/1.1 201 Created',
202 => 'HTTP/1.1 202 Accepted',
203 => 'HTTP/1.1 203 Non-Authoritative Information',
204 => 'HTTP/1.1 204 No Content',
205 => 'HTTP/1.1 205 Reset Content',
206 => 'HTTP/1.1 206 Partial Content',
207 => 'HTTP/1.1 207 Multi-Status',
208 => 'HTTP/1.1 208 Already Reported',
226 => 'HTTP/1.1 226 IM Used',
300 => 'HTTP/1.1 300 Multiple Choices',
301 => 'HTTP/1.1 301 Moved Permanently',
302 => 'HTTP/1.1 302 Found',
303 => 'HTTP/1.1 303 See other',
304 => 'HTTP/1.1 304 Not Modified',
305 => 'HTTP/1.1 305 Use Proxy',
306 => 'HTTP/1.1 306 (Unused)',
307 => 'HTTP/1.1 307 Temporary Redirect',
308 => 'HTTP/1.1 308 Permanent Redirect',
400 => 'HTTP/1.1 400 Bad Request',
401 => 'HTTP/1.1 401 Unauthorized',
402 => 'HTTP/1.1 402 Payment Required',
403 => 'HTTP/1.1 403 Forbidden',
404 => 'HTTP/1.1 404 Not Found',
405 => 'HTTP/1.1 405 Method Not Allowed',
406 => 'HTTP/1.1 406 Not Acceptable',
407 => 'HTTP/1.1 407 Proxy Authentication Required',
408 => 'HTTP/1.1 408 Request Timeout',
409 => 'HTTP/1.1 409 Conflict',
410 => 'HTTP/1.1 410 Gone',
411 => 'HTTP/1.1 411 Length Required',
412 => 'HTTP/1.1 412 Precondition Failed',
413 => 'HTTP/1.1 413 Payload Too Large',
414 => 'HTTP/1.1 414 URI Too Long',
415 => 'HTTP/1.1 415 Unsupported Media Type',
416 => 'HTTP/1.1 416 Requested Range Not Satisfiable',
417 => 'HTTP/1.1 417 Expectation Failed',
418 => 'HTTP/1.1 418 I\'m a teapot',
422 => 'HTTP/1.1 422 Unprocessable Entity',
423 => 'HTTP/1.1 423 Locked',
424 => 'HTTP/1.1 424 Failed Dependency',
425 => 'HTTP/1.1 425 Reserved for WebDAV advanced collections expired proposal',
426 => 'HTTP/1.1 426 Upgrade Required',
428 => 'HTTP/1.1 428 Precondition Required',
429 => 'HTTP/1.1 429 Too Many Requests',
431 => 'HTTP/1.1 431 Request Header Fields Too Large',
500 => 'HTTP/1.1 500 Internal Server Error',
501 => 'HTTP/1.1 501 Not Implemented',
502 => 'HTTP/1.1 502 Bad Gateway',
503 => 'HTTP/1.1 503 Service Unavailable',
504 => 'HTTP/1.1 504 Gateway Timeout',
505 => 'HTTP/1.1 505 HTTP Version Not Supported',
506 => 'HTTP/1.1 506 Variant Also Negotiates (Experimental)',
507 => 'HTTP/1.1 507 Insufficient Storage',
508 => 'HTTP/1.1 508 Loop Detected',
510 => 'HTTP/1.1 510 Not Extended',
511 => 'HTTP/1.1 511 Network Authentication Required',
);

/**
* Method to get the field input markup.
*
* @return string The field input markup.
* @since 3.4
*/
protected function getOptions()
{
$options = array();

foreach ($this->responseMap as $key => $value)
{
$options[] = JHtml::_('select.option', $key, $value);
}

// Merge any additional options in the XML definition.
$options = array_merge(parent::getOptions(), $options);

return $options;
}
}
10 changes: 10 additions & 0 deletions administrator/components/com_redirect/models/forms/link.xml
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@
readonly="true"
filter="unset" />
</fieldset>
<fieldset name="advanced">
<field
name="header"
type="redirect"
default="301"
class="input-xlarge"
label="COM_REDIRECT_FIELD_REDIRECT_STATUS_CODE_LABEL"
description="COM_REDIRECT_FIELD_REDIRECT_STATUS_CODE_DESC"
/>
</fieldset>
</form>
8 changes: 8 additions & 0 deletions administrator/components/com_redirect/models/link.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ public function getForm($data = array(), $loadData = true)
$form->setFieldAttribute('published', 'filter', 'unset');
}

// If in advanced mode then we make sure the new url field is not compulsory and the header
// field compulsory in case people select non-3xx redirects
if (JComponentHelper::getParams('com_redirect')->get('mode', 0) == true)
{
$form->setFieldAttribute('new_url', 'required', 'false');
$form->setFieldAttribute('header', 'required', 'true');
}

return $form;
}

Expand Down
14 changes: 12 additions & 2 deletions administrator/components/com_redirect/tables/link.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,23 @@ public function check()
return false;
}

// Check for valid name.
if (empty($this->new_url))
// Check for valid name if not in advanced mode.
if (empty($this->new_url) && JComponentHelper::getParams('com_redirect')->get('mode', 0) == false)
{
$this->setError(JText::_('COM_REDIRECT_ERROR_DESTINATION_URL_REQUIRED'));

return false;
}
elseif (empty($this->new_url) && JComponentHelper::getParams('com_redirect')->get('mode', 0) == true)
{
// Else if an empty URL and in redirect mode only throw the same error if the code is a 3xx status code
if ($this->header < 400 && $this->header >= 300)
{
$this->setError(JText::_('COM_REDIRECT_ERROR_DESTINATION_URL_REQUIRED'));

return false;
}
}

// Check for duplicates
if ($this->old_url == $this->new_url)
Expand Down
39 changes: 10 additions & 29 deletions administrator/components/com_redirect/views/link/tmpl/edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,36 +31,17 @@
<?php echo JHtml::_('bootstrap.startTabSet', 'myTab', array('active' => 'basic')); ?>

<?php echo JHtml::_('bootstrap.addTab', 'myTab', 'basic', empty($this->item->id) ? JText::_('COM_REDIRECT_NEW_LINK', true) : JText::sprintf('COM_REDIRECT_EDIT_LINK', $this->item->id, array('jsSafe' => true))); ?>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('old_url'); ?></div>
<div class="controls"><?php echo rawurldecode($this->form->getInput('old_url')); ?></div>
</div>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('new_url'); ?></div>
<div class="controls"><?php echo rawurldecode($this->form->getInput('new_url')); ?></div>
</div>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('published'); ?></div>
<div class="controls"><?php echo $this->form->getInput('published'); ?></div>
</div>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('comment'); ?></div>
<div class="controls"><?php echo $this->form->getInput('comment'); ?></div>
</div>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('id'); ?></div>
<div class="controls"><?php echo $this->form->getInput('id'); ?></div>
</div>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('created_date'); ?></div>
<div class="controls"><?php echo $this->form->getInput('created_date'); ?></div>
</div>
<div class="control-group">
<div class="control-label"><?php echo $this->form->getLabel('modified_date'); ?></div>
<div class="controls"><?php echo $this->form->getInput('modified_date'); ?></div>
</div>
<?php echo $this->form->renderField('old_url'); ?>
<?php echo $this->form->renderField('new_url'); ?>
<?php echo $this->form->renderField('published'); ?>
<?php echo $this->form->renderField('comment'); ?>
<?php echo $this->form->renderField('id'); ?>
<?php echo $this->form->renderField('created_date'); ?>
<?php echo $this->form->renderField('modified_date'); ?>
<?php if (JComponentHelper::getParams('com_redirect')->get('mode')) : ?>
<?php echo $this->form->renderFieldset('advanced'); ?>
<?php endif; ?>
<?php echo JHtml::_('bootstrap.endTab'); ?>

<?php echo JHtml::_('bootstrap.endTabSet'); ?>

<input type="hidden" name="task" value="" />
Expand Down
5 changes: 5 additions & 0 deletions administrator/language/en-GB/en-GB.com_redirect.ini
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
; Note : All ini files need to be saved as UTF-8

COM_REDIRECT="Redirect"
COM_REDIRECT_ADVANCED_OPTIONS="Advanced Options"
COM_REDIRECT_BUTTON_UPDATE_LINKS="Update Links"
COM_REDIRECT_CONFIGURATION="Redirect Manager Options"
COM_REDIRECT_DISABLE_LINK="Disable Link"
Expand All @@ -21,6 +22,8 @@ COM_REDIRECT_FIELD_NEW_URL_LABEL="Destination URL"
COM_REDIRECT_FIELD_OLD_URL_DESC="Enter the URL that has to be redirected."
COM_REDIRECT_FIELD_OLD_URL_LABEL="Source URL"
COM_REDIRECT_FIELD_REFERRER_LABEL="Link Referrer"
COM_REDIRECT_FIELD_REDIRECT_STATUS_CODE_LABEL="Redirect Status Code"
COM_REDIRECT_FIELD_REDIRECT_STATUS_CODE_DESC="Choose the HTTP 1.1 status code to associate with the redirect."
COM_REDIRECT_FIELD_UPDATED_DATE_LABEL="Last Updated Date"
COM_REDIRECT_HEADING_CREATED_DATE="Created Date"
COM_REDIRECT_HEADING_HITS="404 Hits"
Expand All @@ -30,6 +33,8 @@ COM_REDIRECT_HEADING_REFERRER="Referring Page"
COM_REDIRECT_HEADING_UPDATE_LINKS="Update selected links to the following new URL"
COM_REDIRECT_MANAGER_LINK="Redirect Manager: Link"
COM_REDIRECT_MANAGER_LINKS="Redirect Manager: Links"
COM_REDIRECT_MODE_LABEL="Activate Advanced Mode"
COM_REDIRECT_MODE_DESC="Enable more advanced functionality for the component. Only use this if you know what you're doing."
COM_REDIRECT_N_ITEMS_ARCHIVED="%d links successfully archived"
COM_REDIRECT_N_ITEMS_ARCHIVED_1="Link successfully archived"
COM_REDIRECT_N_ITEMS_DELETED="%d links successfully deleted"
Expand Down
3 changes: 2 additions & 1 deletion installation/sql/mysql/joomla.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1529,13 +1529,14 @@ INSERT INTO `#__postinstall_messages` (`extension_id`, `title_key`, `description
CREATE TABLE IF NOT EXISTS `#__redirect_links` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`old_url` varchar(255) NOT NULL,
`new_url` varchar(255) NOT NULL,
`new_url` varchar(255),
`referer` varchar(150) NOT NULL,
`comment` varchar(255) NOT NULL,
`hits` int(10) unsigned NOT NULL DEFAULT 0,
`published` tinyint(4) NOT NULL,
`created_date` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
`modified_date` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
`header` smallint(3) NOT NULL DEFAULT 301,
PRIMARY KEY (`id`),
UNIQUE KEY `idx_link_old` (`old_url`),
KEY `idx_link_modifed` (`modified_date`)
Expand Down
3 changes: 2 additions & 1 deletion installation/sql/postgresql/joomla.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1465,13 +1465,14 @@ INSERT INTO "#__postinstall_messages" ("extension_id", "title_key", "description
CREATE TABLE "#__redirect_links" (
"id" serial NOT NULL,
"old_url" varchar(255) NOT NULL,
"new_url" varchar(255) NOT NULL,
"new_url" varchar(255),
"referer" varchar(150) NOT NULL,
"comment" varchar(255) NOT NULL,
"hits" bigint DEFAULT 0 NOT NULL,
"published" smallint NOT NULL,
"created_date" timestamp without time zone DEFAULT '1970-01-01 00:00:00' NOT NULL,
"modified_date" timestamp without time zone DEFAULT '1970-01-01 00:00:00' NOT NULL,
"header" INTEGER DEFAULT 301 NOT NULL,
PRIMARY KEY ("id"),
CONSTRAINT "#__redirect_links_idx_link_old" UNIQUE ("old_url")
);
Expand Down
3 changes: 2 additions & 1 deletion installation/sql/sqlazure/joomla.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2376,13 +2376,14 @@ SET QUOTED_IDENTIFIER ON;
CREATE TABLE [#__redirect_links](
[id] [bigint] IDENTITY(1,1) NOT NULL,
[old_url] [nvarchar](255) NOT NULL,
[new_url] [nvarchar](255) NOT NULL,
[new_url] [nvarchar](255),
[referer] [nvarchar](150) NOT NULL,
[comment] [nvarchar](255) NOT NULL,
[hits] [bigint] NOT NULL DEFAULT 0,
[published] [smallint] NOT NULL,
[created_date] [datetime] NOT NULL DEFAULT '1900-01-01T00:00:00.000',
[modified_date] [datetime] NOT NULL DEFAULT '1900-01-01T00:00:00.000',
[header] [smallint] NOT NULL DEFAULT 301,
CONSTRAINT [PK_#__redirect_links_id] PRIMARY KEY CLUSTERED
(
[id] ASC
Expand Down
14 changes: 7 additions & 7 deletions libraries/cms/application/cms.php
Original file line number Diff line number Diff line change
Expand Up @@ -949,14 +949,14 @@ public function logout($userid = null, $options = array())
* or "303 See Other" code in the header pointing to the new location. If the headers have already been
* sent this will be accomplished using a JavaScript statement.
*
* @param string $url The URL to redirect to. Can only be http/https URL
* @param boolean $moved True if the page is 301 Permanently Moved, otherwise 303 See Other is assumed.
* @param string $url The URL to redirect to. Can only be http/https URL
* @param integer $status The HTTP 1.1 status code to be provided. 303 is assumed by default.
*
* @return void
*
* @since 3.2
*/
public function redirect($url, $moved = false)
public function redirect($url, $status = 303)

This comment has been minimized.

Copy link
@beat

beat Dec 2, 2014

Contributor

This change here introduces a fatal backwards-compatibility failure of the Joomla API outside a major release and should be reverted before next minor release imho (EDIT: ADDED: lastest 3.3.6 does not include this).

EDIT: ADDED:
It affects all extensions using this public function that is part of Joomla API which then get following fatal error:

You have not supplied a valid HTTP 1.1 status code#0 /home/beat/www/j/libraries/cms/application/cms.php(1014): JApplicationWeb->redirect('http://localhos...', '')

This comment has been minimized.

Copy link
@wilsonge

wilsonge Dec 2, 2014

Author Contributor

Beat what is the code you have used to generate this? I can't fix the b/c break if you don't tell how you got it :)

This comment has been minimized.

Copy link
@cyrezdev

cyrezdev Dec 2, 2014

Contributor

@wilsonge same error message as i've got in my extension (but an old crap code, which was working before, but not good one). My code was something like this :

$app = JFactory::getApplication();
$msg = $app->enqueueMessage('Message', 'error');
$uri    = JFactory::getURI();
$return = base64_encode($uri);
$rlink  = JRoute::_("index.php?option=com_users&view=login&return=$return", false);

$app->redirect($rlink, $msg);

As you can see, not the best practice, but it was one i've found 2 years ago on web, when first steps in Joomla development...
My PR #5247 was not the good solution, but it was there where i could solve the issue (but checking enqueued messages is not the right way).
Hope this helps...
Waiting to know how @beat got this error 0... ;-)

This comment has been minimized.

Copy link
@infograf768

infograf768 Dec 2, 2014

Member

@wilsonge
Looks like all our work to get a 301 in languagefilter is now gone...
#5129
#5135

This comment has been minimized.

Copy link
@beat

beat Dec 2, 2014

Contributor

I have extensions out there still using old B/C API of Joomla 2.5 where second argument of redirect() is a string, and third too. The equivalent use was: $app->redirect( $url, $message, $messageType); with $message='' (empty string) as there was no message. Thus the B/C code was not triggered because $arg[2] was tested empty here b979c6b#diff-11a160a70413114ea70faf547cd097ecR974 which was ok, until that change of logic with the is_bool test.

Those would break with fatal errors on 3.4 upgrade because of this API B/C (The API did not get deprecated, and should thus continue to work).

I will happily upgrade my use in next extensions releases, but nevertheless we don't want a Joomla upgrade to break existing sites. Specially if we can easily avoid it with my suggested simple fix below.

This comment has been minimized.

Copy link
@infograf768

infograf768 Dec 2, 2014

Member

Shall we now change this to

if ($this->app->get('sef_rewrite'))
                        {
                            $this->app->redirect($uri->base() . $uri->toString(array('path', 'query', 'fragment')), 301);
                        }
                        else
                        {
                            $path = $uri->toString(array('path', 'query', 'fragment'));
                            $this->app->redirect($uri->base() . 'index.php' . ($path ? ('/' . $path) : ''), 301);
                        }

This comment has been minimized.

Copy link
@wilsonge

wilsonge Dec 2, 2014

Author Contributor

@beat The is_bool() test always used to exist. I just added the integer check. I'll get back to you on this. I don't dispute at all that something b/c is wrong here that needs fixing. Just need to work out exactly where the bug gets triggered :/

This comment has been minimized.

Copy link
@wilsonge

wilsonge Dec 2, 2014

Author Contributor

@beat Did your $messageType have a value or was that also empty? Trying to formulate some unit tests

This comment has been minimized.

Copy link
@wilsonge

wilsonge Dec 2, 2014

Author Contributor

@infograf768 That code is the new 'preferred option' but the old method should still work (and unit tests say that it does). So not sure why you'r code is broken

This comment has been minimized.

Copy link
@beat

beat Dec 2, 2014

Contributor

@wilsonge the code in master broke for the case where the $message ($args[1] is an empty string '' (to not display any redirect message). Already before your commit, that case did (erroneously) not trigger the B/C code part because empty('') gives TRUE, but that wasn't a fatal error before your commit (with or without a "nice fatal error" message, just a 301 vs 303 redirect, and thus a non-issue. But with new code added by this commit, since it's not just a check like before (roughly): $status = $moved ? 301 : 303; here b979c6b#diff-ee867f6f28651cdefd87314f73708ecfL528 but with the is_bool($status) check here b979c6b#diff-ee867f6f28651cdefd87314f73708ecfR548 it now breaks because string '' is not bool, and thus the error gets thrown resulting into a "nice" fatal error message (and that is repetitive as long as you reload the page, so e.g. login and logout looks like failing all the time in the CB login/logout redirects, but also in all other redirects.
EDIT: ADDED:
My best suggestion is to fix the original bug that was only checking for !emtpy($args[1]) but also to check for string '' with $args[1] === '' that way it's the broken B/C code that is fixed, and your new part is not touched. (unless you also think that if !is_int() is better than is_bool check, in which case, both the fix and the improved check could be made for people calling directly the inner redirect() function.

This comment has been minimized.

Copy link
@beat

beat Dec 2, 2014

Contributor

@wilsonge wrote:

@beat Did your $messageType have a value or was that also empty? Trying to formulate some unit tests
No, in all cases ( $message empty or not), message type was 'message' or 'success' depending on the CMS version (as that was triggering the j 3.x bootstrap class/j 2.5 own class for the redirect message).

{
// Handle B/C by checking if a message was passed to the method, will be removed at 4.0
if (func_num_args() > 1)
Expand All @@ -969,9 +969,9 @@ public function redirect($url, $moved = false)
* $args[0] = $url
* $args[1] = Message to enqueue
* $args[2] = Message type
* $args[3] = $moved
* $args[3] = $status (previously moved)
*/
if (isset($args[1]) && !empty($args[1]) && !is_bool($args[1]))
if (isset($args[1]) && !empty($args[1]) && (!is_bool($args[1]) && !is_int($args[1])))
{
// Log that passing the message to the function is deprecated
JLog::add(
Expand All @@ -997,7 +997,7 @@ public function redirect($url, $moved = false)
$this->enqueueMessage($message, $type);

// Reset the $moved variable
$moved = isset($args[3]) ? (boolean) $args[3] : false;
$status = isset($args[3]) ? (boolean) $args[3] : false;
}
}

Expand All @@ -1009,7 +1009,7 @@ public function redirect($url, $moved = false)
}

// Hand over processing to the parent now
parent::redirect($url, $moved);
parent::redirect($url, $status);
}

/**
Expand Down
Loading

0 comments on commit b979c6b

Please sign in to comment.