Skip to content

Commit

Permalink
Return 400 if <5.0.78 client tries to download object using newer schema
Browse files Browse the repository at this point in the history
**Currently in log-only mode**

Before 5.0.78, unknown or invalid-for-type properties were skipped when
saving downloaded objects, so any data in such properties would be
silently discarded if we started returning it. Starting in 5.0.78,
objects with such properties trigger an error saying to upgrade to a new
version to continue syncing all data.

This change causes us to return a 400 if pre-5.0.78 client tries to
download an object using newer properties, which prevents us from having
to cut off those older clients from syncing altogether.
  • Loading branch information
dstillman committed Mar 1, 2020
1 parent 69b07c2 commit 2a7cc04
Show file tree
Hide file tree
Showing 10 changed files with 2,938 additions and 9 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"require": {
"aws/aws-sdk-php": "^3.2",
"doctrine/cache": "1.*",
"elasticsearch/elasticsearch": "~6.0"
"elasticsearch/elasticsearch": "~6.0",
"php-ds/php-ds": "^1.2"
},
"require-dev": {
"phpunit/phpunit": "^8"
Expand Down
51 changes: 50 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions controllers/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
***** END LICENSE BLOCK *****
*/

declare(strict_types=1);

class ApiController extends Controller {
protected $writeTokenCacheTime = 43200; // 12 hours

Expand Down Expand Up @@ -63,6 +65,7 @@ class ApiController extends Controller {
protected $libraryVersion;
protected $libraryVersionOnFailure = false;
protected $headers = [];
protected $isLegacySchemaClient = false;

private $startTime = false;
private $timeLogged = false;
Expand Down Expand Up @@ -363,6 +366,14 @@ public function init($extra) {
$apiVersion = 1;
}

$this->isLegacySchemaClient = false;
if (strpos($_SERVER['HTTP_X_ZOTERO_VERSION'] ?? '', '5.0') === 0) {
require_once '../model/ToolkitVersionComparator.inc.php';
$this->isLegacySchemaClient = ToolkitVersionComparator::compare(
$_SERVER['HTTP_X_ZOTERO_VERSION'], "5.0.78"
) < 0;
}

if (!empty($extra['publications'])) {
// Query parameters not yet parsed, so check version parameter
if (($apiVersion && $apiVersion < 3)
Expand Down Expand Up @@ -1026,6 +1037,29 @@ protected function getWriteTokenCacheKey() {
}


protected function checkObjectsForLegacySchema($objectType, $jsonObjects) {
if (!$this->isLegacySchemaClient) return;

foreach ($jsonObjects as $jsonObject) {
if (!\Zotero\DataObjectUtilities::isLegacySchema($objectType, $jsonObject['data'])) {
// TEMP: Disabled during testing
//$this->outdatedClientError($this->objectLibraryID);
}
}
}


protected function outdatedClientError(int $libraryID) {
if (!Z_ENV_TESTING_SITE) return;

$type = Zotero_Libraries::getType($libraryID);
$libraryName = $type == 'user' ? 'My Library' : Zotero_Libraries::getName($libraryID);
$msg = "Some data in “{$libraryName}” was created in a newer version of Zotero and could not "
. "be downloaded. Upgrade Zotero to continue syncing this library.";
$this->e400($msg);
}


/**
* Handler for HTTP shortcut functions (e404(), e500())
*/
Expand Down
15 changes: 13 additions & 2 deletions controllers/CollectionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public function collections() {

case 'json':
$json = $collection->toResponseJSON($this->queryParams, $this->permissions);
$this->checkObjectsForLegacySchema('collection', [$json]);
echo Zotero_Utilities::formatJSON($json);
break;

Expand Down Expand Up @@ -200,7 +201,8 @@ public function collections() {
'permissions' => $this->permissions,
'head' => $this->method == 'HEAD'
];
switch ($this->queryParams['format']) {
$format = $this->queryParams['format'];
switch ($format) {
case 'atom':
$this->responseXML = Zotero_API::multiResponse(array_merge($options, [
'title' => $this->getFeedNamePrefix($this->objectLibraryID) . $title
Expand All @@ -211,7 +213,16 @@ public function collections() {
case 'keys':
case 'versions':
case 'writereport':
Zotero_API::multiResponse($options);
if ($format == 'json') {
$options['asObject'] = true;
}

$response = Zotero_API::multiResponse($options);

if ($format == 'json') {
$this->checkObjectsForLegacySchema('collection', $response);
echo Zotero_Utilities::formatJSON($response);
}
break;

default:
Expand Down
12 changes: 11 additions & 1 deletion controllers/ItemsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ public function items() {

case 'json':
$json = $item->toResponseJSON($this->queryParams, $this->permissions);
$this->checkObjectsForLegacySchema('item', [$json]);
echo Zotero_Utilities::formatJSON($json);
break;

Expand Down Expand Up @@ -672,7 +673,16 @@ private function generateMultiResponse($results, $title='') {
case 'keys':
case 'versions':
case 'writereport':
Zotero_API::multiResponse($options);
if ($format == 'json') {
$options['asObject'] = true;
}

$response = Zotero_API::multiResponse($options);

if ($format == 'json') {
$this->checkObjectsForLegacySchema('item', $response);
echo Zotero_Utilities::formatJSON($response);
}
break;

default:
Expand Down
15 changes: 13 additions & 2 deletions controllers/SearchesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public function searches() {

case 'json':
$json = $search->toResponseJSON($this->queryParams, $this->permissions);
$this->checkObjectsForLegacySchema('search', [$json]);
echo Zotero_Utilities::formatJSON($json);
break;

Expand Down Expand Up @@ -137,7 +138,8 @@ public function searches() {
'permissions' => $this->permissions,
'head' => $this->method == 'HEAD'
];
switch ($this->queryParams['format']) {
$format = $this->queryParams['format'];
switch ($format) {
case 'atom':
$this->responseXML = Zotero_API::multiResponse(array_merge($options, [
'title' => $this->getFeedNamePrefix($this->objectLibraryID) . $title
Expand All @@ -148,7 +150,16 @@ public function searches() {
case 'keys':
case 'versions':
case 'writereport':
Zotero_API::multiResponse($options);
if ($format == 'json') {
$options['asObject'] = true;
}

$response = Zotero_API::multiResponse($options);

if ($format == 'json') {
$this->checkObjectsForLegacySchema('search', $response);
echo Zotero_Utilities::formatJSON($response);
}
break;

default:
Expand Down
Loading

0 comments on commit 2a7cc04

Please sign in to comment.