diff --git a/.travis.yml b/.travis.yml index 1a802cfa7..e610e822d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,27 +1,21 @@ language: php -php: - - 5.3 - - 5.4 +php: + - 5.3 env: - - DB=MYSQL CORE_RELEASE=master - - DB=PGSQL CORE_RELEASE=master - - DB=SQLITE3 CORE_RELEASE=master - - PHPCS=1 CORE_RELEASE=master + - DB=MYSQL CORE_RELEASE=master matrix: - exclude: - - php: 5.4 + include: + - php: 5.3 env: DB=PGSQL CORE_RELEASE=master + - php: 5.3 + env: DB=SQLITE CORE_RELEASE=master - php: 5.4 - env: DB=SQLITE3 CORE_RELEASE=master - - php: 5.4 - env: PHPCS=1 CORE_RELEASE=master - allow_failures: - - env: DB=PGSQL CORE_RELEASE=master - - env: DB=SQLITE3 CORE_RELEASE=master - - env: PHPCS=1 CORE_RELEASE=master + env: DB=MYSQL CORE_RELEASE=master + - php: 5.5 + env: DB=MYSQL CORE_RELEASE=master before_script: - phpenv rehash diff --git a/README.md b/README.md index da597b0d8..cbc5c1ac7 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ and [installation from source](http://doc.silverstripe.org/framework/en/installa ## Bugtracker ## -Bugs are tracked on [github.com](https://github.com/silverstripe/framework/issues). +Bugs are tracked on [github.com](https://github.com/silverstripe/silverstripe-framework/issues). Please read our [issue reporting guidelines](http://doc.silverstripe.org/framework/en/misc/contributing/issues). ## Development and Contribution ## diff --git a/admin/code/CMSBatchActionHandler.php b/admin/code/CMSBatchActionHandler.php index 0d61fb8d7..d3229e694 100644 --- a/admin/code/CMSBatchActionHandler.php +++ b/admin/code/CMSBatchActionHandler.php @@ -16,6 +16,12 @@ class CMSBatchActionHandler extends RequestHandler { '$BatchAction/confirmation' => 'handleConfirmation', '$BatchAction' => 'handleBatchAction', ); + + private static $allowed_actions = array( + 'handleBatchAction', + 'handleApplicablePages', + 'handleConfirmation', + ); protected $parentController; diff --git a/admin/code/CMSProfileController.php b/admin/code/CMSProfileController.php index 9c35b009d..e1b776541 100644 --- a/admin/code/CMSProfileController.php +++ b/admin/code/CMSProfileController.php @@ -6,6 +6,7 @@ class CMSProfileController extends LeftAndMain { private static $menu_title = 'My Profile'; private static $required_permission_codes = false; + private static $tree_class = 'Member'; public function getResponseNegotiator() { diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index c57341d26..02cf200b0 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -76,7 +76,7 @@ class LeftAndMain extends Controller implements PermissionProvider { * @config * @var string */ - private static $help_link = 'http://3.0.userhelp.silverstripe.org'; + private static $help_link = 'http://userhelp.silverstripe.org/framework/en/3.1'; /** * @var array diff --git a/admin/css/screen.css b/admin/css/screen.css index b9ccb95c3..c414cfb24 100644 --- a/admin/css/screen.css +++ b/admin/css/screen.css @@ -460,7 +460,8 @@ body.cms { overflow: hidden; } .cms-add-form .step-label { opacity: 0.9; } .cms-add-form .step-label .flyout { height: 17px; padding-top: 5px; } .cms-add-form .step-label .title { padding-top: 5px; font-weight: bold; text-shadow: 1px 1px 0 white; } -.cms-add-form ul.SelectionGroup { padding-left: 28px; } +.cms-add-form ul.SelectionGroup { padding-left: 28px; overflow: visible; *zoom: 1; } +.cms-add-form ul.SelectionGroup:after { content: "\0020"; display: block; height: 0; clear: both; overflow: hidden; visibility: hidden; } .cms-add-form .parent-mode { padding: 8px; overflow: auto; } #Form_AddForm_PageType_Holder ul { padding-left: 20px; } @@ -667,7 +668,7 @@ body.cms-dialog { overflow: auto; background: url("../images/textures/bg_cms_mai .htmleditorfield-mediaform .htmleditorfield-from-cms .ss-uploadfield h4 { float: left; margin-top: 4px; margin-bottom: 0; } .htmleditorfield-mediaform .htmleditorfield-from-cms .ss-uploadfield .middleColumn { margin-top: 16px; margin-left: 184px; } .htmleditorfield-mediaform .htmleditorfield-from-cms .ss-uploadfield .field.treedropdown { border-bottom: 0; padding: 0; } -.htmleditorfield-mediaform .ss-uploadfield-editandorganize { display: none; } +.htmleditorfield-mediaform .ss-assetuploadfield .ss-uploadfield-editandorganize .ss-uploadfield-files .ss-uploadfield-item-info { background-color: #9e9e9e; background-image: url('data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4gPHN2ZyB2ZXJzaW9uPSIxLjEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyI+PGRlZnM+PGxpbmVhckdyYWRpZW50IGlkPSJncmFkIiBncmFkaWVudFVuaXRzPSJ1c2VyU3BhY2VPblVzZSIgeDE9IjUwJSIgeTE9IjAlIiB4Mj0iNTAlIiB5Mj0iMTAwJSI+PHN0b3Agb2Zmc2V0PSIwJSIgc3RvcC1jb2xvcj0iIzllOWU5ZSIvPjxzdG9wIG9mZnNldD0iOCUiIHN0b3AtY29sb3I9IiM5ZDlkOWQiLz48c3RvcCBvZmZzZXQ9IjUwJSIgc3RvcC1jb2xvcj0iIzg3ODc4NyIvPjxzdG9wIG9mZnNldD0iNTQlIiBzdG9wLWNvbG9yPSIjODY4Njg2Ii8+PHN0b3Agb2Zmc2V0PSI5NiUiIHN0b3AtY29sb3I9IiM2YjZiNmIiLz48c3RvcCBvZmZzZXQ9IjEwMCUiIHN0b3AtY29sb3I9IiM2YzZjNmMiLz48L2xpbmVhckdyYWRpZW50PjwvZGVmcz48cmVjdCB4PSIwIiB5PSIwIiB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSJ1cmwoI2dyYWQpIiAvPjwvc3ZnPiA='); background-size: 100%; background-image: -webkit-gradient(linear, 50% 0%, 50% 100%, color-stop(0%, #9e9e9e), color-stop(8%, #9d9d9d), color-stop(50%, #878787), color-stop(54%, #868686), color-stop(96%, #6b6b6b), color-stop(100%, #6c6c6c)); background-image: -webkit-linear-gradient(top, #9e9e9e 0%, #9d9d9d 8%, #878787 50%, #868686 54%, #6b6b6b 96%, #6c6c6c 100%); background-image: -moz-linear-gradient(top, #9e9e9e 0%, #9d9d9d 8%, #878787 50%, #868686 54%, #6b6b6b 96%, #6c6c6c 100%); background-image: -o-linear-gradient(top, #9e9e9e 0%, #9d9d9d 8%, #878787 50%, #868686 54%, #6b6b6b 96%, #6c6c6c 100%); background-image: linear-gradient(top, #9e9e9e 0%, #9d9d9d 8%, #878787 50%, #868686 54%, #6b6b6b 96%, #6c6c6c 100%); } /** -------------------------------------------- Search forms (used in AssetAdmin, ModelAdmin, etc) -------------------------------------------- */ .cms-search-form { margin-bottom: 16px; } @@ -751,7 +752,7 @@ form.import-form label.left { width: 250px; } .cms .jstree .jstree-wholerow-real, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow-real { position: relative; z-index: 1; } .cms .jstree .jstree-wholerow-real li, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow-real li { cursor: pointer; } .cms .jstree .jstree-wholerow-real a, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow-real a { border-left-color: transparent !important; border-right-color: transparent !important; } -.cms .jstree .jstree-wholerow, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow, .cms .jstree .jstree-wholerow ul, .cms .jstree .jstree-wholerow li, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow ul, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow ul, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow li, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow li, .cms .jstree .jstree-wholerow a, .cms .jstree .jstree-wholerow a:hover, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow a, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a:hover, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow a:hover, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow ul, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow ul, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow li, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow li, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow ul, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow li, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow a, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow a:hover, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a:hover, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a:hover { margin: 0 !important; padding: 0 !important; } +.cms .jstree .jstree-wholerow, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow, .cms .jstree .jstree-wholerow ul, .cms .jstree .jstree-wholerow li, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow ul, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow ul, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow li, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow li, .cms .jstree .jstree-wholerow a, .cms .jstree .jstree-wholerow a:hover, .cms .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .cms .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow ul, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow li, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a:hover { margin: 0 !important; padding: 0 !important; } .cms .jstree .jstree-wholerow, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow { position: relative; z-index: 0; height: 0; background: transparent !important; } .cms .jstree .jstree-wholerow ul, .cms .jstree .jstree-wholerow li, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow ul, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow li { background: transparent !important; width: 100%; } .cms .jstree .jstree-wholerow a, .cms .jstree .jstree-wholerow a:hover, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a, .TreeDropdownField .treedropdownfield-panel .jstree .jstree-wholerow a:hover { text-indent: -9999px !important; width: 100%; border-right-width: 0px !important; border-left-width: 0px !important; } @@ -770,7 +771,7 @@ form.import-form label.left { width: 250px; } .cms .jstree-themeroller a, .TreeDropdownField .treedropdownfield-panel .jstree-themeroller a { padding: 0 2px; } .cms .jstree-themeroller .ui-icon, .TreeDropdownField .treedropdownfield-panel .jstree-themeroller .ui-icon { overflow: visible; } .cms .jstree-themeroller .jstree-no-icon, .TreeDropdownField .treedropdownfield-panel .jstree-themeroller .jstree-no-icon { display: none; } -.cms #jstree-marker, .cms .TreeDropdownField .treedropdownfield-panel #jstree-marker, .TreeDropdownField .treedropdownfield-panel .cms #jstree-marker, .cms #jstree-marker-line, .cms .TreeDropdownField .treedropdownfield-panel #jstree-marker-line, .TreeDropdownField .treedropdownfield-panel .cms #jstree-marker-line, .TreeDropdownField .treedropdownfield-panel .cms #jstree-marker, .cms .TreeDropdownField .treedropdownfield-panel #jstree-marker, .TreeDropdownField .treedropdownfield-panel #jstree-marker, .TreeDropdownField .treedropdownfield-panel .cms #jstree-marker-line, .cms .TreeDropdownField .treedropdownfield-panel #jstree-marker-line, .TreeDropdownField .treedropdownfield-panel #jstree-marker-line { padding: 0; margin: 0; overflow: hidden; position: absolute; top: -30px; background-repeat: no-repeat; display: none; } +.cms #jstree-marker, .cms .TreeDropdownField .treedropdownfield-panel #jstree-marker, .TreeDropdownField .treedropdownfield-panel .cms #jstree-marker, .cms #jstree-marker-line, .cms .TreeDropdownField .treedropdownfield-panel #jstree-marker-line, .TreeDropdownField .treedropdownfield-panel .cms #jstree-marker-line, .TreeDropdownField .treedropdownfield-panel #jstree-marker, .TreeDropdownField .treedropdownfield-panel #jstree-marker-line { padding: 0; margin: 0; overflow: hidden; position: absolute; top: -30px; background-repeat: no-repeat; display: none; } .cms #jstree-marker, .TreeDropdownField .treedropdownfield-panel #jstree-marker { line-height: 10px; font-size: 12px; height: 12px; width: 8px; z-index: 10001; background-color: transparent; text-shadow: 1px 1px 1px white; color: black; } .cms #jstree-marker-line, .TreeDropdownField .treedropdownfield-panel #jstree-marker-line { line-height: 0%; font-size: 1px; height: 1px; width: 100px; z-index: 10000; background-color: #456c43; cursor: pointer; border: 1px solid #eeeeee; border-left: 0; -moz-box-shadow: 0px 0px 2px #666; -webkit-box-shadow: 0px 0px 2px #666; box-shadow: 0px 0px 2px #666; -moz-border-radius: 1px; border-radius: 1px; -webkit-border-radius: 1px; } .cms #vakata-contextmenu, .TreeDropdownField .treedropdownfield-panel #vakata-contextmenu { display: block; visibility: hidden; left: 0; top: -200px; position: absolute; margin: 0; padding: 0; min-width: 180px; background: #FFF; border: 1px solid silver; z-index: 10000; *width: 180px; -webkit-box-shadow: 0 0 10px #cccccc; -moz-box-shadow: 0 0 10px #cccccc; box-shadow: 0 0 10px #cccccc; } @@ -813,7 +814,7 @@ form.import-form label.left { width: 250px; } .tree-holder.jstree-apple span.badge.status-deletedonlive, .tree-holder.jstree-apple span.badge.status-removedfromdraft, .cms-tree.jstree-apple span.badge.status-deletedonlive, .cms-tree.jstree-apple span.badge.status-removedfromdraft { color: #636363; border: 1px solid #E49393; background-color: #F2DADB; } .tree-holder.jstree-apple span.badge.status-workflow-approval, .cms-tree.jstree-apple span.badge.status-workflow-approval { color: #56660C; border: 1px solid #7C8816; background-color: #DAE79A; } .tree-holder.jstree-apple span.comment-count, .cms-tree.jstree-apple span.comment-count { clear: both; position: relative; text-transform: uppercase; display: inline-block; overflow: visible; padding: 0px 3px; font-size: 0.75em; line-height: 1em; margin-left: 3px; margin-right: 6px; -webkit-border-radius: 2px 2px; -moz-border-radius: 2px / 2px; border-radius: 2px / 2px; color: #7E7470; border: 1px solid #C9B800; background-color: #FFF0BC; } -.tree-holder.jstree-apple span.comment-count span.comment-count:before, .tree-holder.jstree-apple span.comment-count .cms-tree.jstree-apple span.comment-count:before, .cms-tree.jstree-apple .tree-holder.jstree-apple span.comment-count span.comment-count:before, .tree-holder.jstree-apple span.comment-count span.comment-count:after, .tree-holder.jstree-apple span.comment-count .cms-tree.jstree-apple span.comment-count:after, .cms-tree.jstree-apple .tree-holder.jstree-apple span.comment-count span.comment-count:after, .cms-tree.jstree-apple span.comment-count .tree-holder.jstree-apple span.comment-count:before, .tree-holder.jstree-apple .cms-tree.jstree-apple span.comment-count span.comment-count:before, .cms-tree.jstree-apple span.comment-count span.comment-count:before, .cms-tree.jstree-apple span.comment-count .tree-holder.jstree-apple span.comment-count:after, .tree-holder.jstree-apple .cms-tree.jstree-apple span.comment-count span.comment-count:after, .cms-tree.jstree-apple span.comment-count span.comment-count:after { content: ""; position: absolute; border-style: solid; /* reduce the damage in FF3.0 */ display: block; width: 0; } +.tree-holder.jstree-apple span.comment-count span.comment-count:before, .tree-holder.jstree-apple span.comment-count span.comment-count:after, .cms-tree.jstree-apple span.comment-count span.comment-count:before, .cms-tree.jstree-apple span.comment-count span.comment-count:after { content: ""; position: absolute; border-style: solid; /* reduce the damage in FF3.0 */ display: block; width: 0; } .tree-holder.jstree-apple span.comment-count:before, .cms-tree.jstree-apple span.comment-count:before { bottom: -4px; /* value = - border-top-width - border-bottom-width */ left: 3px; /* controls horizontal position */ border-width: 4px 4px 0; border-color: #C9B800 transparent; } .tree-holder.jstree-apple span.comment-count:after, .cms-tree.jstree-apple span.comment-count:after { bottom: -3px; /* value = - border-top-width - border-bottom-width */ left: 4px; /* value = (:before left) + (:before border-left) - (:after border-left) */ border-width: 3px 3px 0; border-color: #FFF0BC transparent; } .tree-holder.jstree-apple .jstree-hovered, .cms-tree.jstree-apple .jstree-hovered { text-shadow: none; text-decoration: none; } diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 38a88b263..66899d64b 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -5,10 +5,23 @@ jQuery.noConflict(); */ (function($) { - window.onresize = function(e) { + var windowWidth, windowHeight; + $(window).bind('resize.leftandmain', function(e) { // Entwine's 'fromWindow::onresize' does not trigger on IE8. Use synthetic event. - $('.cms-container').trigger('windowresize'); - }; + var cb = function() {$('.cms-container').trigger('windowresize');}; + + // Workaround to avoid IE8 infinite loops when elements are resized as a result of this event + if($.browser.msie && parseInt($.browser.version, 10) < 9) { + var newWindowWidth = $(window).width(), newWindowHeight = $(window).height(); + if(newWindowWidth != windowWidth || newWindowHeight != windowHeight) { + windowWidth = newWindowWidth; + windowHeight = newWindowHeight; + cb(); + } + } else { + cb(); + } + }); // setup jquery.entwine $.entwine.warningLevel = $.entwine.WARN_LEVEL_BESTPRACTISE; @@ -136,7 +149,7 @@ jQuery.noConflict(); this._super(); return; } - + // Initialize layouts this.redraw(); @@ -228,15 +241,15 @@ jQuery.noConflict(); }, this.getLayoutOptions() )); - + // Trigger layout algorithm once at the top. This also lays out children - we move from outside to // inside, resizing to fit the parent. this.layout(); // Redraw on all the children that need it - this.find('.cms-panel-layout').redraw(); - this.find('.cms-content-fields[data-layout-type]').redraw(); - this.find('.cms-edit-form[data-layout-type]').redraw(); + this.find('.cms-panel-layout').redraw(); + this.find('.cms-content-fields[data-layout-type]').redraw(); + this.find('.cms-edit-form[data-layout-type]').redraw(); this.find('.cms-preview').redraw(); this.find('.cms-content').redraw(); }, @@ -590,7 +603,7 @@ jQuery.noConflict(); if(typeof(window.sessionStorage)=="undefined" || window.sessionStorage === null) return; var selectedTabs = [], url = this._tabStateUrl(); - this.find('.cms-tabset,.ss-tabset').each(function(i, el) { + this.find('.cms-tabset,.ss-tabset').each(function(i, el) { var id = $(el).attr('id'); if(!id) return; // we need a unique reference if(!$(el).data('tabs')) return; // don't act on uninit'ed controls @@ -650,8 +663,8 @@ jQuery.noConflict(); } else if(sessionStates) { $.each(sessionStates, function(i, sessionState) { if(tabset.is('#' + sessionState.id)) index = sessionState.selected; - }); - } + }); + } if(index !== null) tabset.tabs('select', index); }); }, @@ -671,7 +684,7 @@ jQuery.noConflict(); } else { for(var i=0;iresponse) $this->response = new SS_HTTPResponse(); @@ -473,7 +475,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { $url = Director::baseURL() . $url; } - $this->response->redirect($url, $code); + return $this->response->redirect($url, $code); } /** diff --git a/control/RequestHandler.php b/control/RequestHandler.php index a6db29fe4..06604c914 100644 --- a/control/RequestHandler.php +++ b/control/RequestHandler.php @@ -96,6 +96,16 @@ class RequestHandler extends ViewableData { * @config */ private static $allowed_actions = null; + + /** + * @config + * @var boolean Enforce presence of $allowed_actions when checking acccess. + * Defaults to TRUE, meaning all URL actions will be denied. + * When set to FALSE, the controller will allow *all* public methods to be called. + * In most cases this isn't desireable, and in fact a security risk, + * since some helper methods can cause side effects which shouldn't be exposed through URLs. + */ + private static $require_allowed_actions = true; public function __construct() { $this->brokenOnConstruct = false; @@ -430,12 +440,12 @@ class RequestHandler extends ViewableData { // If defined as empty array, deny action $isAllowed = false; } elseif($allowedActions === null) { - // If undefined, allow action - $isAllowed = true; + // If undefined, allow action based on configuration + $isAllowed = !Config::inst()->get('RequestHandler', 'require_allowed_actions'); } // If we don't have a match in allowed_actions, - // whitelist the 'index' action as well as undefined actions. + // whitelist the 'index' action as well as undefined actions based on configuration. if(!$isDefined && ($action == 'index' || empty($action))) { $isAllowed = true; } diff --git a/control/Session.php b/control/Session.php index 43ae43f29..8c24d3ba5 100644 --- a/control/Session.php +++ b/control/Session.php @@ -86,7 +86,7 @@ class Session { /** - * @var $timeout Set session timeout + * @var $timeout Set session timeout in seconds. * @config */ private static $timeout = 0; @@ -523,9 +523,11 @@ class Session { if(!session_id() && !headers_sent()) { if($domain) { - session_set_cookie_params($timeout, $path, $domain, $secure /* secure */, true /* httponly */); + session_set_cookie_params($timeout, $path, $domain, + $secure /* secure */, true /* httponly */); } else { - session_set_cookie_params($timeout, $path, null, $secure /* secure */, true /* httponly */); + session_set_cookie_params($timeout, $path, null, + $secure /* secure */, true /* httponly */); } // Allow storing the session in a non standard location @@ -541,7 +543,8 @@ class Session { // Modify the timeout behaviour so it's the *inactive* time before the session expires. // By default it's the total session lifetime if($timeout && !headers_sent()) { - Cookie::set(session_name(), session_id(), time()+$timeout, $path, $domain ? $domain : null, $secure, true); + Cookie::set(session_name(), session_id(), $timeout/86400, $path, $domain ? $domain + : null, $secure, true); } } diff --git a/core/Config.php b/core/Config.php index 10c00ed3c..752e73c27 100644 --- a/core/Config.php +++ b/core/Config.php @@ -295,8 +295,12 @@ class Config { * instead, the last manifest to be added always wins */ public function pushConfigYamlManifest(SS_ConfigManifest $manifest) { - array_unshift($this->manifests, $manifest->yamlConfig); + array_unshift($this->manifests, $manifest); + + // Now that we've got another yaml config manifest we need to clean the cache $this->cache->clean(); + // We also need to clean the cache if the manifest's calculated config values change + $manifest->registerChangeCallback(array($this->cache, 'clean')); // @todo: Do anything with these. They're for caching after config.php has executed $this->collectConfigPHPSettings = true; @@ -479,10 +483,13 @@ class Config { } } + $value = $nothing = null; + // Then the manifest values foreach($this->manifests as $manifest) { - if (isset($manifest[$class][$name])) { - self::merge_low_into_high($result, $manifest[$class][$name], $suppress); + $value = $manifest->get($class, $name, $nothing); + if ($value !== $nothing) { + self::merge_low_into_high($result, $value, $suppress); if ($result !== null && !is_array($result)) return $result; } } diff --git a/core/PaginatedList.php b/core/PaginatedList.php index e4c8139be..3cdd456b5 100644 --- a/core/PaginatedList.php +++ b/core/PaginatedList.php @@ -80,7 +80,7 @@ class PaginatedList extends SS_ListDecorator { * @param int $page */ public function setCurrentPage($page) { - $this->pageStart = ($page - 1) * $this->pageLength; + $this->pageStart = ($page - 1) * $this->getPageLength(); return $this; } @@ -91,8 +91,8 @@ class PaginatedList extends SS_ListDecorator { */ public function getPageStart() { if ($this->pageStart === null) { - if ($this->request && isset($this->request[$this->getVar])) { - $this->pageStart = (int) $this->request[$this->getVar]; + if ($this->request && isset($this->request[$this->getPaginationGetVar()])) { + $this->pageStart = (int) $this->request[$this->getPaginationGetVar()]; } else { $this->pageStart = 0; } @@ -181,7 +181,7 @@ class PaginatedList extends SS_ListDecorator { if($this->limitItems) { $tmptList = clone $this->list; return new IteratorIterator( - $tmptList->limit($this->pageLength, $this->getPageStart()) + $tmptList->limit($this->getPageLength(), $this->getPageStart()) ); } else { return new IteratorIterator($this->list); @@ -223,7 +223,7 @@ class PaginatedList extends SS_ListDecorator { for ($i = $start; $i < $end; $i++) { $result->push(new ArrayData(array( 'PageNum' => $i + 1, - 'Link' => HTTP::setGetVar($this->getVar, $i * $this->pageLength), + 'Link' => HTTP::setGetVar($this->getPaginationGetVar(), $i * $this->getPageLength()), 'CurrentBool' => $this->CurrentPage() == ($i + 1) ))); } @@ -292,7 +292,7 @@ class PaginatedList extends SS_ListDecorator { } for ($i = 0; $i < $total; $i++) { - $link = HTTP::setGetVar($this->getVar, $i * $this->pageLength); + $link = HTTP::setGetVar($this->getPaginationGetVar(), $i * $this->getPageLength()); $num = $i + 1; $emptyRange = $num != 1 && $num != $total && ( @@ -321,14 +321,14 @@ class PaginatedList extends SS_ListDecorator { * @return int */ public function CurrentPage() { - return floor($this->getPageStart() / $this->pageLength) + 1; + return floor($this->getPageStart() / $this->getPageLength()) + 1; } /** * @return int */ public function TotalPages() { - return ceil($this->getTotalItems() / $this->pageLength); + return ceil($this->getTotalItems() / $this->getPageLength()); } /** @@ -369,9 +369,9 @@ class PaginatedList extends SS_ListDecorator { */ public function LastItem() { if ($start = $this->getPageStart()) { - return min($start + $this->pageLength, $this->getTotalItems()); + return min($start + $this->getPageLength(), $this->getTotalItems()); } else { - return min($this->pageLength, $this->getTotalItems()); + return min($this->getPageLength(), $this->getTotalItems()); } } @@ -381,7 +381,7 @@ class PaginatedList extends SS_ListDecorator { * @return string */ public function FirstLink() { - return HTTP::setGetVar($this->getVar, 0); + return HTTP::setGetVar($this->getPaginationGetVar(), 0); } /** @@ -390,7 +390,7 @@ class PaginatedList extends SS_ListDecorator { * @return string */ public function LastLink() { - return HTTP::setGetVar($this->getVar, ($this->TotalPages() - 1) * $this->pageLength); + return HTTP::setGetVar($this->getPaginationGetVar(), ($this->TotalPages() - 1) * $this->getPageLength()); } /** @@ -401,7 +401,7 @@ class PaginatedList extends SS_ListDecorator { */ public function NextLink() { if ($this->NotLastPage()) { - return HTTP::setGetVar($this->getVar, $this->getPageStart() + $this->pageLength); + return HTTP::setGetVar($this->getPaginationGetVar(), $this->getPageStart() + $this->getPageLength()); } } @@ -413,8 +413,8 @@ class PaginatedList extends SS_ListDecorator { */ public function PrevLink() { if ($this->NotFirstPage()) { - return HTTP::setGetVar($this->getVar, $this->getPageStart() - $this->pageLength); + return HTTP::setGetVar($this->getPaginationGetVar(), $this->getPageStart() - $this->getPageLength()); } } - } +} diff --git a/core/manifest/ConfigManifest.php b/core/manifest/ConfigManifest.php index cf1d13008..1398518aa 100644 --- a/core/manifest/ConfigManifest.php +++ b/core/manifest/ConfigManifest.php @@ -9,7 +9,16 @@ */ class SS_ConfigManifest { - /** + /** @var string - The base path used when building the manifest */ + protected $base; + + /** @var string - A string to prepend to all cache keys to ensure all keys are unique to just this $base */ + protected $key; + + /** @var bool - Whether `test` directories should be searched when searching for configuration */ + protected $includeTests; + + /** * All the values needed to be collected to determine the correct combination of fragements for * the current environment. * @var array @@ -34,6 +43,17 @@ class SS_ConfigManifest { */ public $yamlConfig = array(); + /** + * The variant key state as when yamlConfig was loaded + * @var string + */ + protected $yamlConfigVariantKey = null; + + /** + * @var [callback] A list of callbacks to be called whenever the content of yamlConfig changes + */ + protected $configChangeCallbacks = array(); + /** * A side-effect of collecting the _config fragments is the calculation of all module directories, since * the definition of a module is "a directory that contains either a _config.php file or a _config directory @@ -64,6 +84,8 @@ class SS_ConfigManifest { */ public function __construct($base, $includeTests = false, $forceRegen = false ) { $this->base = $base; + $this->key = sha1($base).'_'; + $this->includeTests = $includeTests; // Get the Zend Cache to load/store cache into $this->cache = SS_Cache::factory('SS_Configuration', 'Core', array( @@ -74,24 +96,31 @@ class SS_ConfigManifest { // Unless we're forcing regen, try loading from cache if (!$forceRegen) { // The PHP config sources are always needed - $this->phpConfigSources = $this->cache->load('php_config_sources'); + $this->phpConfigSources = $this->cache->load($this->key.'php_config_sources'); // Get the variant key spec - $this->variantKeySpec = $this->cache->load('variant_key_spec'); - // Try getting the pre-filtered & merged config for this variant - if (!($this->yamlConfig = $this->cache->load('yaml_config_'.$this->variantKey()))) { - // Otherwise, if we do have the yaml config fragments (and we should since we have a variant key spec) - // work out the config for this variant - if ($this->yamlConfigFragments = $this->cache->load('yaml_config_fragments')) { - $this->buildYamlConfigVariant(); - } - } + $this->variantKeySpec = $this->cache->load($this->key.'variant_key_spec'); } - // If we don't have a config yet, we need to do a full regen to get it - if (!$this->yamlConfig) { + // If we don't have a variantKeySpec (because we're forcing regen, or it just wasn't in the cache), generate it + if (!$this->variantKeySpec) { $this->regenerate($includeTests); - $this->buildYamlConfigVariant(); } + + // At this point $this->variantKeySpec will always contain something valid, so we can build the variant + $this->buildYamlConfigVariant(); + } + + /** + * Register a callback to be called whenever the calculated merged config changes + * + * In some situations the merged config can change - for instance, code in _config.php can cause which Only + * and Except fragments match. Registering a callback with this function allows code to be called when + * this happens. + * + * @param callback $callback + */ + public function registerChangeCallback($callback) { + $this->configChangeCallbacks[] = $callback; } /** @@ -102,6 +131,22 @@ class SS_ConfigManifest { foreach ($this->phpConfigSources as $config) { require_once $config; } + + if ($this->variantKey() != $this->yamlConfigVariantKey) $this->buildYamlConfigVariant(); + } + + /** + * Gets the (merged) config value for the given class and config property name + * + * @param string $class - The class to get the config property value for + * @param string $name - The config property to get the value for + * @param any $default - What to return if no value was contained in any YAML file for the passed $class and $name + * @return any - The merged set of all values contained in all the YAML configuration files for the passed + * $class and $name, or $default if there are none + */ + public function get($class, $name, $default=null) { + if (isset($this->yamlConfig[$class][$name])) return $this->yamlConfig[$class][$name]; + else return $default; } /** @@ -165,9 +210,9 @@ class SS_ConfigManifest { $this->buildVariantKeySpec(); if ($cache) { - $this->cache->save($this->phpConfigSources, 'php_config_sources'); - $this->cache->save($this->yamlConfigFragments, 'yaml_config_fragments'); - $this->cache->save($this->variantKeySpec, 'variant_key_spec'); + $this->cache->save($this->phpConfigSources, $this->key.'php_config_sources'); + $this->cache->save($this->yamlConfigFragments, $this->key.'yaml_config_fragments'); + $this->cache->save($this->variantKeySpec, $this->key.'variant_key_spec'); } } @@ -395,10 +440,17 @@ class SS_ConfigManifest { $matchingFragments = array(); foreach ($this->yamlConfigFragments as $i => $fragment) { - $failsonly = isset($fragment['only']) && !$this->matchesPrefilterVariantRules($fragment['only']); - $matchesexcept = isset($fragment['except']) && $this->matchesPrefilterVariantRules($fragment['except']); + $matches = true; - if (!$failsonly && !$matchesexcept) $matchingFragments[] = $fragment; + if (isset($fragment['only'])) { + $matches = $matches && ($this->matchesPrefilterVariantRules($fragment['only']) !== false); + } + + if (isset($fragment['except'])) { + $matches = $matches && ($this->matchesPrefilterVariantRules($fragment['except']) !== true); + } + + if ($matches) $matchingFragments[] = $fragment; } $this->yamlConfigFragments = $matchingFragments; @@ -413,22 +465,26 @@ class SS_ConfigManifest { * which values means accept or reject a fragment */ public function matchesPrefilterVariantRules($rules) { + $matches = "undefined"; // Needs to be truthy, but not true + foreach ($rules as $k => $v) { switch (strtolower($k)) { case 'classexists': - if (!ClassInfo::exists($v)) return false; + $matches = $matches && ClassInfo::exists($v); break; case 'moduleexists': - if (!$this->moduleExists($v)) return false; + $matches = $matches && $this->moduleExists($v); break; default: // NOP } + + if ($matches === false) return $matches; } - return true; + return $matches; } /** @@ -481,26 +537,61 @@ class SS_ConfigManifest { /** * Calculates which yaml config fragments are applicable in this variant, and merge those all together into * the $this->yamlConfig propperty + * + * Checks cache and takes care of loading yamlConfigFragments if they aren't already present, but expects + * $variantKeySpec to already be set */ public function buildYamlConfigVariant($cache = true) { + // Only try loading from cache if we don't have the fragments already loaded, as there's no way to know if a + // given variant is stale compared to the complete set of fragments + if (!$this->yamlConfigFragments) { + // First try and just load the exact variant + if ($this->yamlConfig = $this->cache->load($this->key.'yaml_config_'.$this->variantKey())) { + $this->yamlConfigVariantKey = $this->variantKey(); + return; + } + // Otherwise try and load the fragments so we can build the variant + else { + $this->yamlConfigFragments = $this->cache->load($this->key.'yaml_config_fragments'); + } + } + + // If we still don't have any fragments we have to build them + if (!$this->yamlConfigFragments) { + $this->regenerate($this->includeTests, $cache); + } + $this->yamlConfig = array(); + $this->yamlConfigVariantKey = $this->variantKey(); foreach ($this->yamlConfigFragments as $i => $fragment) { - $failsonly = isset($fragment['only']) && !$this->matchesVariantRules($fragment['only']); - $matchesexcept = isset($fragment['except']) && $this->matchesVariantRules($fragment['except']); + $matches = true; - if (!$failsonly && !$matchesexcept) $this->mergeInYamlFragment($this->yamlConfig, $fragment['fragment']); + if (isset($fragment['only'])) { + $matches = $matches && ($this->matchesVariantRules($fragment['only']) !== false); + } + + if (isset($fragment['except'])) { + $matches = $matches && ($this->matchesVariantRules($fragment['except']) !== true); + } + + if ($matches) $this->mergeInYamlFragment($this->yamlConfig, $fragment['fragment']); } if ($cache) { - $this->cache->save($this->yamlConfig, 'yaml_config_'.$this->variantKey()); + $this->cache->save($this->yamlConfig, $this->key.'yaml_config_'.$this->variantKey()); } + + // Since yamlConfig has changed, call any callbacks that are interested + foreach ($this->configChangeCallbacks as $callback) call_user_func($callback); } /** * Returns false if the non-prefilterable parts of the rule aren't met, and true if they are */ public function matchesVariantRules($rules) { + $matches = "undefined"; // Needs to be truthy, but not true + foreach ($rules as $k => $v) { switch (strtolower($k)) { case 'classexists': @@ -510,13 +601,13 @@ class SS_ConfigManifest { case 'environment': switch (strtolower($v)) { case 'live': - if (!Director::isLive()) return false; + $matches = $matches && Director::isLive(); break; case 'test': - if (!Director::isTest()) return false; + $matches = $matches && Director::isTest(); break; case 'dev': - if (!Director::isDev()) return false; + $matches = $matches && Director::isDev(); break; default: user_error('Unknown environment '.$v.' in config fragment', E_USER_ERROR); @@ -524,21 +615,25 @@ class SS_ConfigManifest { break; case 'envvarset': - if (isset($_ENV[$k])) break; - return false; + $matches = $matches && isset($_ENV[$v]); + break; case 'constantdefined': - if (defined($k)) break; - return false; + $matches = $matches && defined($v); + break; default: - if (isset($_ENV[$k]) && $_ENV[$k] == $v) break; - if (defined($k) && constant($k) == $v) break; - return false; + $matches = $matches && ( + (isset($_ENV[$k]) && $_ENV[$k] == $v) || + (defined($k) && constant($k) == $v) + ); + break; } + + if ($matches === false) return $matches; } - return true; + return $matches; } /** diff --git a/core/manifest/ConfigStaticManifest.php b/core/manifest/ConfigStaticManifest.php index a972e88c4..5dddd9266 100644 --- a/core/manifest/ConfigStaticManifest.php +++ b/core/manifest/ConfigStaticManifest.php @@ -78,7 +78,8 @@ class SS_ConfigStaticManifest { $static = $this->statics[$class][$name]; if ($static['access'] != T_PRIVATE) { - Deprecation::notice('3.2.0', "Config static $class::\$$name must be marked as private", Deprecation::SCOPE_GLOBAL); + Deprecation::notice('3.2.0', "Config static $class::\$$name must be marked as private", + Deprecation::SCOPE_GLOBAL); // Don't warn more than once per static $this->statics[$class][$name]['access'] = T_PRIVATE; } @@ -211,13 +212,23 @@ class SS_ConfigStaticManifest_Parser { $class = $next[1]; } else if($type == T_NAMESPACE) { - $next = $this->next(); + $namespace = ''; + while(true) { + $next = $this->next(); - if($next[0] != T_STRING) { - user_error("Couldn\'t parse {$this->path} when building config static manifest", E_USER_ERROR); + if($next == ';') { + break; + } elseif($next[0] == T_NS_SEPARATOR) { + $namespace .= $next[1]; + $next = $this->next(); + } + + if($next[0] != T_STRING) { + user_error("Couldn\'t parse {$this->path} when building config static manifest", E_USER_ERROR); + } + + $namespace .= $next[1]; } - - $namespace = $next[1]; } else if($type == '{' || $type == T_CURLY_OPEN || $type == T_DOLLAR_OPEN_CURLY_BRACES){ $depth += 1; @@ -288,7 +299,8 @@ class SS_ConfigStaticManifest_Parser { $depth -= 1; } - // Parse out the assignment side of a static declaration, ending on either a ';' or a ',' outside an array + // Parse out the assignment side of a static declaration, + // ending on either a ';' or a ',' outside an array if($type == T_WHITESPACE) { $value .= ' '; } diff --git a/css/UploadField.css b/css/UploadField.css index 050a051db..5d6468336 100644 --- a/css/UploadField.css +++ b/css/UploadField.css @@ -43,4 +43,4 @@ Used in side panels and action tabs .ss-uploadfield .ss-uploadfield-addfile.borderTop { border-top: 1px solid #b3b3b3; } .ss-upload .clear { clear: both; } -.ss-upload .ss-uploadfield-fromcomputer input { /* since we can't really style the file input, we use this hack to make it as big as the button and hide it */ position: absolute; top: 0; right: 0; margin: 0; border: solid #000; border-width: 0 0 100px 200px; opacity: 0; filter: alpha(opacity=0); -o-transform: translate(250px, -50px) scale(1); -moz-transform: translate(-300px, 0) scale(4); direction: ltr; cursor: pointer; } +.ss-upload .ss-uploadfield-fromcomputer input { /* since we can't really style the file input, we use this hack to make it as big as the button and hide it */ position: absolute; top: 0; right: 0; margin: 0; opacity: 0; filter: alpha(opacity=0); transform: translate(-300px, 0) scale(4); font-size: 23px; direction: ltr; cursor: pointer; height: 30px; line-height: 30px; } diff --git a/dev/TestRunner.php b/dev/TestRunner.php old mode 100644 new mode 100755 index e9572b274..786499de4 --- a/dev/TestRunner.php +++ b/dev/TestRunner.php @@ -325,6 +325,12 @@ class TestRunner extends Controller { $phpunitwrapper->setSuite($suite); $phpunitwrapper->setCoverageStatus($coverage); + // Make sure TearDown is called (even in the case of a fatal error) + $self = $this; + register_shutdown_function(function() use ($self) { + $self->tearDown(); + }); + $phpunitwrapper->runTests(); // get results of the PhpUnitWrapper class diff --git a/docs/en/changelogs/3.1.0.md b/docs/en/changelogs/3.1.0.md index 2030715e3..8795f02e5 100644 --- a/docs/en/changelogs/3.1.0.md +++ b/docs/en/changelogs/3.1.0.md @@ -35,6 +35,7 @@ * Optional integration with ImageMagick as a new image manipulation backend * Support for PHP 5.4's built-in webserver * Support for [Composer](http://getcomposer.org) dependency manager (also works with 3.0) + * Added support for filtering incoming HTML from TinyMCE (disabled by default, see [security](/topics/security)) ## Upgrading @@ -219,8 +220,7 @@ For more information about how to use the config system, see the ["Configuration In order to make controller access checks more consistent and easier to understand, the routing will require definition of `$allowed_actions` -on your own `Controller` subclasses if they contain any actions -accessible through URLs, or any forms. +on your own `Controller` subclasses if they contain any actions accessible through URLs. :::php class MyController extends Controller { @@ -228,8 +228,13 @@ accessible through URLs, or any forms. public function myaction($request) {} } -Please review all rules governing allowed actions in the -["controller" topic](/topics/controller). +You can overwrite the default behaviour on undefined `$allowed_actions` to allow all actions, +by setting the `RequestHandler.require_allowed_actions` config value to `false` (not recommended). + +This applies to anything extending `RequestHandler`, so please check your `Form` and `FormField` +subclasses as well. Keep in mind, action methods as denoted through `FormAction` names should NOT +be mentioned in `$allowed_actions` to avoid CSRF issues. +Please review all rules governing allowed actions in the ["controller" topic](/topics/controller). ### Removed support for "*" rules in `Controller::$allowed_actions` diff --git a/docs/en/howto/simple-contact-form.md b/docs/en/howto/simple-contact-form.md index 1c63dfcaa..f77fb1b5d 100644 --- a/docs/en/howto/simple-contact-form.md +++ b/docs/en/howto/simple-contact-form.md @@ -44,7 +44,7 @@ First we create all the fields we want in the contact form, and put them inside We then create a `[api:FieldList]` of the form actions, or the buttons that submit the form. Here we add a single form action, with the name 'submit', and the label 'Submit'. We'll use the name of the form action later. :::php - return new Form('Form', $this, $fields, $actions); + return new Form($this, 'Form', $fields, $actions); Finally we create the `Form` object and return it. The first argument is the name of the form – this has to be the same as the name of the function that creates the form, so we've used 'Form'. The second argument is the controller that the form is on – this is almost always $this. The third and fourth arguments are the fields and actions we created earlier. diff --git a/docs/en/misc/contributing/code.md b/docs/en/misc/contributing/code.md index a777c49a8..3a96989a8 100644 --- a/docs/en/misc/contributing/code.md +++ b/docs/en/misc/contributing/code.md @@ -85,6 +85,25 @@ there are any problems they will follow up with you, so please ensure they have ![Workflow diagram](http://www.silverstripe.org/assets/doc-silverstripe-org/collaboration-on-github.png) +### Quickfire Do's and Don't's + +If you aren't familiar with git and GitHub, try reading the ["GitHub bootcamp documentation"](http://help.github.com/). +We also found the [free online git book](http://progit.org/book/) and the [git crash course](http://gitref.org/) useful. +If you're familiar with it, here's the short version of what you need to know. Once you fork and download the code: + + * **Don't develop on the master branch.** Always create a development branch specific to "the issue" you're working on (mostly on our [bugtracker](/misc/contributing/issues)). Name it by issue number and description. For example, if you're working on Issue #100, a `DataObject::get_one()` bugfix, your development branch should be called 100-dataobject-get-one. If you decide to work on another issue mid-stream, create a new branch for that issue--don't work on both in one branch. + + * **Do not merge the upstream master** with your development branch; *rebase* your branch on top of the upstream master. + + * **A single development branch should represent changes related to a single issue.** If you decide to work on another issue, create another branch. + + * **Squash your commits, so that each commit addresses a single issue.** After you rebase your work on top of the upstream master, you can squash multiple commits into one. Say, for instance, you've got three commits in related to Issue #100. Squash all three into one with the message "Issue #100 Description of the issue here." We won't accept pull requests for multiple commits related to a single issue; it's up to you to squash and clean your commit tree. (Remember, if you squash commits you've already pushed to GitHub, you won't be able to push that same branch again. Create a new local branch, squash, and push the new squashed branch.) + + * **Choose the correct branch**: Assume the current release is 3.0.3, and 3.1.0 is in beta state. + Most pull requests should go against the `3.1.x-dev` *pre-release branch*, only critical bugfixes + against the `3.0.x-dev` *release branch*. If you're changing an API or introducing a major feature, + the pull request should go against `master` (read more about our [release process](/misc/release-process)). Branches are periodically merged "upwards" (3.0 into 3.1, 3.1 into master). + ### Editing files directly on GitHub.com If you see a typo or another small fix that needs to be made, and you don't have an installation set up for contributions, you can edit files directly in the github.com web interface. Every file view has an "edit this file" link. diff --git a/docs/en/reference/grid-field.md b/docs/en/reference/grid-field.md index 3b30f6bc8..06326568b 100644 --- a/docs/en/reference/grid-field.md +++ b/docs/en/reference/grid-field.md @@ -1,7 +1,7 @@ # GridField -GridField is SilverStripe's implementation of data grids. Its main purpose is to display tabular data -in a format that is easy to view and modify. It's a can be thought of as a HTML table with some tricks. +Gridfield is SilverStripe's implementation of data grids. Its main purpose is to display tabular data +in a format that is easy to view and modify. It can be thought of as a HTML table with some tricks. It's built in a way that provides developers with an extensible way to display tabular data in a table and minimise the amount of code that needs to be written. diff --git a/docs/en/reference/templates.md b/docs/en/reference/templates.md index 9e1120ca4..99287e312 100644 --- a/docs/en/reference/templates.md +++ b/docs/en/reference/templates.md @@ -173,7 +173,7 @@ the markup in the `else` clause is used, if that clause is present. :::ss <% if $MyDinner=="quiche" %> Real men don't eat quiche - <% else_if $MyDinner=$YourDinner %> + <% else_if $MyDinner==$YourDinner %> We both have good taste <% else %> Can I have some of your chips? diff --git a/docs/en/topics/configuration.md b/docs/en/topics/configuration.md index 95b77fc09..f0dc40073 100644 --- a/docs/en/topics/configuration.md +++ b/docs/en/topics/configuration.md @@ -227,8 +227,8 @@ To accommodate this, value sections can be filtered to only be used when either current environment. To achieve this you add a key to the related header section, either "Only" when the value section should be included -only when the rules contained match, or "Except" when the value section should be included except when the rules -contained match. +only when all the rules contained match, or "Except" when the value section should be included except when all of the +rules contained match. You then list any of the following rules as sub-keys, with informational values as either a single value or a list. @@ -256,6 +256,15 @@ For instance, to add a property to "foo" when a module exists, and "bar" otherwi property: 'bar' --- +Note than when you have more than one rule for a nested fragment, they're joined like + + FRAGMENT_INCLUDED = (ONLY && ONLY) && !(EXCEPT && EXCEPT) + +That is, the fragment will be included if all Only rules match, except if all Except rules match. + +Also, due to YAML limitations, having multiple conditions of the same kind (say, two `EnvVarSet` in one "Only" block) +will result in only the latter coming through. + ### The values The values section of YAML configuration files is quite simple - it is simply a nested key / value pair structure diff --git a/docs/en/topics/controller.md b/docs/en/topics/controller.md index 81880c948..a8c90ddc2 100644 --- a/docs/en/topics/controller.md +++ b/docs/en/topics/controller.md @@ -87,9 +87,7 @@ way to perform checks against permission codes or custom logic. There's a couple of rules guiding these checks: * Each class is only responsible for access control on the methods it defines - * If `$allowed_actions` is defined as an empty array, no actions are allowed - * If `$allowed_actions` is undefined, all public methods on the specific class are allowed - (not recommended) + * If `$allowed_actions` is an empty array or undefined, only the `index` action is allowed * Access checks on parent classes need to be overwritten via the Config API * Only public methods can be made accessible * If a method on a parent class is overwritten, access control for it has to be redefined as well @@ -102,6 +100,8 @@ There's a couple of rules guiding these checks: * `$allowed_actions` can be defined on `Extension` classes applying to the controller. If the permission check fails, SilverStripe will return a "403 Forbidden" HTTP status. +You can overwrite the default behaviour on undefined `$allowed_actions` to allow all actions, +by setting the `RequestHandler.require_allowed_actions` config value to `false` (not recommended). ### Through the action @@ -173,21 +173,6 @@ through `/fastfood/drivethrough/` to use the same order function. 'drivethrough/$Action/$ID/$Name' => 'order' ); -## Access Control - -### Through $allowed_actions - - * If `$allowed_actions` is undefined, `null` or `array()`, no actions are accessible - * Each class is only responsible for access control on the methods it defines - * Access checks on parent classes need to be overwritten via the Config API - * Only public methods can be made accessible - * If a method on a parent class is overwritten, access control for it has to be redefined as well - * An action named "index" is whitelisted by default - * Methods returning forms also count as actions which need to be defined - * Form action methods (targets of `FormAction`) should NOT be included in `$allowed_actions`, - they're handled separately through the form routing (see the ["forms" topic](/topics/forms)) - * `$allowed_actions` can be defined on `Extension` classes applying to the controller. - ## URL Patterns The `[api:RequestHandler]` class will parse all rules you specify against the diff --git a/docs/en/topics/security.md b/docs/en/topics/security.md index 2d711ba80..acc9ba89a 100644 --- a/docs/en/topics/security.md +++ b/docs/en/topics/security.md @@ -272,6 +272,15 @@ Some rules of thumb: * Don't concatenate URLs in a template. It only works in extremely simple cases that usually contain bugs. * Use *Controller::join_links()* to concatenate URLs. It deals with query strings and other such edge cases. +### Filtering incoming HTML from TinyMCE + +In some cases you may be particularly concerned about which HTML elements are addable to Content via the CMS. +By default, although TinyMCE is configured to restrict some dangerous tags (such as `script` tags), this restriction +is not enforced server-side. A malicious user with write access to the CMS might create a specific request to avoid +these restrictions. + +To enable server side filtering using the same whitelisting controls as TinyMCE, set the +HtmlEditorField::$sanitise_server_side config property to true. ## Cross-Site Request Forgery (CSRF) diff --git a/docs/en/topics/testing/index.md b/docs/en/topics/testing/index.md index ec29bb2ca..9f2053d4b 100644 --- a/docs/en/topics/testing/index.md +++ b/docs/en/topics/testing/index.md @@ -56,16 +56,16 @@ The `phpunit` binary should be used from the root directory of your website. # Runs all tests defined in phpunit.xml phpunit - + # Run all tests of a specific module phpunit framework/tests/ - + # Run specific tests within a specific module phpunit framework/tests/filesystem - + # Run a specific test phpunit framework/tests/filesystem/FolderTest.php - + # Run tests with optional `$_GET` parameters (you need an empty second argument) phpunit framework/tests '' flush=all @@ -81,16 +81,16 @@ particularly around formatting test output. # Run all tests sake dev/tests/all - + # Run all tests of a specific module (comma-separated) sake dev/tests/module/framework,cms - + # Run specific tests (comma-separated) sake dev/tests/FolderTest,OtherTest - + # Run tests with optional `$_GET` parameters sake dev/tests/all flush=all - + # Skip some tests sake dev/tests/all SkipTests=MySkippedTest @@ -187,4 +187,4 @@ understand the problem space and discover suitable APIs for performing specific **Behavior Driven Development (BDD):** An extension of the test-driven programming style, where tests are used primarily for describing the specification of how code should perform. In practice, there's little or no technical difference - it all comes down to language. In BDD, the usual terminology is changed to reflect this change of focus, so *Specification* -is used in place of *Test Case*, and *should* is used in place of *expect* and *assert*. \ No newline at end of file +is used in place of *Test Case*, and *should* is used in place of *expect* and *assert*. diff --git a/forms/ConfirmedPasswordField.php b/forms/ConfirmedPasswordField.php index 7cebfa134..bf24434a3 100644 --- a/forms/ConfirmedPasswordField.php +++ b/forms/ConfirmedPasswordField.php @@ -247,7 +247,10 @@ class ConfirmedPasswordField extends FormField { * * @return ConfirmedPasswordField */ - public function setValue($value) { + public function setValue($value, $data = null) { + // If $data is a DataObject, don't use the value, since it's a hashed value + if ($data && $data instanceof DataObject) $value = ''; + if(is_array($value)) { if($value['_Password'] || (!$value['_Password'] && !$this->canBeEmpty)) { $this->value = $value['_Password']; diff --git a/forms/Form.php b/forms/Form.php index 5dd6c3be6..e914458e6 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -149,6 +149,12 @@ class Form extends RequestHandler { */ protected $attributes = array(); + private static $allowed_actions = array( + 'handleField', + 'httpSubmission', + 'forTemplate', + ); + /** * @var FormTemplateHelper */ @@ -223,7 +229,7 @@ class Form extends RequestHandler { 'GET ' => 'httpSubmission', 'HEAD ' => 'httpSubmission', ); - + /** * Set up current form errors in session to * the current form if appropriate. @@ -313,7 +319,7 @@ class Form extends RequestHandler { Form::set_current_action($funcName); $this->setButtonClicked($funcName); } - + // Permission checks (first on controller, then falling back to form) if( // Ensure that the action is actually a button or method on the form, @@ -375,6 +381,19 @@ class Form extends RequestHandler { return $this->httpError(404); } + public function checkAccessAction($action) { + return ( + parent::checkAccessAction($action) + // Always allow actions which map to buttons. See httpSubmission() for further access checks. + || $this->actions->dataFieldByName('action_' . $action) + // Always allow actions on fields + || ( + $field = $this->checkFieldsForAction($this->Fields(), $action) + && $field->checkAccessAction($action) + ) + ); + } + /** * Returns the appropriate response up the controller chain * if {@link validate()} fails (which is checked prior to executing any form actions). @@ -427,7 +446,7 @@ class Form extends RequestHandler { if($field = $this->checkFieldsForAction($field->FieldList(), $funcName)) { return $field; } - } elseif ($field->hasMethod($funcName)) { + } elseif ($field->hasMethod($funcName) && $field->checkAccessAction($funcName)) { return $field; } } diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php index 337a703bf..bc8f88173 100644 --- a/forms/HtmlEditorField.php +++ b/forms/HtmlEditorField.php @@ -14,6 +14,18 @@ class HtmlEditorField extends TextareaField { */ private static $use_gzip = true; + /** + * @config + * @var Integer Default insertion width for Images and Media + */ + private static $insert_width = 600; + + /** + * @config + * @var bool Should we check the valid_elements (& extended_valid_elements) rules from HtmlEditorConfig server side? + */ + private static $sanitise_server_side = false; + protected $rows = 30; /** @@ -105,7 +117,12 @@ class HtmlEditorField extends TextareaField { $linkedFiles = array(); $htmlValue = Injector::inst()->create('HTMLValue', $this->value); - + + if($this->config()->sanitise_server_side) { + $santiser = Injector::inst()->create('HtmlEditorSanitiser', HtmlEditorConfig::get_active()); + $santiser->sanitise($htmlValue); + } + if(class_exists('SiteTree')) { // Populate link tracking for internal links & links to asset files. if($links = $htmlValue->getElementsByTagName('a')) foreach($links as $link) { @@ -428,8 +445,6 @@ class HtmlEditorField_Toolbar extends RequestHandler { $computerUploadField->removeExtraClass('ss-uploadfield'); $computerUploadField->setTemplate('HtmlEditorField_UploadField'); $computerUploadField->setFolderName(Config::inst()->get('Upload', 'uploads_folder')); - // @todo - Remove this once this field supports display and recovery of file upload validation errors - $computerUploadField->setOverwriteWarning(false); $tabSet = new TabSet( "MediaFormInsertMediaTabs", @@ -622,10 +637,10 @@ class HtmlEditorField_Toolbar extends RequestHandler { 'CSSClass', _t('HtmlEditorField.CSSCLASS', 'Alignment / style'), array( - 'left' => _t('HtmlEditorField.CSSCLASSLEFT', 'On the left, with text wrapping around.'), 'leftAlone' => _t('HtmlEditorField.CSSCLASSLEFTALONE', 'On the left, on its own.'), - 'right' => _t('HtmlEditorField.CSSCLASSRIGHT', 'On the right, with text wrapping around.'), 'center' => _t('HtmlEditorField.CSSCLASSCENTER', 'Centered, on its own.'), + 'left' => _t('HtmlEditorField.CSSCLASSLEFT', 'On the left, with text wrapping around.'), + 'right' => _t('HtmlEditorField.CSSCLASSRIGHT', 'On the right, with text wrapping around.') ) )->addExtraClass('last') ); @@ -636,12 +651,12 @@ class HtmlEditorField_Toolbar extends RequestHandler { TextField::create( 'Width', _t('HtmlEditorField.IMAGEWIDTHPX', 'Width'), - $file->Width + $file->InsertWidth )->setMaxLength(5), TextField::create( 'Height', _t('HtmlEditorField.IMAGEHEIGHTPX', 'Height'), - $file->Height + $file->InsertHeight )->setMaxLength(5) )->addExtraClass('dimensions last') ); @@ -661,7 +676,7 @@ class HtmlEditorField_Toolbar extends RequestHandler { ), 'CaptionText'); } - $this->extend('updateFieldsForImage', $fields, $url, $file); + $this->extend('updateFieldsForOembed', $fields, $url, $file); return $fields; } @@ -746,10 +761,10 @@ class HtmlEditorField_Toolbar extends RequestHandler { 'CSSClass', _t('HtmlEditorField.CSSCLASS', 'Alignment / style'), array( - 'left' => _t('HtmlEditorField.CSSCLASSLEFT', 'On the left, with text wrapping around.'), 'leftAlone' => _t('HtmlEditorField.CSSCLASSLEFTALONE', 'On the left, on its own.'), - 'right' => _t('HtmlEditorField.CSSCLASSRIGHT', 'On the right, with text wrapping around.'), 'center' => _t('HtmlEditorField.CSSCLASSCENTER', 'Centered, on its own.'), + 'left' => _t('HtmlEditorField.CSSCLASSLEFT', 'On the left, with text wrapping around.'), + 'right' => _t('HtmlEditorField.CSSCLASSRIGHT', 'On the right, with text wrapping around.') ) )->addExtraClass('last') ); @@ -759,12 +774,12 @@ class HtmlEditorField_Toolbar extends RequestHandler { TextField::create( 'Width', _t('HtmlEditorField.IMAGEWIDTHPX', 'Width'), - $file->Width + $file->InsertWidth )->setMaxLength(5), TextField::create( 'Height', " x " . _t('HtmlEditorField.IMAGEHEIGHTPX', 'Height'), - $file->Height + $file->InsertHeight )->setMaxLength(5) )->addExtraClass('dimensions last') ); @@ -910,6 +925,29 @@ class HtmlEditorField_Embed extends HtmlEditorField_File { return $this->oembed->Height ?: 100; } + /** + * Provide an initial width for inserted media, restricted based on $embed_width + * + * @return int + */ + public function getInsertWidth() { + $width = $this->getWidth(); + $maxWidth = Config::inst()->get('HtmlEditorField', 'insert_width'); + return ($width <= $maxWidth) ? $width : $maxWidth; + } + + /** + * Provide an initial height for inserted media, scaled proportionally to the initial width + * + * @return int + */ + public function getInsertHeight() { + $width = $this->getWidth(); + $height = $this->getHeight(); + $maxWidth = Config::inst()->get('HtmlEditorField', 'insert_width'); + return ($width <= $maxWidth) ? $height : round($height*($maxWidth/$width)); + } + public function getPreview() { if(isset($this->oembed->thumbnail_url)) { return sprintf('', $this->oembed->thumbnail_url); @@ -966,6 +1004,29 @@ class HtmlEditorField_Image extends HtmlEditorField_File { return ($this->file) ? $this->file->Height : $this->height; } + /** + * Provide an initial width for inserted image, restricted based on $embed_width + * + * @return int + */ + public function getInsertWidth() { + $width = $this->getWidth(); + $maxWidth = Config::inst()->get('HtmlEditorField', 'insert_width'); + return ($width <= $maxWidth) ? $width : $maxWidth; + } + + /** + * Provide an initial height for inserted image, scaled proportionally to the initial width + * + * @return int + */ + public function getInsertHeight() { + $width = $this->getWidth(); + $height = $this->getHeight(); + $maxWidth = Config::inst()->get('HtmlEditorField', 'insert_width'); + return ($width <= $maxWidth) ? $height : round($height*($maxWidth/$width)); + } + public function getPreview() { return ($this->file) ? $this->file->CMSThumbnail() : sprintf('', $this->url); } diff --git a/forms/HtmlEditorSanitiser.php b/forms/HtmlEditorSanitiser.php new file mode 100644 index 000000000..98b78faf7 --- /dev/null +++ b/forms/HtmlEditorSanitiser.php @@ -0,0 +1,304 @@ + $rule hash for whitelist element rules where the element name isn't a pattern */ + protected $elements = array(); + /** @var [stdClass] - Sequential list of whitelist element rules where the element name is a pattern */ + protected $elementPatterns = array(); + + /** @var [stdClass] - The list of attributes that apply to all further whitelisted elements added */ + protected $globalAttributes = array(); + + /** + * Construct a sanitiser from a given HtmlEditorConfig + * + * Note that we build data structures from the current state of HtmlEditorConfig - later changes to + * the passed instance won't cause this instance to update it's whitelist + * + * @param HtmlEditorConfig $config + */ + public function __construct(HtmlEditorConfig $config) { + $valid = $config->getOption('valid_elements'); + if ($valid) $this->addValidElements($valid); + + $valid = $config->getOption('extended_valid_elements'); + if ($valid) $this->addValidElements($valid); + } + + /** + * Given a TinyMCE pattern (close to unix glob style), create a regex that does the match + * + * @param $str - The TinyMCE pattern + * @return string - The equivalent regex + */ + protected function patternToRegex($str) { + return '/^' . preg_replace('/([?+*])/', '.$1', $str) . '$/'; + } + + /** + * Given a valid_elements string, parse out the actual element and attribute rules and add to the + * internal whitelist + * + * Logic based heavily on javascript version from tiny_mce_src.js + * + * @param string $validElements - The valid_elements or extended_valid_elements string to add to the whitelist + */ + protected function addValidElements($validElements) { + $elementRuleRegExp = '/^([#+\-])?([^\[\/]+)(?:\/([^\[]+))?(?:\[([^\]]+)\])?$/'; + $attrRuleRegExp = '/^([!\-])?(\w+::\w+|[^=:<]+)?(?:([=:<])(.*))?$/'; + $hasPatternsRegExp = '/[*?+]/'; + + foreach(explode(',', $validElements) as $validElement) { + if(preg_match($elementRuleRegExp, $validElement, $matches)) { + + $prefix = @$matches[1]; + $elementName = @$matches[2]; + $outputName = @$matches[3]; + $attrData = @$matches[4]; + + // Create the new element + $element = new stdClass(); + $element->attributes = array(); + $element->attributePatterns = array(); + + $element->attributesRequired = array(); + $element->attributesDefault = array(); + $element->attributesForced = array(); + + foreach(array('#' => 'paddEmpty', '-' => 'removeEmpty') as $match => $means) { + $element->$means = ($prefix === $match); + } + + // Copy attributes from global rule into current rule + if($this->globalAttributes) { + $element->attributes = array_merge($element->attributes, $this->globalAttributes); + } + + // Attributes defined + if($attrData) { + foreach(explode('|', $attrData) as $attr) { + if(preg_match($attrRuleRegExp, $attr, $matches)) { + $attr = new stdClass(); + + $attrType = @$matches[1]; + $attrName = str_replace('::', ':', @$matches[2]); + $prefix = @$matches[3]; + $value = @$matches[4]; + + // Required + if($attrType === '!') { + $element->attributesRequired[] = $attrName; + $attr->required = true; + } + + // Denied from global + else if($attrType === '-') { + unset($element->attributes[$attrName]); + continue; + } + + // Default value + if($prefix) { + // Default value + if($prefix === '=') { + $element->attributesDefault[$attrName] = $value; + $attr->defaultValue = $value; + } + + // Forced value + else if($prefix === ':') { + $element->attributesForced[$attrName] = $value; + $attr->forcedValue = $value; + } + + // Required values + else if($prefix === '<') { + $attr->validValues = explode('?', $value); + } + } + + // Check for attribute patterns + if(preg_match($hasPatternsRegExp, $attrName)) { + $attr->pattern = $this->patternToRegex($attrName); + $element->attributePatterns[] = $attr; + } + else { + $element->attributes[$attrName] = $attr; + } + } + } + } + + // Global rule, store away these for later usage + if(!$this->globalAttributes && $elementName == '@') { + $this->globalAttributes = $element->attributes; + } + + // Handle substitute elements such as b/strong + if($outputName) { + $element->outputName = $elementName; + $this->elements[$outputName] = $element; + } + + // Add pattern or exact element + if(preg_match($hasPatternsRegExp, $elementName)) { + $element->pattern = $this->patternToRegex($elementName); + $this->elementPatterns[] = $element; + } + else { + $this->elements[$elementName] = $element; + } + } + } + } + + /** + * Given an element tag, return the rule structure for that element + * @param string $tag - The element tag + * @return stdClass - The element rule + */ + protected function getRuleForElement($tag) { + if(isset($this->elements[$tag])) { + return $this->elements[$tag]; + } + else foreach($this->elementPatterns as $element) { + if(preg_match($element->pattern, $tag)) return $element; + } + } + + /** + * Given an attribute name, return the rule structure for that attribute + * @param string $name - The attribute name + * @return stdClass - The attribute rule + */ + protected function getRuleForAttribute($elementRule, $name) { + if(isset($elementRule->attributes[$name])) { + return $elementRule->attributes[$name]; + } + else foreach($elementRule->attributePatterns as $attribute) { + if(preg_match($attribute->pattern, $name)) return $attribute; + } + } + + /** + * Given a DOMElement and an element rule, check if that element passes the rule + * @param DOMElement $element - the element to check + * @param stdClass $rule - the rule to check against + * @return bool - true if the element passes (and so can be kept), false if it fails (and so needs stripping) + */ + protected function elementMatchesRule($element, $rule = null) { + // If the rule doesn't exist at all, the element isn't allowed + if(!$rule) return false; + + // If the rule has attributes required, check them to see if this element has at least one + if($rule->attributesRequired) { + $hasMatch = false; + + foreach($rule->attributesRequired as $attr) { + if($element->getAttribute($attr)) { + $hasMatch = true; + break; + } + } + + if(!$hasMatch) return false; + } + + // If the rule says to remove empty elements, and this element is empty, remove it + if($rule->removeEmpty && !$element->firstChild) return false; + + // No further tests required, element passes + return true; + } + + /** + * Given a DOMAttr and an attribute rule, check if that attribute passes the rule + * @param DOMAttr $attr - the attribute to check + * @param stdClass $rule - the rule to check against + * @return bool - true if the attribute passes (and so can be kept), false if it fails (and so needs stripping) + */ + protected function attributeMatchesRule($attr, $rule = null) { + // If the rule doesn't exist at all, the attribute isn't allowed + if(!$rule) return false; + + // If the rule has a set of valid values, check them to see if this attribute is one + if(isset($rule->validValues) && !in_array($attr->value, $rule->validValues)) return false; + + // No further tests required, attribute passes + return true; + } + + /** + * Given an SS_HTMLValue instance, will remove and elements and attributes that are + * not explicitly included in the whitelist passed to __construct on instance creation + * + * @param SS_HTMLValue $html - The HTMLValue to remove any non-whitelisted elements & attributes from + */ + public function sanitise (SS_HTMLValue $html) { + if(!$this->elements && !$this->elementPatterns) return; + + $doc = $html->getDocument(); + + foreach($html->query('//body//*') as $el) { + $elementRule = $this->getRuleForElement($el->tagName); + + // If this element isn't allowed, strip it + if(!$this->elementMatchesRule($el, $elementRule)) { + // If it's a script or style, we don't keep contents + if($el->tagName === 'script' || $el->tagName === 'style') { + $el->parentNode->removeChild($el); + } + // Otherwise we replace this node with all it's children + else { + // First, create a new fragment with all of $el's children moved into it + $frag = $doc->createDocumentFragment(); + while($el->firstChild) $frag->appendChild($el->firstChild); + + // Then replace $el with the frags contents (which used to be it's children) + $el->parentNode->replaceChild($frag, $el); + } + } + // Otherwise tidy the element + else { + // First, if we're supposed to pad & this element is empty, fix that + if($elementRule->paddEmpty && !$el->firstChild) { + $el->nodeValue = ' '; + } + + // Then filter out any non-whitelisted attributes + $children = $el->attributes; + $i = $children->length; + while($i--) { + $attr = $children->item($i); + $attributeRule = $this->getRuleForAttribute($elementRule, $attr->name); + + // If this attribute isn't allowed, strip it + if(!$this->attributeMatchesRule($attr, $attributeRule)) { + $el->removeAttributeNode($attr); + } + } + + // Then enforce any default attributes + foreach($elementRule->attributesDefault as $attr => $default) { + if(!$el->getAttribute($attr)) $el->setAttribute($attr, $default); + } + + // And any forced attributes + foreach($elementRule->attributesForced as $attr => $forced) { + $el->setAttribute($attr, $forced); + } + } + } + } + +} diff --git a/forms/UploadField.php b/forms/UploadField.php index 0c6e668ad..a3d265682 100644 --- a/forms/UploadField.php +++ b/forms/UploadField.php @@ -1331,6 +1331,12 @@ class UploadField_ItemHandler extends RequestHandler { '' => 'index', ); + private static $allowed_actions = array( + 'delete', + 'edit', + 'EditForm', + ); + /** * @param UploadFIeld $parent * @param int $item @@ -1484,6 +1490,10 @@ class UploadField_SelectHandler extends RequestHandler { '' => 'index', ); + private static $allowed_actions = array( + 'Form' + ); + public function __construct($parent, $folderName = null) { $this->parent = $parent; $this->folderName = $folderName; diff --git a/forms/gridfield/GridFieldDetailForm.php b/forms/gridfield/GridFieldDetailForm.php index 6a6dd89f5..0c7dc2187 100644 --- a/forms/gridfield/GridFieldDetailForm.php +++ b/forms/gridfield/GridFieldDetailForm.php @@ -194,6 +194,12 @@ class GridFieldDetailForm implements GridField_URLHandler { * @subpackage fields-gridfield */ class GridFieldDetailForm_ItemRequest extends RequestHandler { + + private static $allowed_actions = array( + 'edit', + 'view', + 'ItemEditForm' + ); /** * diff --git a/javascript/HtmlEditorField.js b/javascript/HtmlEditorField.js index e34148a7a..3de5c67a9 100644 --- a/javascript/HtmlEditorField.js +++ b/javascript/HtmlEditorField.js @@ -382,7 +382,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; success: function(html) { dialog.html(html); dialog.getForm().setElement(self); - dialog.trigger('dialogopen'); + dialog.trigger('ssdialogopen'); } }); } @@ -452,7 +452,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; }, fromDialog: { - ondialogopen: function(){ + onssdialogopen: function(){ var ed = this.getEditor(); ed.onopen(); @@ -467,7 +467,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; this.redraw(); }, - ondialogclose: function(){ + onssdialogclose: function(){ var ed = this.getEditor(); ed.onclose(); @@ -826,7 +826,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; }); ed.repaint(); - }) + }); this.getDialog().close(); return false; @@ -944,8 +944,9 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; var uploadedFiles = $('.ss-uploadfield-files', this).children('.ss-uploadfield-item'); uploadedFiles.each(function(){ var uploadedID = $(this).data('fileid'); - if ($.inArray(uploadedID, editFieldIDs) == -1) { + if (uploadedID && $.inArray(uploadedID, editFieldIDs) == -1) { //trigger the detail view for filling out details about the file we are about to insert into TinyMCE + $(this).remove(); // Remove successfully added item from the queue form.showFileView(uploadedID); } }); @@ -1271,12 +1272,6 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; this._super(); this.setOrigVal(parseInt(this.val(), 10)); - - // Default to a managable size for the HTML view. Can be overwritten by user after initialization - if(this.attr('name') == 'Width') { - this.closest('.ss-htmleditorfield-file').updateDimensions('Width', 600); - } - }, onunmatch: function() { this._super(); diff --git a/javascript/TreeDropdownField.js b/javascript/TreeDropdownField.js index d58313d72..1b46d96c6 100644 --- a/javascript/TreeDropdownField.js +++ b/javascript/TreeDropdownField.js @@ -4,8 +4,22 @@ * On resize of any close the open treedropdownfields * as we'll need to redo with widths */ - $(window).resize(function() { - $('.TreeDropdownField').closePanel(); + var windowWidth, windowHeight; + $(window).bind('resize.treedropdownfield', function() { + // Entwine's 'fromWindow::onresize' does not trigger on IE8. Use synthetic event. + var cb = function() {$('.TreeDropdownField').closePanel();}; + + // Workaround to avoid IE8 infinite loops when elements are resized as a result of this event + if($.browser.msie && parseInt($.browser.version, 10) < 9) { + var newWindowWidth = $(window).width(), newWindowHeight = $(window).height(); + if(newWindowWidth != windowWidth || newWindowHeight != windowHeight) { + windowWidth = newWindowWidth; + windowHeight = newWindowHeight; + cb(); + } + } else { + cb(); + } }); var strings = { diff --git a/javascript/UploadField.js b/javascript/UploadField.js index 98598d9ab..30f509ed6 100644 --- a/javascript/UploadField.js +++ b/javascript/UploadField.js @@ -64,13 +64,16 @@ .addClass('ui-state-warning-text'); data.context.find('.ss-uploadfield-item-progress').hide(); data.context.find('.ss-uploadfield-item-overwrite').show(); - data.context.find('.ss-uploadfield-item-overwrite-warning').on('click', function(){ + data.context.find('.ss-uploadfield-item-overwrite-warning').on('click', function(e){ data.context.find('.ss-uploadfield-item-progress').show(); data.context.find('.ss-uploadfield-item-overwrite').hide(); data.context.find('.ss-uploadfield-item-status') .removeClass('ui-state-warning-text'); //upload only if the "overwrite" button is clicked $.blueimpUI.fileupload.prototype._onSend.call(that, e, data); + + e.preventDefault(); // Avoid a form submit + return false; }); } else { //regular file upload return $.blueimpUI.fileupload.prototype._onSend.call(that, e, data); @@ -319,12 +322,14 @@ $('div.ss-upload .ss-uploadfield-startall').entwine({ onclick: function(e) { this.closest('.ss-upload').find('.ss-uploadfield-item-start button').click(); + e.preventDefault(); // Avoid a form submit return false; } }); $('div.ss-upload .ss-uploadfield-item-cancelfailed').entwine({ onclick: function(e) { this.closest('.ss-uploadfield-item').remove(); + e.preventDefault(); // Avoid a form submit return false; } }); @@ -349,6 +354,7 @@ fileupload._trigger('destroy', e, {context: item}); } + e.preventDefault(); // Avoid a form submit return false; } }); @@ -371,6 +377,7 @@ } e.preventDefault(); // Avoid a form submit + return false; } }); $( 'div.ss-upload:not(.disabled):not(.readonly) .ss-uploadfield-item-edit').entwine({ @@ -403,6 +410,7 @@ editform.toggleEditForm(); } e.preventDefault(); // Avoid a form submit + return false; } }); @@ -489,8 +497,9 @@ }); $('div.ss-upload .ss-uploadfield-fromfiles').entwine({ onclick: function(e) { - e.preventDefault(); this.getUploadField().openSelectDialog(this.closest('.ss-uploadfield-item')); + e.preventDefault(); // Avoid a form submit + return false; } }); }); diff --git a/model/DataObject.php b/model/DataObject.php index cab6b72dc..c89e550d5 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -156,6 +156,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity protected static $_cache_custom_database_fields = array(); protected static $_cache_field_labels = array(); + // base fields which are not defined in static $db + private static $fixed_fields = array( + 'ID' => 'Int', + 'ClassName' => 'Enum', + 'LastEdited' => 'SS_Datetime', + 'Created' => 'SS_Datetime', + ); + /** * Non-static relationship cache, indexed by component name. */ @@ -223,6 +231,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } return array_merge ( + // TODO: should this be using self::$fixed_fields? only difference is ID field + // and ClassName creates an Enum with all values array ( 'ClassName' => self::$classname_spec_cache[$class], 'Created' => 'SS_Datetime', @@ -1651,11 +1661,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Return all of the database fields defined in self::$db and all the parent classes. * Doesn't include any fields specified by self::$has_one. Use $this->has_one() to get these fields + * Also returns "base" fields like "Created", "LastEdited", et cetera. * * @param string $fieldName Limit the output to a specific field name * @return array The database fields */ public function db($fieldName = null) { + if ($fieldName && array_key_exists($fieldName, self::$fixed_fields)) { + return self::$fixed_fields[$fieldName]; + } + $classes = ClassInfo::ancestry($this); $good = false; $items = array(); @@ -1694,6 +1709,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } } + if (!$fieldName) { + // trying to get all fields, so add the fixed fields to return value + $items = array_merge(self::$fixed_fields, $items); + } return $items; } @@ -2387,15 +2406,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return boolean */ public function hasDatabaseField($field) { - // Add base fields which are not defined in static $db - static $fixedFields = array( - 'ID' => 'Int', - 'ClassName' => 'Enum', - 'LastEdited' => 'SS_Datetime', - 'Created' => 'SS_Datetime', - ); - - if(isset($fixedFields[$field])) return true; + if(isset(self::$fixed_fields[$field])) return true; return array_key_exists($field, $this->inheritedDatabaseFields()); } @@ -2658,9 +2669,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Special case for ID field } else if($fieldName == 'ID') { return new PrimaryKey($fieldName, $this); - - // General casting information for items in $db or $casting - } else if($helper = $this->castingHelper($fieldName)) { + + // Special case for ClassName + } else if($fieldName == 'ClassName') { + $val = get_class($this); + return DBField::create_field('Varchar', $val, $fieldName, $this); + + // General casting information for items in $db + } else if($helper = $this->db($fieldName)) { $obj = Object::create_from_string($helper, $fieldName); $obj->setValue($this->$fieldName, $this->record, false); return $obj; @@ -2669,11 +2685,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } else if(preg_match('/ID$/', $fieldName) && $this->has_one(substr($fieldName,0,-2))) { $val = $this->$fieldName; return DBField::create_field('ForeignKey', $val, $fieldName, $this); - - // Special case for ClassName - } else if($fieldName == 'ClassName') { - $val = get_class($this); - return DBField::create_field('Varchar', $val, $fieldName, $this); } } diff --git a/scss/UploadField.scss b/scss/UploadField.scss index a56a994fb..01efe3ef7 100644 --- a/scss/UploadField.scss +++ b/scss/UploadField.scss @@ -217,14 +217,14 @@ top: 0; right: 0; margin: 0; - border: solid #000; - border-width: 0 0 100px 200px; opacity: 0; filter: alpha(opacity=0); - -o-transform: translate(250px, -50px) scale(1); - -moz-transform: translate(-300px, 0) scale(4); + transform: translate(-300px, 0) scale(4); + font-size: 23px; direction: ltr; cursor: pointer; + height: 30px; + line-height: 30px; } } } \ No newline at end of file diff --git a/search/filters/SearchFilter.php b/search/filters/SearchFilter.php index 7a02b0e66..c647a3eab 100644 --- a/search/filters/SearchFilter.php +++ b/search/filters/SearchFilter.php @@ -300,4 +300,4 @@ abstract class SearchFilter extends Object { else return null; } -} \ No newline at end of file +} diff --git a/security/Security.php b/security/Security.php index f9f5af3d9..0b3741ef3 100644 --- a/security/Security.php +++ b/security/Security.php @@ -346,7 +346,9 @@ class Security extends Controller { $member = Member::currentUser(); if($member) $member->logOut(); - if($redirect) $this->redirectBack(); + if($redirect && (!$this->response || !$this->response->isFinished())) { + $this->redirectBack(); + } } diff --git a/tests/behat/features/bootstrap/SilverStripe/Framework/Test/Behaviour/CmsUiContext.php b/tests/behat/features/bootstrap/SilverStripe/Framework/Test/Behaviour/CmsUiContext.php index 55e738cc7..6c528cf9d 100644 --- a/tests/behat/features/bootstrap/SilverStripe/Framework/Test/Behaviour/CmsUiContext.php +++ b/tests/behat/features/bootstrap/SilverStripe/Framework/Test/Behaviour/CmsUiContext.php @@ -363,13 +363,13 @@ class CmsUiContext extends BehatContext } assertNotNull($container, 'Chosen.js field container not found'); - + // Click on newly expanded list element, indirectly setting the dropdown value $linkEl = $container->find('xpath', './/a[./@href]'); assertNotNull($linkEl, 'Chosen.js link element not found'); $this->getSession()->wait(100); // wait for dropdown overlay to appear $linkEl->click(); - + if(in_array('treedropdown', explode(' ', $container->getAttribute('class')))) { // wait for ajax dropdown to load $this->getSession()->wait( @@ -422,7 +422,7 @@ class CmsUiContext extends BehatContext ) { if($container->isVisible() && in_array($class, explode(' ', $container->getAttribute('class')))) { return $container; - } +} $container = $container->getParent(); } diff --git a/tests/behat/features/insert-an-image.feature b/tests/behat/features/insert-an-image.feature index 8b5614eb9..c517579a0 100644 --- a/tests/behat/features/insert-an-image.feature +++ b/tests/behat/features/insert-an-image.feature @@ -1,3 +1,4 @@ +@assets Feature: Insert an image into a page As a cms author I want to insert an image into a page diff --git a/tests/control/ControllerTest.php b/tests/control/ControllerTest.php index 40bc0ad0c..c0e9bd090 100644 --- a/tests/control/ControllerTest.php +++ b/tests/control/ControllerTest.php @@ -5,7 +5,7 @@ class ControllerTest extends FunctionalTest { protected static $fixture_file = 'ControllerTest.yml'; protected $autoFollowRedirection = false; - + protected $requiredExtensions = array( 'ControllerTest_AccessBaseController' => array( 'ControllerTest_AccessBaseControllerExtension' @@ -44,7 +44,7 @@ class ControllerTest extends FunctionalTest { public function testAllowedActions() { $adminUser = $this->objFromFixture('Member', 'admin'); - + $response = $this->get("ControllerTest_UnsecuredController/"); $this->assertEquals(200, $response->getStatusCode(), 'Access granted on index action without $allowed_actions on defining controller, ' . @@ -53,16 +53,32 @@ class ControllerTest extends FunctionalTest { $response = $this->get("ControllerTest_UnsecuredController/index"); $this->assertEquals(200, $response->getStatusCode(), - 'Access granted on index action without $allowed_actions on defining controller, ' . + 'Access denied on index action without $allowed_actions on defining controller, ' . 'when called with an action in the URL' ); - $response = $this->get("ControllerTest_UnsecuredController/method1"); + Config::inst()->update('RequestHandler', 'require_allowed_actions', false); + $response = $this->get("ControllerTest_UnsecuredController/index"); $this->assertEquals(200, $response->getStatusCode(), - 'Access granted on action without $allowed_actions on defining controller, ' . + 'Access granted on index action without $allowed_actions on defining controller, ' . + 'when called with an action in the URL, and explicitly allowed through config' + ); + Config::inst()->update('RequestHandler', 'require_allowed_actions', true); + + $response = $this->get("ControllerTest_UnsecuredController/method1"); + $this->assertEquals(403, $response->getStatusCode(), + 'Access denied on action without $allowed_actions on defining controller, ' . 'when called without an action in the URL' ); + Config::inst()->update('RequestHandler', 'require_allowed_actions', false); + $response = $this->get("ControllerTest_UnsecuredController/method1"); + $this->assertEquals(200, $response->getStatusCode(), + 'Access granted on action without $allowed_actions on defining controller, ' . + 'when called without an action in the URL, and explicitly allowed through config' + ); + Config::inst()->update('RequestHandler', 'require_allowed_actions', true); + $response = $this->get("ControllerTest_AccessBaseController/"); $this->assertEquals(200, $response->getStatusCode(), 'Access granted on index with empty $allowed_actions on defining controller, ' . @@ -110,6 +126,12 @@ class ControllerTest extends FunctionalTest { 'if action is not a method but rather a template discovered by naming convention' ); + $response = $this->get("ControllerTest_AccessSecuredController/templateaction"); + $this->assertEquals(403, $response->getStatusCode(), + 'Access denied on action with $allowed_actions on defining controller, ' . + 'if action is not a method but rather a template discovered by naming convention' + ); + $this->session()->inst_set('loggedInAs', $adminUser->ID); $response = $this->get("ControllerTest_AccessSecuredController/templateaction"); $this->assertEquals(200, $response->getStatusCode(), @@ -147,25 +169,25 @@ class ControllerTest extends FunctionalTest { "Access granted to method defined in allowed_actions on extension, " . "where method is also defined on extension" ); - + $response = $this->get('ControllerTest_AccessSecuredController/extensionmethod1'); $this->assertEquals(200, $response->getStatusCode(), "Access granted to method defined in allowed_actions on extension, " . "where method is also defined on extension, even when called in a subclass" ); - + $response = $this->get('ControllerTest_AccessBaseController/extensionmethod2'); - $this->assertEquals(404, $response->getStatusCode(), + $this->assertEquals(404, $response->getStatusCode(), "Access denied to method not defined in allowed_actions on extension, " . "where method is also defined on extension" ); - + $response = $this->get('ControllerTest_IndexSecuredController/'); $this->assertEquals(403, $response->getStatusCode(), "Access denied when index action is limited through allowed_actions, " . "and doesn't satisfy checks, and action is empty" ); - + $response = $this->get('ControllerTest_IndexSecuredController/index'); $this->assertEquals(403, $response->getStatusCode(), "Access denied when index action is limited through allowed_actions, " . @@ -174,13 +196,13 @@ class ControllerTest extends FunctionalTest { $this->session()->inst_set('loggedInAs', $adminUser->ID); $response = $this->get('ControllerTest_IndexSecuredController/'); - $this->assertEquals(200, $response->getStatusCode(), + $this->assertEquals(200, $response->getStatusCode(), "Access granted when index action is limited through allowed_actions, " . "and does satisfy checks" ); $this->session()->inst_set('loggedInAs', null); } - + /** * @expectedException PHPUnit_Framework_Error * @expectedExceptionMessage Wildcards (*) are no longer valid @@ -358,7 +380,7 @@ class ControllerTest extends FunctionalTest { class ControllerTest_Controller extends Controller implements TestOnly { public $Content = "default content"; - + private static $allowed_actions = array( 'methodaction', 'stringaction', @@ -385,13 +407,13 @@ class ControllerTest_UnsecuredController extends Controller implements TestOnly // Not defined, allow access to all // static $allowed_actions = array(); - + // Granted for all public function method1() {} // Granted for all public function method2() {} -} + } class ControllerTest_AccessBaseController extends Controller implements TestOnly { @@ -402,7 +424,7 @@ class ControllerTest_AccessBaseController extends Controller implements TestOnly // Denied for all public function method2() {} -} + } class ControllerTest_AccessSecuredController extends ControllerTest_AccessBaseController implements TestOnly { @@ -414,7 +436,7 @@ class ControllerTest_AccessSecuredController extends ControllerTest_AccessBaseCo ); public function method2() {} - + public function adminonly() {} protected function protectedmethod() {} @@ -427,18 +449,18 @@ class ControllerTest_AccessWildcardSecuredController extends ControllerTest_Acce "*" => "ADMIN", // should throw exception ); -} - + } + class ControllerTest_IndexSecuredController extends ControllerTest_AccessBaseController implements TestOnly { private static $allowed_actions = array( "index" => "ADMIN", ); -} + } class ControllerTest_AccessBaseControllerExtension extends Extension implements TestOnly { - + private static $allowed_actions = array( "extensionmethod1" => true, // granted because defined on this class "method1" => true, // ignored because method not defined on this class @@ -457,7 +479,7 @@ class ControllerTest_AccessBaseControllerExtension extends Extension implements public function internalextensionmethod() {} -} + } class ControllerTest_HasAction extends Controller { diff --git a/tests/control/DirectorTest.php b/tests/control/DirectorTest.php index 7cd8d4cca..9568307e5 100644 --- a/tests/control/DirectorTest.php +++ b/tests/control/DirectorTest.php @@ -348,6 +348,13 @@ class DirectorTest extends SapphireTest { class DirectorTestRequest_Controller extends Controller implements TestOnly { + private static $allowed_actions = array( + 'returnGetValue', + 'returnPostValue', + 'returnRequestValue', + 'returnCookieValue', + ); + public function returnGetValue($request) { return $_GET['somekey']; } public function returnPostValue($request) { return $_POST['somekey']; } diff --git a/tests/control/RequestHandlingTest.php b/tests/control/RequestHandlingTest.php index 94da0d469..0b9cdfd05 100644 --- a/tests/control/RequestHandlingTest.php +++ b/tests/control/RequestHandlingTest.php @@ -130,10 +130,6 @@ class RequestHandlingTest extends FunctionalTest { } public function testDisallowedExtendedActions() { - /* Actions on magic methods are only accessible if explicitly allowed on the controller. */ - $response = Director::test("testGoodBase1/extendedMethod"); - $this->assertEquals(404, $response->getStatusCode()); - /* Actions on an extension are allowed because they specifically provided appropriate allowed_actions items */ $response = Director::test("testGoodBase1/otherExtendedMethod"); $this->assertEquals("otherExtendedMethod", $response->getBody()); @@ -146,9 +142,10 @@ class RequestHandlingTest extends FunctionalTest { $response = Director::test("RequestHandlingTest_AllowedController/failoverMethod"); $this->assertEquals("failoverMethod", $response->getBody()); - /* The action on the extension has also been explicitly allowed even though it wasn't on the extension */ + /* The action on the extension is allowed when explicitly allowed on extension, + even if its not mentioned in controller */ $response = Director::test("RequestHandlingTest_AllowedController/extendedMethod"); - $this->assertEquals("extendedMethod", $response->getBody()); + $this->assertEquals(200, $response->getStatusCode()); /* This action has been blocked by an argument to a method */ $response = Director::test('RequestHandlingTest_AllowedController/blockMethod'); @@ -421,9 +418,13 @@ class RequestHandlingTest_FormActionController extends Controller { * Simple extension for the test controller */ class RequestHandlingTest_ControllerExtension extends Extension { + public static $called_error = false; + public static $called_404_error = false; + private static $allowed_actions = array('extendedMethod'); + public function extendedMethod() { return "extendedMethod"; } @@ -455,7 +456,6 @@ class RequestHandlingTest_AllowedController extends Controller implements TestOn private static $allowed_actions = array( 'failoverMethod', // part of the failover object - 'extendedMethod', // part of the RequestHandlingTest_ControllerExtension object 'blockMethod' => '->provideAccess(false)', 'allowMethod' => '->provideAccess', ); @@ -537,6 +537,8 @@ class RequestHandlingTest_Form extends Form { } class RequestHandlingTest_ControllerFormWithAllowedActions extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); public function Form() { return new RequestHandlingTest_FormWithAllowedActions( @@ -544,8 +546,7 @@ class RequestHandlingTest_ControllerFormWithAllowedActions extends Controller im 'Form', new FieldList(), new FieldList( - new FormAction('allowedformaction'), - new FormAction('disallowedformaction') // disallowed through $allowed_actions in form + new FormAction('allowedformaction') ) ); } @@ -555,7 +556,6 @@ class RequestHandlingTest_FormWithAllowedActions extends Form { private static $allowed_actions = array( 'allowedformaction' => 1, - 'httpSubmission' => 1, // TODO This should be an exception on the parent class ); public function allowedformaction() { @@ -603,6 +603,9 @@ class RequestHandlingTest_FormField extends FormField { * Form field for the test */ class RequestHandlingTest_SubclassedFormField extends RequestHandlingTest_FormField { + + private static $allowed_actions = array('customSomething'); + // We have some url_handlers defined that override RequestHandlingTest_FormField handlers. // We will confirm that the url_handlers inherit. private static $url_handlers = array( @@ -620,6 +623,8 @@ class RequestHandlingTest_SubclassedFormField extends RequestHandlingTest_FormFi * Controller for the test */ class RequestHandlingFieldTest_Controller extends Controller implements TestOnly { + + private static $allowed_actions = array('TestForm'); public function TestForm() { return new Form($this, "TestForm", new FieldList( @@ -642,4 +647,8 @@ class RequestHandlingTest_HandlingField extends FormField { public function actionOnField() { return "Test method on $this->name"; } + + public function actionNotAllowedOnField() { + return "actionNotAllowedOnField on $this->name"; + } } diff --git a/tests/core/manifest/ConfigManifestTest.php b/tests/core/manifest/ConfigManifestTest.php index 85800ad0e..27a2da60f 100644 --- a/tests/core/manifest/ConfigManifestTest.php +++ b/tests/core/manifest/ConfigManifestTest.php @@ -8,6 +8,174 @@ class ConfigManifestTest_ConfigManifestAccess extends SS_ConfigManifest { class ConfigManifestTest extends SapphireTest { + protected function getConfigFixtureValue($name) { + $manifest = new SS_ConfigManifest(dirname(__FILE__).'/fixtures/configmanifest', true, true); + return $manifest->get('ConfigManifestTest', $name); + } + + public function testClassRules() { + $config = $this->getConfigFixtureValue('Class'); + + $this->assertEquals( + 'Yes', @$config['DirectorExists'], + 'Only rule correctly detects existing class' + ); + + $this->assertEquals( + 'No', @$config['NoSuchClassExists'], + 'Except rule correctly detects missing class' + ); + } + + public function testModuleRules() { + $config = $this->getConfigFixtureValue('Module'); + + $this->assertEquals( + 'Yes', @$config['MysiteExists'], + 'Only rule correctly detects existing module' + ); + + $this->assertEquals( + 'No', @$config['NoSuchModuleExists'], + 'Except rule correctly detects missing module' + ); + } + + public function testEnvVarSetRules() { + $_ENV['EnvVarSet_Foo'] = 1; + $config = $this->getConfigFixtureValue('EnvVarSet'); + + $this->assertEquals( + 'Yes', @$config['FooSet'], + 'Only rule correctly detects set environment variable' + ); + + $this->assertEquals( + 'No', @$config['BarSet'], + 'Except rule correctly detects unset environment variable' + ); + } + + public function testConstantDefinedRules() { + define('ConstantDefined_Foo', 1); + $config = $this->getConfigFixtureValue('ConstantDefined'); + + $this->assertEquals( + 'Yes', @$config['FooDefined'], + 'Only rule correctly detects defined constant' + ); + + $this->assertEquals( + 'No', @$config['BarDefined'], + 'Except rule correctly detects undefined constant' + ); + } + + public function testEnvOrConstantMatchesValueRules() { + $_ENV['EnvOrConstantMatchesValue_Foo'] = 'Foo'; + define('EnvOrConstantMatchesValue_Bar', 'Bar'); + $config = $this->getConfigFixtureValue('EnvOrConstantMatchesValue'); + + $this->assertEquals( + 'Yes', @$config['FooIsFoo'], + 'Only rule correctly detects environment variable matches specified value' + ); + + $this->assertEquals( + 'Yes', @$config['BarIsBar'], + 'Only rule correctly detects constant matches specified value' + ); + + $this->assertEquals( + 'No', @$config['FooIsQux'], + 'Except rule correctly detects environment variable that doesn\'t match specified value' + ); + + $this->assertEquals( + 'No', @$config['BarIsQux'], + 'Except rule correctly detects environment variable that doesn\'t match specified value' + ); + + $this->assertEquals( + 'No', @$config['BazIsBaz'], + 'Except rule correctly detects undefined variable' + ); + } + + public function testEnvironmentRules() { + foreach (array('dev', 'test', 'live') as $env) { + Config::inst()->nest(); + + Config::inst()->update('Director', 'environment_type', $env); + $config = $this->getConfigFixtureValue('Environment'); + + foreach (array('dev', 'test', 'live') as $check) { + $this->assertEquals( + $env == $check ? $check : 'not'.$check, @$config[ucfirst($check).'Environment'], + 'Only & except rules correctly detect environment' + ); + } + + Config::inst()->unnest(); + } + } + + public function testDynamicEnvironmentRules() { + Config::inst()->nest(); + + // First, make sure environment_type is live + Config::inst()->update('Director', 'environment_type', 'live'); + $this->assertEquals('live', Config::inst()->get('Director', 'environment_type')); + + // Then, load in a new manifest, which includes a _config.php that sets environment_type to dev + $manifest = new SS_ConfigManifest(dirname(__FILE__).'/fixtures/configmanifest_dynamicenv', true, true); + Config::inst()->pushConfigYamlManifest($manifest); + + // Make sure that stuck + $this->assertEquals('dev', Config::inst()->get('Director', 'environment_type')); + + // And that the dynamic rule was calculated correctly + $this->assertEquals('dev', Config::inst()->get('ConfigManifestTest', 'DynamicEnvironment')); + + Config::inst()->unnest(); + } + + public function testMultipleRules() { + $_ENV['MultilpleRules_EnvVariableSet'] = 1; + define('MultilpleRules_DefinedConstant', 'defined'); + $config = $this->getConfigFixtureValue('MultipleRules'); + + $this->assertFalse( + isset($config['TwoOnlyFail']), + 'Fragment is not included if one of the Only rules fails.' + ); + + $this->assertTrue( + isset($config['TwoOnlySucceed']), + 'Fragment is included if both Only rules succeed.' + ); + + $this->assertTrue( + isset($config['TwoExceptSucceed']), + 'Fragment is included if one of the Except rules matches.' + ); + + $this->assertFalse( + isset($config['TwoExceptFail']), + 'Fragment is not included if both of the Except rules fail.' + ); + + $this->assertFalse( + isset($config['TwoBlocksFail']), + 'Fragment is not included if one block fails.' + ); + + $this->assertTrue( + isset($config['TwoBlocksSucceed']), + 'Fragment is included if both blocks succeed.' + ); + } + public function testRelativeOrder() { $accessor = new ConfigManifestTest_ConfigManifestAccess(BASE_PATH, true, false); @@ -88,4 +256,4 @@ class ConfigManifestTest extends SapphireTest { ), 'after'); } -} \ No newline at end of file +} diff --git a/tests/core/manifest/ConfigStaticManifestTest.php b/tests/core/manifest/ConfigStaticManifestTest.php index cb87841b5..87d1103db 100644 --- a/tests/core/manifest/ConfigStaticManifestTest.php +++ b/tests/core/manifest/ConfigStaticManifestTest.php @@ -170,7 +170,8 @@ DOC; return; } - $parser = new SS_ConfigStaticManifest_Parser(__DIR__ . '/ConfigStaticManifestTest/ConfigStaticManifestTestMyObject.php'); + $parser = new SS_ConfigStaticManifest_Parser(__DIR__ . + '/ConfigStaticManifestTest/ConfigStaticManifestTestMyObject.php'); $parser->parse(); $statics = $parser->getStatics(); @@ -182,4 +183,19 @@ DOC; $this->assertEquals($expectedValue, $statics['ConfigStaticManifestTestMyObject']['db']['value']); } + + public function testParsingNamespacesclass() { + $parser = new SS_ConfigStaticManifest_Parser(__DIR__ . + '/ConfigStaticManifestTest/ConfigStaticManifestTestNamespace.php'); + $parser->parse(); + + $statics = $parser->getStatics(); + + $expectedValue = array( + 'Name' => 'Varchar', + 'Description' => 'Text', + ); + + $this->assertEquals($expectedValue, $statics['config\staticmanifest\NamespaceTest']['db']['value']); + } } diff --git a/tests/core/manifest/ConfigStaticManifestTest/ConfigStaticManifestTestNamespace.php b/tests/core/manifest/ConfigStaticManifestTest/ConfigStaticManifestTestNamespace.php new file mode 100644 index 000000000..a2128af98 --- /dev/null +++ b/tests/core/manifest/ConfigStaticManifestTest/ConfigStaticManifestTestNamespace.php @@ -0,0 +1,10 @@ + 'Varchar', + 'Description' => 'Text', + ); +} diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/classrules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/classrules.yml new file mode 100644 index 000000000..eaadf2c7a --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/classrules.yml @@ -0,0 +1,28 @@ +--- +Only: + ClassExists: Director +--- +ConfigManifestTest: + Class: + DirectorExists: Yes +--- +Only: + ClassExists: NoSuchClass +--- +ConfigManifestTest: + Class: + NoSuchClassExists: Yes +--- +Except: + ClassExists: Director +--- +ConfigManifestTest: + Class: + DirectorExists: No +--- +Except: + ClassExists: NoSuchClass +--- +ConfigManifestTest: + Class: + NoSuchClassExists: No diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/constantdefinedrules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/constantdefinedrules.yml new file mode 100644 index 000000000..2833cb592 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/constantdefinedrules.yml @@ -0,0 +1,28 @@ +--- +Only: + ConstantDefined: ConstantDefined_Foo +--- +ConfigManifestTest: + ConstantDefined: + FooDefined: Yes +--- +Only: + ConstantDefined: ConstantDefined_Bar +--- +ConfigManifestTest: + ConstantDefined: + BarDefined: Yes +--- +Except: + ConstantDefined: ConstantDefined_Foo +--- +ConfigManifestTest: + ConstantDefined: + FooDefined: No +--- +Except: + ConstantDefined: ConstantDefined_Bar +--- +ConfigManifestTest: + ConstantDefined: + BarDefined: No diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/envrules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/envrules.yml new file mode 100644 index 000000000..6573ac565 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/envrules.yml @@ -0,0 +1,42 @@ +--- +Only: + Environment: live +--- +ConfigManifestTest: + Environment: + LiveEnvironment: live +--- +Only: + Environment: dev +--- +ConfigManifestTest: + Environment: + DevEnvironment: dev +--- +Only: + Environment: test +--- +ConfigManifestTest: + Environment: + TestEnvironment: test +--- +Except: + Environment: live +--- +ConfigManifestTest: + Environment: + LiveEnvironment: notlive +--- +Except: + Environment: dev +--- +ConfigManifestTest: + Environment: + DevEnvironment: notdev +--- +Except: + Environment: test +--- +ConfigManifestTest: + Environment: + TestEnvironment: nottest diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/envvarsetrules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/envvarsetrules.yml new file mode 100644 index 000000000..45368aa00 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/envvarsetrules.yml @@ -0,0 +1,28 @@ +--- +Only: + EnvVarSet: EnvVarSet_Foo +--- +ConfigManifestTest: + EnvVarSet: + FooSet: Yes +--- +Only: + EnvVarSet: EnvVarSet_Bar +--- +ConfigManifestTest: + EnvVarSet: + BarSet: Yes +--- +Except: + EnvVarSet: EnvVarSet_Foo +--- +ConfigManifestTest: + EnvVarSet: + FooSet: No +--- +Except: + EnvVarSet: EnvVarSet_Bar +--- +ConfigManifestTest: + EnvVarSet: + BarSet: No diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/modulerules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/modulerules.yml new file mode 100644 index 000000000..1cb9232f6 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/modulerules.yml @@ -0,0 +1,28 @@ +--- +Only: + ModuleExists: mysite +--- +ConfigManifestTest: + Module: + MysiteExists: Yes +--- +Only: + ModuleExists: nosuchmodule +--- +ConfigManifestTest: + Module: + NoSuchModuleExists: Yes +--- +Except: + ModuleExists: mysite +--- +ConfigManifestTest: + Module: + MysiteExists: No +--- +Except: + ModuleExists: nosuchmodule +--- +ConfigManifestTest: + Module: + NoSuchModuleExists: No diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/multiplerules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/multiplerules.yml new file mode 100644 index 000000000..459e4e635 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/multiplerules.yml @@ -0,0 +1,50 @@ +--- +Only: + ConstantDefined: MultilpleRules_UndefinedConstant + EnvVarSet: MultilpleRules_EnvVariableSet +--- +ConfigManifestTest: + MultipleRules: + TwoOnlyFail: "not included - one of the onlies fails" +--- +Only: + ConstantDefined: MultilpleRules_DefinedConstant + EnvVarSet: MultilpleRules_EnvVariableSet +--- +ConfigManifestTest: + MultipleRules: + TwoOnlySucceed: "included - both onlies succeed" +--- +Except: + ConstantDefined: MultilpleRules_UndefinedConstant + EnvVarSet: MultilpleRules_EnvVariableSet +--- +ConfigManifestTest: + MultipleRules: + TwoExceptSucceed: "included - one of the excepts succeeds" +--- +Except: + ConstantDefined: MultilpleRules_DefinedConstant + EnvVarSet: MultilpleRules_EnvVariableSet +--- +ConfigManifestTest: + MultipleRules: + TwoExceptFail: "not included - both excepts fail" +--- +Except: + EnvVarSet: MultilpleRules_EnvVariableSet +Only: + EnvVarSet: MultilpleRules_EnvVariableSet +--- +ConfigManifestTest: + MultipleRules: + TwoBlocksFail: "not included - one block fails" +--- +Except: + ConstantDefined: MultilpleRules_UndefinedConstant +Only: + EnvVarSet: MultilpleRules_EnvVariableSet +--- +ConfigManifestTest: + MultipleRules: + TwoBlocksSucceed: "included - both blocks succeed" diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/variablerules.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/variablerules.yml new file mode 100644 index 000000000..ebb7c61a1 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/variablerules.yml @@ -0,0 +1,70 @@ +--- +Only: + EnvOrConstantMatchesValue_Foo: Foo +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + FooIsFoo: Yes +--- +Only: + EnvOrConstantMatchesValue_Foo: Qux +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + FooIsQux: Yes +--- +Only: + EnvOrConstantMatchesValue_Bar: Bar +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + BarIsBar: Yes +--- +Only: + EnvOrConstantMatchesValue_Bar: Qux +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + BarIsQux: Yes +--- +Only: + EnvOrConstantMatchesValue_Baz: Baz +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + BazIsBaz: Yes +--- +Except: + EnvOrConstantMatchesValue_Foo: Foo +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + FooIsFoo: No +--- +Except: + EnvOrConstantMatchesValue_Foo: Qux +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + FooIsQux: No +--- +Except: + EnvOrConstantMatchesValue_Bar: Bar +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + BarIsBar: No +--- +Except: + EnvOrConstantMatchesValue_Bar: Qux +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + BarIsQux: No +--- +Except: + EnvOrConstantMatchesValue_Baz: Baz +--- +ConfigManifestTest: + EnvOrConstantMatchesValue: + BazIsBaz: No diff --git a/tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php b/tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php new file mode 100644 index 000000000..337294dec --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php @@ -0,0 +1,4 @@ +update('Director', 'environment_type', 'dev'); diff --git a/tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config/environment.yml b/tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config/environment.yml new file mode 100644 index 000000000..fb5e190e3 --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config/environment.yml @@ -0,0 +1,18 @@ +--- +Only: + Environment: live +--- +ConfigManifestTest: + DynamicEnvironment: live +--- +Only: + Environment: dev +--- +ConfigManifestTest: + DynamicEnvironment: dev +--- +Only: + Environment: test +--- +ConfigManifestTest: + DynamicEnvironment: test diff --git a/tests/forms/ConfirmedPasswordFieldTest.php b/tests/forms/ConfirmedPasswordFieldTest.php index 4f7bfc213..abe8a9794 100644 --- a/tests/forms/ConfirmedPasswordFieldTest.php +++ b/tests/forms/ConfirmedPasswordFieldTest.php @@ -15,6 +15,26 @@ class ConfirmedPasswordFieldTest extends SapphireTest { $this->assertEquals('valueB', $field->children->fieldByName($field->getName() . '[_ConfirmPassword]')->Value()); } + public function testHashHidden() { + $field = new ConfirmedPasswordField('Password', 'Password', 'valueA'); + $field->setCanBeEmpty(true); + + $this->assertEquals('valueA', $field->Value()); + $this->assertEquals('valueA', $field->children->fieldByName($field->getName() . '[_Password]')->Value()); + $this->assertEquals('valueA', $field->children->fieldByName($field->getName() . '[_ConfirmPassword]')->Value()); + + $member = new Member(); + $member->Password = "valueB"; + $member->write(); + + $form = new Form($this, 'Form', new FieldList($field), new FieldList()); + $form->loadDataFrom($member); + + $this->assertEquals('', $field->Value()); + $this->assertEquals('', $field->children->fieldByName($field->getName() . '[_Password]')->Value()); + $this->assertEquals('', $field->children->fieldByName($field->getName() . '[_ConfirmPassword]')->Value()); + } + public function testSetShowOnClick() { //hide by default and display show/hide toggle button $field = new ConfirmedPasswordField('Test', 'Testing', 'valueA', null, true); diff --git a/tests/forms/EmailFieldTest.php b/tests/forms/EmailFieldTest.php index d60676211..caeb86154 100644 --- a/tests/forms/EmailFieldTest.php +++ b/tests/forms/EmailFieldTest.php @@ -73,6 +73,8 @@ class EmailFieldTest_Validator extends Validator { class EmailFieldTest_Controller extends Controller implements TestOnly { + private static $allowed_actions = array('Form'); + private static $url_handlers = array( '$Action//$ID/$OtherID' => "handleAction", ); diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index f08c3e241..6d1cbf760 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -479,6 +479,9 @@ class FormTest_Team extends DataObject implements TestOnly { * @subpackage tests */ class FormTest_Controller extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + private static $url_handlers = array( '$Action//$ID/$OtherID' => "handleAction", ); @@ -528,6 +531,9 @@ class FormTest_Controller extends Controller implements TestOnly { * @subpackage tests */ class FormTest_ControllerWithSecurityToken extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + private static $url_handlers = array( '$Action//$ID/$OtherID' => "handleAction", ); @@ -562,6 +568,9 @@ class FormTest_ControllerWithSecurityToken extends Controller implements TestOnl } class FormTest_ControllerWithStrictPostCheck extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + protected $template = 'BlankPage'; public function Link($action = null) { diff --git a/tests/forms/HtmlEditorSanitiserTest.php b/tests/forms/HtmlEditorSanitiserTest.php new file mode 100644 index 000000000..d9927c35b --- /dev/null +++ b/tests/forms/HtmlEditorSanitiserTest.php @@ -0,0 +1,57 @@ +Leave Alone

Strip parentBut keep children in order
', + '

Leave Alone

Strip parentBut keep children in order', + 'Non-whitelisted elements are stripped, but children are kept' + ), + array( + 'p,strong', + '
A B
Nested elements are still filtered
C D
', + 'A B Nested elements are still filtered C D', + 'Non-whitelisted elements are stripped even when children of non-whitelisted elements' + ), + array( + 'p', + '

Keep

', + '

Keep

', + 'Non-whitelisted script elements are totally stripped, including any children' + ), + array( + 'p[id]', + '

Test

', + '

Test

', + 'Non-whitelisted attributes are stripped' + ), + array( + 'p[default1=default1|default2=default2|force1:force1|force2:force2]', + '

Test

', + '

Test

', + 'Default attributes are set when not present in input, forced attributes are always set' + ) + ); + + $config = HtmlEditorConfig::get('htmleditorsanitisertest'); + + foreach($tests as $test) { + list($validElements, $input, $output, $desc) = $test; + + $config->setOptions(array('valid_elements' => $validElements)); + $sanitiser = new HtmlEditorSanitiser($config); + + $htmlValue = Injector::inst()->create('HTMLValue', $input); + $sanitiser->sanitise($htmlValue); + + $this->assertEquals($output, $htmlValue->getContent(), $desc); + } + } + +} \ No newline at end of file diff --git a/tests/forms/gridfield/GridFieldAddExistingAutocompleterTest.php b/tests/forms/gridfield/GridFieldAddExistingAutocompleterTest.php index 2141a7fcb..fdff0d0d9 100644 --- a/tests/forms/gridfield/GridFieldAddExistingAutocompleterTest.php +++ b/tests/forms/gridfield/GridFieldAddExistingAutocompleterTest.php @@ -97,6 +97,8 @@ class GridFieldAddExistingAutocompleterTest extends FunctionalTest { class GridFieldAddExistingAutocompleterTest_Controller extends Controller implements TestOnly { + private static $allowed_actions = array('Form'); + protected $template = 'BlankPage'; public function Form() { diff --git a/tests/forms/gridfield/GridFieldDetailFormTest.php b/tests/forms/gridfield/GridFieldDetailFormTest.php index 997d5c291..5b08248d1 100644 --- a/tests/forms/gridfield/GridFieldDetailFormTest.php +++ b/tests/forms/gridfield/GridFieldDetailFormTest.php @@ -282,6 +282,7 @@ class GridFieldDetailFormTest_PeopleGroup extends DataObject implements TestOnly } class GridFieldDetailFormTest_Category extends DataObject implements TestOnly { + private static $db = array( 'Name' => 'Varchar' ); @@ -306,6 +307,9 @@ class GridFieldDetailFormTest_Category extends DataObject implements TestOnly { } class GridFieldDetailFormTest_Controller extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + protected $template = 'BlankPage'; public function Form() { @@ -326,6 +330,9 @@ class GridFieldDetailFormTest_Controller extends Controller implements TestOnly } class GridFieldDetailFormTest_GroupController extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + protected $template = 'BlankPage'; public function Form() { @@ -339,6 +346,9 @@ class GridFieldDetailFormTest_GroupController extends Controller implements Test } class GridFieldDetailFormTest_CategoryController extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + protected $template = 'BlankPage'; public function Form() { diff --git a/tests/forms/gridfield/GridField_URLHandlerTest.php b/tests/forms/gridfield/GridField_URLHandlerTest.php index ec7204b60..914e6e0a3 100644 --- a/tests/forms/gridfield/GridField_URLHandlerTest.php +++ b/tests/forms/gridfield/GridField_URLHandlerTest.php @@ -30,6 +30,9 @@ class GridField_URLHandlerTest extends FunctionalTest { } class GridField_URLHandlerTest_Controller extends Controller implements TestOnly { + + private static $allowed_actions = array('Form'); + public function Link() { return get_class($this) ."/"; } @@ -51,6 +54,9 @@ class GridField_URLHandlerTest_Controller extends Controller implements TestOnly * Test URLHandler with a nested request handler */ class GridField_URLHandlerTest_Component extends RequestHandler implements GridField_URLHandler { + + private static $allowed_actions = array('Form', 'showform', 'testpage', 'handleItem'); + protected $gridField; public function getURLHandlers($gridField) { @@ -96,8 +102,13 @@ class GridField_URLHandlerTest_Component extends RequestHandler implements GridF } class GridField_URLHandlerTest_Component_ItemRequest extends RequestHandler { + + private static $allowed_actions = array('Form', 'showform', 'testpage'); + protected $gridField; + protected $link; + protected $id; public function __construct($gridField, $id, $link) { diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index e6e2b9941..456d48120 100644 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -21,7 +21,47 @@ class DataListTest extends SapphireTest { 'DataObjectTest_TeamComment', 'DataObjectTest\NamespacedClass', ); - + + public function testFilterDataObjectByCreatedDate() { + // create an object to test with + $obj1 = new DataObjectTest_ValidatedObject(); + $obj1->Name = 'test obj 1'; + $obj1->write(); + $this->assertTrue($obj1->isInDB()); + + // reload the object from the database and reset its Created timestamp to a known value + $obj1 = DataObjectTest_ValidatedObject::get()->filter(array('Name' => 'test obj 1'))->first(); + $this->assertTrue(is_object($obj1)); + $this->assertEquals('test obj 1', $obj1->Name); + $obj1->Created = '2013-01-01 00:00:00'; + $obj1->write(); + + // reload the object again and make sure that our Created date was properly persisted + $obj1 = DataObjectTest_ValidatedObject::get()->filter(array('Name' => 'test obj 1'))->first(); + $this->assertTrue(is_object($obj1)); + $this->assertEquals('test obj 1', $obj1->Name); + $this->assertEquals('2013-01-01 00:00:00', $obj1->Created); + + // now save a second object to the DB with an automatically-set Created value + $obj2 = new DataObjectTest_ValidatedObject(); + $obj2->Name = 'test obj 2'; + $obj2->write(); + $this->assertTrue($obj2->isInDB()); + + // and a third object + $obj3 = new DataObjectTest_ValidatedObject(); + $obj3->Name = 'test obj 3'; + $obj3->write(); + $this->assertTrue($obj3->isInDB()); + + // now test the filtering based on Created timestamp + $list = DataObjectTest_ValidatedObject::get() + ->filter(array('Created:GreaterThan' => '2013-02-01 00:00:00')) + ->toArray(); + $this->assertEquals(2, count($list)); + + } + public function testSubtract(){ $comment1 = $this->objFromFixture('DataObjectTest_TeamComment', 'comment1'); $subtractList = DataObjectTest_TeamComment::get()->filter('ID', $comment1->ID); diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 534bdbda1..fee16b636 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -18,7 +18,20 @@ class DataObjectTest extends SapphireTest { 'DataObjectTest_Player', 'DataObjectTest_TeamComment' ); - + + public function testValidObjectsForBaseFields() { + $obj = new DataObjectTest_ValidatedObject(); + + foreach (array('Created', 'LastEdited', 'ClassName', 'ID') as $field) { + $helper = $obj->dbObject($field); + $this->assertTrue( + ($helper instanceof DBField), + "for {$field} expected helper to be DBField, but was " . + (is_object($helper) ? get_class($helper) : "null") + ); + } + } + public function testDataIntegrityWhenTwoSubclassesHaveSameField() { // Save data into DataObjectTest_SubTeam.SubclassDatabaseField $obj = new DataObjectTest_SubTeam(); diff --git a/tests/security/GroupTest.php b/tests/security/GroupTest.php index 2e951772c..f89a2cd74 100644 --- a/tests/security/GroupTest.php +++ b/tests/security/GroupTest.php @@ -44,12 +44,12 @@ class GroupTest extends FunctionalTest { $form->saveInto($member); $updatedGroups = $member->Groups(); - $this->assertEquals( - array($adminGroup->ID, $parentGroup->ID), - $updatedGroups->column(), + $this->assertEquals(2, count($updatedGroups->column()), "Adding a toplevel group works" ); - + $this->assertContains($adminGroup->ID, $updatedGroups->column('ID')); + $this->assertContains($parentGroup->ID, $updatedGroups->column('ID')); + // Test unsetting relationship $form->loadDataFrom($member); $checkboxSetField = $form->Fields()->fieldByName('Groups'); @@ -60,11 +60,10 @@ class GroupTest extends FunctionalTest { $form->saveInto($member); $member->flushCache(); $updatedGroups = $member->Groups(); - $this->assertEquals( - array($adminGroup->ID), - $updatedGroups->column(), + $this->assertEquals(1, count($updatedGroups->column()), "Removing a previously added toplevel group works" ); + $this->assertContains($adminGroup->ID, $updatedGroups->column('ID')); // Test adding child group @@ -77,23 +76,21 @@ class GroupTest extends FunctionalTest { $orphanGroup->ParentID = 99999; $orphanGroup->write(); - $this->assertEquals( - array($parentGroup->ID), - $parentGroup->collateAncestorIDs(), + $this->assertEquals(1, count($parentGroup->collateAncestorIDs()), 'Root node only contains itself' ); + $this->assertContains($parentGroup->ID, $parentGroup->collateAncestorIDs()); - $this->assertEquals( - array($childGroup->ID, $parentGroup->ID), - $childGroup->collateAncestorIDs(), + $this->assertEquals(2, count($childGroup->collateAncestorIDs()), 'Contains parent nodes, with child node first' ); + $this->assertContains($parentGroup->ID, $childGroup->collateAncestorIDs()); + $this->assertContains($childGroup->ID, $childGroup->collateAncestorIDs()); - $this->assertEquals( - array($orphanGroup->ID), - $orphanGroup->collateAncestorIDs(), + $this->assertEquals(1, count($orphanGroup->collateAncestorIDs()), 'Orphaned nodes dont contain invalid parent IDs' ); + $this->assertContains($orphanGroup->ID, $orphanGroup->collateAncestorIDs()); } public function testDelete() { diff --git a/tests/security/MemberCsvBulkLoaderTest.php b/tests/security/MemberCsvBulkLoaderTest.php index c8ea526df..400103f61 100644 --- a/tests/security/MemberCsvBulkLoaderTest.php +++ b/tests/security/MemberCsvBulkLoaderTest.php @@ -41,8 +41,11 @@ class MemberCsvBulkLoaderTest extends SapphireTest { $results = $loader->load($this->getCurrentRelativePath() . '/MemberCsvBulkLoaderTest.csv'); $created = $results->Created()->toArray(); - $this->assertEquals($created[0]->Groups()->column('ID'), array($existinggroup->ID)); - $this->assertEquals($created[1]->Groups()->column('ID'), array($existinggroup->ID)); + $this->assertEquals(1, count($created[0]->Groups()->column('ID'))); + $this->assertContains($existinggroup->ID, $created[0]->Groups()->column('ID')); + + $this->assertEquals(1, count($created[1]->Groups()->column('ID'))); + $this->assertContains($existinggroup->ID, $created[1]->Groups()->column('ID')); } public function testAddToCsvColumnGroupsByCode() { @@ -55,8 +58,12 @@ class MemberCsvBulkLoaderTest extends SapphireTest { $this->assertEquals($newgroup->Title, 'newgroup'); $created = $results->Created()->toArray(); - $this->assertEquals($created[0]->Groups()->column('ID'), array($existinggroup->ID)); - $this->assertEquals($created[1]->Groups()->column('ID'), array($existinggroup->ID, $newgroup->ID)); + $this->assertEquals(1, count($created[0]->Groups()->column('ID'))); + $this->assertContains($existinggroup->ID, $created[0]->Groups()->column('ID')); + + $this->assertEquals(2, count($created[1]->Groups()->column('ID'))); + $this->assertContains($existinggroup->ID, $created[1]->Groups()->column('ID')); + $this->assertContains($newgroup->ID, $created[1]->Groups()->column('ID')); } public function testCleartextPasswordsAreHashedWithDefaultAlgo() { diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index d23246db5..294b082b4 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -439,6 +439,9 @@ class SecurityTest extends FunctionalTest { } class SecurityTest_SecuredController extends Controller implements TestOnly { + + private static $allowed_actions = array('index'); + public function index() { if(!Permission::check('ADMIN')) return Security::permissionFailure($this); diff --git a/tests/view/SSViewerTest.php b/tests/view/SSViewerTest.php index 92b181b28..5e7589165 100644 --- a/tests/view/SSViewerTest.php +++ b/tests/view/SSViewerTest.php @@ -165,6 +165,18 @@ SS; 'Permissions template functions result correct result'); } + public function testNonFieldCastingHelpersNotUsedInHasValue() { + // check if Link without $ in front of variable + $result = $this->render( + 'A<% if Link %>$Link<% end_if %>B', new SSViewerTest_Object()); + $this->assertEquals('Asome/url.htmlB', $result, 'casting helper not used for <% if Link %>'); + + // check if Link with $ in front of variable + $result = $this->render( + 'A<% if $Link %>$Link<% end_if %>B', new SSViewerTest_Object()); + $this->assertEquals('Asome/url.htmlB', $result, 'casting helper not used for <% if $Link %>'); + } + public function testLocalFunctionsTakePriorityOverGlobals() { $data = new ArrayData(array( 'Page' => new SSViewerTest_Object() @@ -1062,7 +1074,7 @@ after') $origEnv = Config::inst()->get('Director', 'environment_type'); Config::inst()->update('Director', 'environment_type', 'dev'); Config::inst()->update('SSViewer', 'source_file_comments', true); - $f = FRAMEWORK_PATH . '/tests/templates/SSViewerTestComments'; + $f = FRAMEWORK_PATH . '/tests/templates/SSViewerTestComments'; $templates = array( array( 'name' => 'SSViewerTestCommentsFullSource', @@ -1078,7 +1090,8 @@ after') array( 'name' => 'SSViewerTestCommentsFullSourceHTML4Doctype', 'expected' => "" - . "" + . "" . "" . "" . "\t" @@ -1197,6 +1210,45 @@ after') "tests/forms/RequirementsTest_a.js" )); } + + public function testCallsWithArguments() { + $data = new ArrayData(array( + 'Set' => new ArrayList(array( + new SSViewerTest_Object("1"), + new SSViewerTest_Object("2"), + new SSViewerTest_Object("3"), + new SSViewerTest_Object("4"), + new SSViewerTest_Object("5"), + )), + 'Level' => new SSViewerTest_LevelTest(1), + 'Nest' => array( + 'Level' => new SSViewerTest_LevelTest(2), + ), + )); + + $tests = array( + '$Level.output(1)' => '1-1', + '$Nest.Level.output($Set.First.Number)' => '2-1', + '<% with $Set %>$Up.Level.output($First.Number)<% end_with %>' => '1-1', + '<% with $Set %>$Top.Nest.Level.output($First.Number)<% end_with %>' => '2-1', + '<% loop $Set %>$Up.Nest.Level.output($Number)<% end_loop %>' => '2-12-22-32-42-5', + '<% loop $Set %>$Top.Level.output($Number)<% end_loop %>' => '1-11-21-31-41-5', + '<% with $Nest %>$Level.output($Top.Set.First.Number)<% end_with %>' => '2-1', + '<% with $Level %>$output($Up.Set.Last.Number)<% end_with %>' => '1-5', + '<% with $Level.forWith($Set.Last.Number) %>$output("hi")<% end_with %>' => '5-hi', + '<% loop $Level.forLoop($Set.First.Number) %>$Number<% end_loop %>' => '!0', + '<% with $Nest %> + <% with $Level.forWith($Up.Set.First.Number) %>$output("hi")<% end_with %> + <% end_with %>' => '1-hi', + '<% with $Nest %> + <% loop $Level.forLoop($Top.Set.Last.Number) %>$Number<% end_loop %> + <% end_with %>' => '!0!1!2!3!4', + ); + + foreach($tests as $template => $expected) { + $this->assertEquals($expected, trim($this->render($template, $data))); + } + } } /** @@ -1274,6 +1326,11 @@ class SSViewerTest_Object extends DataObject { public $number = null; + private static $casting = array( + 'Link' => 'Text', + ); + + public function __construct($number = null) { parent::__construct(); $this->number = $number; @@ -1290,6 +1347,10 @@ class SSViewerTest_Object extends DataObject { public function lotsOfArguments11($a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k) { return $a. $b. $c. $d. $e. $f. $g. $h. $i. $j. $k; } + + public function Link() { + return 'some/url.html'; + } } class SSViewerTest_GlobalProvider implements TemplateGlobalProvider, TestOnly { @@ -1326,3 +1387,28 @@ class SSViewerTest_GlobalProvider implements TemplateGlobalProvider, TestOnly { } } + +class SSViewerTest_LevelTest extends ViewableData implements TestOnly { + protected $depth; + + public function __construct($depth = 1) { + $this->depth = $depth; + } + + public function output($val) { + return "$this->depth-$val"; + } + + public function forLoop($number) { + $ret = array(); + for($i = 0; $i < (int)$number; ++$i) { + $ret[] = new SSViewerTest_Object("!$i"); + } + return new ArrayList($ret); + } + + public function forWith($number) { + return new self($number); + } +} + diff --git a/tests/view/ViewableDataTest.php b/tests/view/ViewableDataTest.php index a90c165fe..855520691 100644 --- a/tests/view/ViewableDataTest.php +++ b/tests/view/ViewableDataTest.php @@ -81,6 +81,19 @@ class ViewableDataTest extends SapphireTest { $this->assertEquals('casted', $newViewableData->forTemplate()); } + public function testDefaultValueWrapping() { + $data = new ArrayData(array('Title' => 'SomeTitleValue')); + // this results in a cached raw string in ViewableData: + $this->assertTrue($data->hasValue('Title')); + $this->assertFalse($data->hasValue('SomethingElse')); + // this should cast the raw string to a StringField since we are + // passing true as the third argument: + $obj = $data->obj('Title', null, true); + $this->assertTrue(is_object($obj)); + // and the string field should have the value of the raw string: + $this->assertEquals('SomeTitleValue', $obj->forTemplate()); + } + public function testRAWVal() { $data = new ViewableDataTest_Castable(); $data->test = 'This & This'; @@ -121,6 +134,28 @@ class ViewableDataTest extends SapphireTest { ); } } + + public function testObjWithCachedStringValueReturnsValidObject() { + $obj = new ViewableDataTest_NoCastingInformation(); + + // Save a literal string into cache + $cache = true; + $uncastedData = $obj->obj('noCastingInformation', null, false, $cache); + + // Fetch the cached string as an object + $forceReturnedObject = true; + $castedData = $obj->obj('noCastingInformation', null, $forceReturnedObject); + + // Uncasted data should always be the nonempty string + $this->assertNotEmpty($uncastedData, 'Uncasted data was empty.'); + $this->assertTrue(is_string($uncastedData), 'Uncasted data should be a string.'); + + // Casted data should be the string wrapped in a DBField-object. + $this->assertNotEmpty($castedData, 'Casted data was empty.'); + $this->assertInstanceOf('DBField', $castedData, 'Casted data should be instance of DBField.'); + + $this->assertEquals($uncastedData, $castedData->getValue(), 'Casted and uncasted strings are not equal.'); + } } /**#@+ @@ -212,4 +247,10 @@ class ViewableDataTest_CastingClass extends ViewableData { ); } +class ViewableDataTest_NoCastingInformation extends ViewableData { + public function noCastingInformation() { + return "No casting information"; + } +} + /**#@-*/ diff --git a/view/SSTemplateParser.php b/view/SSTemplateParser.php index c3e3532b6..efe7e27ea 100644 --- a/view/SSTemplateParser.php +++ b/view/SSTemplateParser.php @@ -615,7 +615,7 @@ class SSTemplateParser extends Parser { function Lookup__construct(&$res) { - $res['php'] = '$scope'; + $res['php'] = '$scope->locally()'; $res['LookupSteps'] = array(); } diff --git a/view/SSTemplateParser.php.inc b/view/SSTemplateParser.php.inc index 68249b3ed..3c1349efe 100644 --- a/view/SSTemplateParser.php.inc +++ b/view/SSTemplateParser.php.inc @@ -157,7 +157,7 @@ class SSTemplateParser extends Parser { */ function Lookup__construct(&$res) { - $res['php'] = '$scope'; + $res['php'] = '$scope->locally()'; $res['LookupSteps'] = array(); } diff --git a/view/SSViewer.php b/view/SSViewer.php index 220d59d87..40df8f510 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -49,18 +49,34 @@ class SSViewer_Scope { public function __construct($item){ $this->item = $item; - $this->localIndex=0; + $this->localIndex = 0; + $this->localStack = array(); $this->itemStack[] = array($this->item, null, 0, null, null, 0); } public function getItem(){ return $this->itemIterator ? $this->itemIterator->current() : $this->item; } - - public function resetLocalScope(){ + + /** Called at the start of every lookup chain by SSTemplateParser to indicate a new lookup from local scope */ + public function locally() { list($this->item, $this->itemIterator, $this->itemIteratorTotal, $this->popIndex, $this->upIndex, $this->currentIndex) = $this->itemStack[$this->localIndex]; - array_splice($this->itemStack, $this->localIndex+1); + + // Remember any un-completed (resetLocalScope hasn't been called) lookup chain. Even if there isn't an + // un-completed chain we need to store an empty item, as resetLocalScope doesn't know the difference later + $this->localStack[] = array_splice($this->itemStack, $this->localIndex+1); + + return $this; + } + + public function resetLocalScope(){ + $previousLocalState = $this->localStack ? array_pop($this->localStack) : null; + + array_splice($this->itemStack, $this->localIndex+1, count($this->itemStack), $previousLocalState); + + list($this->item, $this->itemIterator, $this->itemIteratorTotal, $this->popIndex, $this->upIndex, + $this->currentIndex) = end($this->itemStack); } public function getObj($name, $arguments = null, $forceReturnedObject = true, $cache = false, $cacheName = null) { diff --git a/view/ViewableData.php b/view/ViewableData.php index dc428d24f..7a6cf5b4c 100644 --- a/view/ViewableData.php +++ b/view/ViewableData.php @@ -383,7 +383,9 @@ class ViewableData extends Object implements IteratorAggregate { if(!is_object($value) && $forceReturnedObject) { $default = Config::inst()->get('ViewableData', 'default_cast', Config::FIRST_SET); - $value = new $default($fieldName); + $castedValue = new $default($fieldName); + $castedValue->setValue($value); + $value = $castedValue; } return $value;