Skip to content

Commit

Permalink
BUG Fixed instances of loosely defined SQL predicates not qualified b…
Browse files Browse the repository at this point in the history 
…y table name

Fixed duplicate SQL escaping on SiteTree::get_by_link
  • Loading branch information
@tractorcow
tractorcow committed Aug 29, 2013
1 parent 6d694a5 commit 5f82814
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 25 deletions.
5 changes: 4 additions & 1 deletion code/controllers/CMSMain.php
View file
Expand Up @@ -814,7 +814,10 @@ public function currentPageID() {
// 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;
} }


Expand Down
2 changes: 1 addition & 1 deletion code/controllers/CMSPageAddController.php
View file
Expand Up @@ -123,7 +123,7 @@ public function doAdd($data, $form) {
$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;
} }


Expand Down
14 changes: 9 additions & 5 deletions code/controllers/ContentController.php
View file
Expand Up @@ -163,9 +163,10 @@ public function handleRequest(SS_HTTPRequest $request, DataModel $model = null)
// 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
Expand Down Expand Up @@ -258,7 +259,10 @@ public function data() {
*/ */
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();
Expand Down Expand Up @@ -399,7 +403,7 @@ public function successfullyinstalled() {
$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");
Expand Down
16 changes: 8 additions & 8 deletions code/controllers/ModelAsController.php
View file
Expand Up @@ -93,9 +93,9 @@ public function getNestedController() {
$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();
Expand Down Expand Up @@ -146,16 +146,15 @@ public function getNestedController() {
* @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;
Expand All @@ -164,10 +163,11 @@ static public function find_old_page($URLSegment,$parentID = 0, $ignoreNestedURL
} }


// 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,
Expand Down
4 changes: 2 additions & 2 deletions code/model/ErrorPage.php
View file
Expand Up @@ -50,7 +50,7 @@ public function canAddChildren($member = null) {
*/ */
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();


Expand Down Expand Up @@ -93,7 +93,7 @@ public function requireDefaultRecords() {
$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);
Expand Down
19 changes: 14 additions & 5 deletions code/model/SiteTree.php
View file
Expand Up @@ -310,11 +310,18 @@ static public function get_by_link($link, $cache = true) {
// 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;
} }


Expand All @@ -335,7 +342,9 @@ static public function get_by_link($link, $cache = true) {
// 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) {
Expand Down Expand Up @@ -405,7 +414,7 @@ static public function link_shortcode_handler($arguments, $content = null, $pars
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.
} }
Expand Down Expand Up @@ -1603,7 +1612,7 @@ function($v) {return !is_null($v);}


$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);
Expand Down
2 changes: 1 addition & 1 deletion tasks/SiteTreeMaintenanceTask.php
View file
Expand Up @@ -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: ";
Expand Down
4 changes: 2 additions & 2 deletions tests/model/SiteTreeTest.php
View file
Expand Up @@ -136,7 +136,7 @@ public function testGetOneFromLive() {
$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);
Expand Down Expand Up @@ -426,7 +426,7 @@ public function testDeleteFromLiveOperatesRecursivelyStrict() {
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(),
Expand Down

0 comments on commit 5f82814

Please sign in to comment.