updateLink versioned extension regression on dev/build

[dhensby]

Affected Version

3.x-dev

Description

When running a dev/build via the browser on an empty database (or specifically without any ErrorPage records when using the CMS module) the user is logged out and sent back to /Security/login?BackURL=%2Fpage-not-found%2F%3Fstage%3DStage

Expected behaviour: for dev/build to complete successfully.

I've narrowed this down to the following:

1. DevelopmentAdmin::init() calls Versioned::reading_stage('Stage'); (

   silverstripe-framework/dev/DevelopmentAdmin.php

   Lines 76 to 79 in 47a9cdf

   	// Backwards compat: Default to "draft" stage, which is important
   	// for tasks like dev/build which call DataObject->requireDefaultRecords(),
   	// but also for other administrative tasks which have assumptions about the default stage.
   	Versioned::reading_stage('Stage');

   )
2. VersionedStateExtension::updateLink() appends stage=Stage to links when the object was queried from the draft stage (

   silverstripe-framework/model/VersionedStateExtension.php

   Lines 23 to 48 in 47a9cdf

   	public function updateLink(&$link)
   	{
   	// Skip if link already contains reading mode
   	if ($this->hasVersionedQuery($link)) {
   	return;
   	}
   	// Skip if current mode matches default mode
   	// See LeftAndMain::init() for example of this being overridden.
   	$readingMode = $this->getReadingmode();
   	if ($readingMode === Versioned::get_default_reading_mode()) {
   	return;
   	}
   	// Determine if query args are supported for the current mode
   	$queryargs = VersionedReadingMode::toQueryString($readingMode);
   	if (!$queryargs) {
   	return;
   	}
   	// Decorate
   	$link = Controller::join_links(
   	$link,
   	'?' . http_build_query($queryargs)
   	);
   	}

   ), this means switching stages after the object has been fetched (and possibly published) won't fix the problem and ?stage=Stage will still be appended to the URL.
3. ErrorPage::requireDefaultRecords() uses Director::test() to "statically publish" the error pages it generates (https://github.com/silverstripe/silverstripe-cms/blob/12826b2ace76808d29b5017e1f445514b7fcf62a/code/model/ErrorPage.php#L116)
4. VersionedRequestFilter::preRequest() takes outputting responses into it's own hands instead of returning a boolean response
5. VersionedRequestFilter::preRequest() saves the dummy session from Director::test() which causes the current session to be invalidated and the user logged out

Steps to Reproduce

1. Create a vanilla 3.x-dev site or delete the error pages from the CMS
2. Go to dev/build in the browser

Potential fix

I've fixed this locally by wrapping the ErrorPage::requireDefaultRecords() logic in Versioned::reading_stage(Versioned::LIVE); which fixes the specific error, but I feel this only resolves part of the issue.

Another fix could be to strip the ?stage query param from $page->Link() in ErrorPage::requireDefaultRecords()

---

I've opened this against framework instead of CMS because most of the root issues are in framework and the PR to framework caused this regression

PRs:

• BUG Prevent stage querystring args during dev/build #8122
• BUG Ensure errorpage is built in live mode silverstripe-cms#2181

[tractorcow]

I've fixed this with the suggested adjustment, but also reset the "default" stage to draft in dev/build in order to prevent other links being generated with querystring args. This is the same approach we used in LeftAndMain to prevent stage args in the CMS.