BUG Fixed instances of loosely defined SQL predicates not qualified by table name

Fixed duplicate SQL escaping on SiteTree::get_by_link
This commit is contained in:
Damian Mooyman 2013-08-29 13:59:45 +12:00
parent 6d694a550a
commit 5f828149c3
8 changed files with 41 additions and 25 deletions

View File

@ -814,7 +814,10 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
// Fall back to homepage record // Fall back to homepage record
if(!$id) { if(!$id) {
$homepageSegment = RootURLController::get_homepage_link(); $homepageSegment = RootURLController::get_homepage_link();
$homepageRecord = DataObject::get_one('SiteTree', sprintf('"URLSegment" = \'%s\'', $homepageSegment)); $homepageRecord = DataObject::get_one('SiteTree', sprintf(
'"SiteTree"."URLSegment" = \'%s\'',
Convert::raw2sql($homepageSegment)
));
if($homepageRecord) $id = $homepageRecord->ID; if($homepageRecord) $id = $homepageRecord->ID;
} }

View File

@ -123,7 +123,7 @@ class CMSPageAddController extends CMSPageEditController {
$suffix = isset($data['Suffix']) ? "-" . $data['Suffix'] : null; $suffix = isset($data['Suffix']) ? "-" . $data['Suffix'] : null;
if(!$parentID && isset($data['Parent'])) { if(!$parentID && isset($data['Parent'])) {
$page = SiteTree:: get_by_link(Convert::raw2sql($data['Parent'])); $page = SiteTree::get_by_link($data['Parent']);
if($page) $parentID = $page->ID; if($page) $parentID = $page->ID;
} }

View File

@ -163,9 +163,10 @@ class ContentController extends Controller {
// See ModelAdController->getNestedController() for similar logic // See ModelAdController->getNestedController() for similar logic
if(class_exists('Translatable')) Translatable::disable_locale_filter(); if(class_exists('Translatable')) Translatable::disable_locale_filter();
// look for a page with this URLSegment // look for a page with this URLSegment
$child = $this->model->SiteTree->where(sprintf ( $child = $this->model->SiteTree->filter(array(
"\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql(rawurlencode($action)) 'ParentID' => $this->ID,
))->First(); 'URLSegment' => rawurlencode($action)
))->first();
if(class_exists('Translatable')) Translatable::enable_locale_filter(); if(class_exists('Translatable')) Translatable::enable_locale_filter();
// if we can't find a page with this URLSegment try to find one that used to have // if we can't find a page with this URLSegment try to find one that used to have
@ -258,7 +259,10 @@ class ContentController extends Controller {
*/ */
public function getMenu($level = 1) { public function getMenu($level = 1) {
if($level == 1) { if($level == 1) {
$result = DataObject::get("SiteTree", "\"ShowInMenus\" = 1 AND \"ParentID\" = 0"); $result = SiteTree::get()->filter(array(
"ShowInMenus" => 1,
"ParentID" => 0
));
} else { } else {
$parent = $this->data(); $parent = $this->data();
@ -399,7 +403,7 @@ HTML;
$this->httpError(410); $this->httpError(410);
} }
// The manifest should be built by now, so it's safe to publish the 404 page // The manifest should be built by now, so it's safe to publish the 404 page
$fourohfour = Versioned::get_one_by_stage('ErrorPage', 'Stage', '"ErrorCode" = 404'); $fourohfour = Versioned::get_one_by_stage('ErrorPage', 'Stage', '"ErrorPage"."ErrorCode" = 404');
if($fourohfour) { if($fourohfour) {
$fourohfour->write(); $fourohfour->write();
$fourohfour->publish("Stage", "Live"); $fourohfour->publish("Stage", "Live");

View File

@ -93,9 +93,9 @@ class ModelAsController extends Controller implements NestedController {
$sitetree = DataObject::get_one( $sitetree = DataObject::get_one(
'SiteTree', 'SiteTree',
sprintf( sprintf(
'"URLSegment" = \'%s\' %s', '"SiteTree"."URLSegment" = \'%s\' %s',
Convert::raw2sql(rawurlencode($URLSegment)), Convert::raw2sql(rawurlencode($URLSegment)),
(SiteTree::config()->nested_urls ? 'AND "ParentID" = 0' : null) (SiteTree::config()->nested_urls ? 'AND "SiteTree"."ParentID" = 0' : null)
) )
); );
if(class_exists('Translatable')) Translatable::enable_locale_filter(); if(class_exists('Translatable')) Translatable::enable_locale_filter();
@ -146,16 +146,15 @@ class ModelAsController extends Controller implements NestedController {
* @return SiteTree * @return SiteTree
*/ */
static public function find_old_page($URLSegment,$parentID = 0, $ignoreNestedURLs = false) { static public function find_old_page($URLSegment,$parentID = 0, $ignoreNestedURLs = false) {
$URLSegment = Convert::raw2sql(rawurlencode($URLSegment));
$useParentIDFilter = SiteTree::config()->nested_urls && $parentID; $useParentIDFilter = SiteTree::config()->nested_urls && $parentID;
// First look for a non-nested page that has a unique URLSegment and can be redirected to. // First look for a non-nested page that has a unique URLSegment and can be redirected to.
if(SiteTree::config()->nested_urls) { if(SiteTree::config()->nested_urls) {
$pages = DataObject::get( $pages = SiteTree::get()->filter("URLSegment", rawurlencode($URLSegment));
'SiteTree', if($useParentIDFilter) {
"\"URLSegment\" = '$URLSegment'" . ($useParentIDFilter ? ' AND "ParentID" = ' . (int)$parentID : '') $pages = $pages->filter("ParentID", (int)$parentID);
); }
if($pages && $pages->Count() == 1 && ($page = $pages->First())) { if($pages && $pages->Count() == 1 && ($page = $pages->First())) {
$parent = $page->ParentID ? $page->Parent() : $page; $parent = $page->ParentID ? $page->Parent() : $page;
@ -164,10 +163,11 @@ class ModelAsController extends Controller implements NestedController {
} }
// Get an old version of a page that has been renamed. // Get an old version of a page that has been renamed.
$URLSegmentSQL = Convert::raw2sql(rawurlencode($URLSegment));
$query = new SQLQuery ( $query = new SQLQuery (
'"RecordID"', '"RecordID"',
'"SiteTree_versions"', '"SiteTree_versions"',
"\"URLSegment\" = '$URLSegment' AND \"WasPublished\" = 1" . ($useParentIDFilter ? ' AND "ParentID" = ' . (int)$parentID : ''), "\"URLSegment\" = '$URLSegmentSQL' AND \"WasPublished\" = 1" . ($useParentIDFilter ? ' AND "ParentID" = ' . (int)$parentID : ''),
'"LastEdited" DESC', '"LastEdited" DESC',
null, null,
null, null,

View File

@ -50,7 +50,7 @@ class ErrorPage extends Page {
*/ */
public static function response_for($statusCode) { public static function response_for($statusCode) {
// first attempt to dynamically generate the error page // first attempt to dynamically generate the error page
if($errorPage = DataObject::get_one('ErrorPage', "\"ErrorCode\" = $statusCode")) { if($errorPage = DataObject::get_one('ErrorPage', "\"ErrorPage\".\"ErrorCode\" = $statusCode")) {
Requirements::clear(); Requirements::clear();
Requirements::clear_combined_files(); Requirements::clear_combined_files();
@ -93,7 +93,7 @@ class ErrorPage extends Page {
$code = $defaultData['ErrorCode']; $code = $defaultData['ErrorCode'];
$page = DataObject::get_one( $page = DataObject::get_one(
'ErrorPage', 'ErrorPage',
sprintf("\"ErrorCode\" = '%s'", $code) sprintf("\"ErrorPage\".\"ErrorCode\" = '%s'", $code)
); );
$pageExists = ($page && $page->exists()); $pageExists = ($page && $page->exists());
$pagePath = self::get_filepath_for_errorcode($code); $pagePath = self::get_filepath_for_errorcode($code);

View File

@ -310,11 +310,18 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
// Grab the initial root level page to traverse down from. // Grab the initial root level page to traverse down from.
$URLSegment = array_shift($parts); $URLSegment = array_shift($parts);
$sitetree = DataObject::get_one ( $sitetree = DataObject::get_one (
'SiteTree', "\"URLSegment\" = '$URLSegment'" . (self::config()->nested_urls ? ' AND "ParentID" = 0' : ''), $cache 'SiteTree',
"\"SiteTree\".\"URLSegment\" = '$URLSegment'" . (
self::config()->nested_urls ? ' AND "SiteTree"."ParentID" = 0' : ''
),
$cache
); );
/// Fall back on a unique URLSegment for b/c. /// Fall back on a unique URLSegment for b/c.
if(!$sitetree && self::config()->nested_urls && $page = DataObject::get('SiteTree', "\"URLSegment\" = '$URLSegment'")->First()) { if(!$sitetree
&& self::config()->nested_urls
&& $page = DataObject::get_one('SiteTree', "\"SiteTree\".\"URLSegment\" = '$URLSegment'", $cache)
) {
return $page; return $page;
} }
@ -335,7 +342,9 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
// Traverse down the remaining URL segments and grab the relevant SiteTree objects. // Traverse down the remaining URL segments and grab the relevant SiteTree objects.
foreach($parts as $segment) { foreach($parts as $segment) {
$next = DataObject::get_one ( $next = DataObject::get_one (
'SiteTree', "\"URLSegment\" = '$segment' AND \"ParentID\" = $sitetree->ID", $cache 'SiteTree',
"\"SiteTree\".\"URLSegment\" = '$segment' AND \"SiteTree\".\"ParentID\" = $sitetree->ID",
$cache
); );
if(!$next) { if(!$next) {
@ -405,7 +414,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
if ( if (
!($page = DataObject::get_by_id('SiteTree', $arguments['id'])) // Get the current page by ID. !($page = DataObject::get_by_id('SiteTree', $arguments['id'])) // Get the current page by ID.
&& !($page = Versioned::get_latest_version('SiteTree', $arguments['id'])) // Attempt link to old version. && !($page = Versioned::get_latest_version('SiteTree', $arguments['id'])) // Attempt link to old version.
&& !($page = DataObject::get_one('ErrorPage', '"ErrorCode" = \'404\'')) // Link to 404 page directly. && !($page = DataObject::get_one('ErrorPage', '"ErrorPage"."ErrorCode" = \'404\'')) // Link to 404 page.
) { ) {
return; // There were no suitable matches at all. return; // There were no suitable matches at all.
} }
@ -1603,7 +1612,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
$existingPage = DataObject::get_one( $existingPage = DataObject::get_one(
'SiteTree', 'SiteTree',
"\"URLSegment\" = '$this->URLSegment' $IDFilter $parentFilter" "\"SiteTree\".\"URLSegment\" = '$this->URLSegment' $IDFilter $parentFilter"
); );
return !($existingPage); return !($existingPage);

View File

@ -10,7 +10,7 @@ class SiteTreeMaintenanceTask extends Controller {
public function makelinksunique() { public function makelinksunique() {
$badURLs = "'" . implode("', '", DB::query("SELECT URLSegment, count(*) FROM SiteTree GROUP BY URLSegment HAVING count(*) > 1")->column()) . "'"; $badURLs = "'" . implode("', '", DB::query("SELECT URLSegment, count(*) FROM SiteTree GROUP BY URLSegment HAVING count(*) > 1")->column()) . "'";
$pages = DataObject::get("SiteTree", "\"URLSegment\" IN ($badURLs)"); $pages = DataObject::get("SiteTree", "\"SiteTree\".\"URLSegment\" IN ($badURLs)");
foreach($pages as $page) { foreach($pages as $page) {
echo "<li>$page->Title: "; echo "<li>$page->Title: ";

View File

@ -136,7 +136,7 @@ class SiteTreeTest extends SapphireTest {
$oldMode = Versioned::get_reading_mode(); $oldMode = Versioned::get_reading_mode();
Versioned::reading_stage('Live'); Versioned::reading_stage('Live');
$checkSiteTree = DataObject::get_one("SiteTree", "\"URLSegment\" = 'get-one-test-page'"); $checkSiteTree = DataObject::get_one("SiteTree", "\"SiteTree\".\"URLSegment\" = 'get-one-test-page'");
$this->assertEquals("V1", $checkSiteTree->Title); $this->assertEquals("V1", $checkSiteTree->Title);
Versioned::set_reading_mode($oldMode); Versioned::set_reading_mode($oldMode);
@ -426,7 +426,7 @@ class SiteTreeTest extends SapphireTest {
public function testReadArchiveDate() { public function testReadArchiveDate() {
$date = '2009-07-02 14:05:07'; $date = '2009-07-02 14:05:07';
Versioned::reading_archived_date($date); Versioned::reading_archived_date($date);
DataObject::get('SiteTree', "\"ParentID\" = 0"); DataObject::get('SiteTree', "\"SiteTree\".\"ParentID\" = 0");
Versioned::reading_archived_date(null); Versioned::reading_archived_date(null);
$this->assertEquals( $this->assertEquals(
Versioned::get_reading_mode(), Versioned::get_reading_mode(),