Skip to content

Commit

Permalink
Force live object on unserialise
Browse files Browse the repository at this point in the history
This corrects behaviour that potentially allows indexing of draft content. Queues are normally run via a "dev task" which is run in the DevelopmentAdmin context. This controller will set the reading mode to DRAFT. The upshot of this is that an Index job will unserialise a DataObject in DRAFT reading mode (saved as a Data Query param) and the subsequent `toArray` call could get draft content. This is often seen as a link indexed with `?stage=Stage` parameters.

The impact is likely limited because an IndexJob is usually queued by a publish action and run shortly afterwards. In these cases it is unlikely that draft changes are present - but is isn't impossible.

I checked the ReIndexJob run and it doesn't appear to have this issue because it sets the reading mode in the setup method. I have instead added this to the unserialise function as it seemed a safer method to ensure the reading mode was as expected.
  • Loading branch information
blueo committed Apr 7, 2024
1 parent 6081ac9 commit 5e1579b
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 8 deletions.
28 changes: 21 additions & 7 deletions src/DataObject/DataObjectDocument.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
use SilverStripe\SearchService\Service\Traits\ConfigurationAware;
use SilverStripe\SearchService\Service\Traits\ServiceAware;
use SilverStripe\Security\Member;
use SilverStripe\Versioned\ReadingMode;
use SilverStripe\Versioned\Versioned;
use SilverStripe\Versioned\VersionedStateExtension;
use SilverStripe\View\ViewableData;

class DataObjectDocument implements
Expand Down Expand Up @@ -600,16 +602,28 @@ public function __serialize(): array

public function __unserialize(array $data): void
{
$dataObject = DataObject::get_by_id($data['className'], $data['id']);
if (DataObject::has_extension($data['className'], Versioned::class)) {
// get data object in live mode always - needed as queue runners normally use a dev task to run
// so will have a DRAFT reading mode @see SilverStripe\Dev\DevelopmentAdmin
$dataObject = Versioned::withVersionedMode(function () use ($data): ?DataObject {
Versioned::set_stage(Versioned::LIVE);

if (!$dataObject && DataObject::has_extension($data['className'], Versioned::class) && $data['fallback']) {
// get the latest version - usually this is an object that has been deleted
$dataObject = Versioned::get_latest_version(
$data['className'],
$data['id']
);
return DataObject::get_by_id($data['className'], $data['id']);
});

if (!$dataObject && $data['fallback']) {
// get the latest version - usually this is an object that has been deleted
$dataObject = Versioned::get_latest_version(
$data['className'],
$data['id']
);
}
} else {
// un-versioned object so we don't need to worry about stages and fallbacks do not exist
$dataObject = DataObject::get_by_id($data['className'], $data['id']);
}


if (!$dataObject) {
throw new Exception(sprintf('DataObject %s : %s does not exist', $data['className'], $data['id']));
}
Expand Down
48 changes: 47 additions & 1 deletion tests/DataObject/DataObjectDocumentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
use SilverStripe\SearchService\Tests\Fake\DataObjectFakeVersioned;
use SilverStripe\SearchService\Tests\Fake\DataObjectSubclassFake;
use SilverStripe\SearchService\Tests\Fake\ImageFake;
use SilverStripe\SearchService\Tests\Fake\PageFake;
use SilverStripe\SearchService\Tests\Fake\TagFake;
use SilverStripe\SearchService\Tests\SearchServiceTest;
use SilverStripe\Security\Member;
use SilverStripe\Subsites\Model\Subsite;
use SilverStripe\Versioned\Versioned;

class DataObjectDocumentTest extends SearchServiceTest
{
Expand Down Expand Up @@ -612,7 +614,7 @@ public function testDeletedDataObject(): void
$id = $dataObject->ID;

$doc = DataObjectDocument::create($dataObject)->setShouldFallbackToLatestVersion(true);
$dataObject->delete();
$dataObject->doArchive();

/** @var DataObjectDocument $serialDoc */
$serialDoc = unserialize(serialize($doc));
Expand All @@ -626,4 +628,48 @@ public function testDeletedDataObject(): void
unserialize(serialize($doc));
}

public function testToArrayInQueueRun(): void
{
$config = $this->mockConfig();
$config->set('getSearchableClasses', [
PageFake::class,
]);
$config->set('getFieldsForClass', [
PageFake::class => [
new Field('title'),
new Field('page_link', 'Link'),
],
]);

Versioned::withVersionedMode(function (): void {
// Reading mode as if run by job - development Admin sets Draft reading mode
// usually via /dev/tasks/ProcessJobQueueTask
// @see SilverStripe\Dev\DevelopmentAdmin
Versioned::set_stage(Versioned::DRAFT);

$dataObject = PageFake::create();
$dataObject->Title = 'Published';
$dataObject->write();
$dataObject->publishRecursive();
$dataObject->Title = 'Draft';
$dataObject->write();

$doc = DataObjectDocument::create($dataObject);

/** @var DataObjectDocument $serialDoc */
$serialDoc = unserialize(serialize($doc));
$indexData = $serialDoc->toArray();

$this->assertEqualsCanonicalizing(
[
'page_content' => '',
'title' => 'Published',
'page_link' => '/published',
],
$indexData,
'Potential draft content being indexed'
);
});
}

}
20 changes: 20 additions & 0 deletions tests/Fake/PageFake.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace SilverStripe\SearchService\Tests\Fake;

use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Dev\TestOnly;
use SilverStripe\SearchService\Extensions\SearchServiceExtension;

/**
* @property string $Title
* @property int $ShowInSearch
* @property int $Sort
* @mixin SearchServiceExtension
*/
class PageFake extends SiteTree implements TestOnly
{

private static string $table_name = 'PageFake';

}

0 comments on commit 5e1579b

Please sign in to comment.