Skip to content

Commit

Permalink
Return 404 for POST/PATCH requests with version > 0 for missing object
Browse files Browse the repository at this point in the history
This reverts dba4944, which changed the response code from 404 to
412, on the grounds that the HTTP spec says 412 is appropriate for an
If-Match (or If-Unmodified-Since-Version, in our case) when the entity
doesn't exist. That makes sense for a PUT, which without the If-Match
would succeed, but, per the PATCH spec (RFC 5789), a PATCH for a missing
object should return a 404, and the HTTP spec (RFC 7232) also says that
"A server MUST ignore all received preconditions if its response to the
same request without those conditions would have been a status code
other than a 2xx (Successful) or 412 (Precondition Failed)."
  • Loading branch information
dstillman committed Apr 14, 2017
1 parent 9b02cb4 commit aafda6d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 14 deletions.
11 changes: 8 additions & 3 deletions controllers/ApiController.php
Expand Up @@ -744,10 +744,15 @@ protected function checkSingleObjectWriteVersion($objectType, $obj=null, $json=f

$version = $headerVersion !== false ? $headerVersion : $propVersion;

// If object doesn't exist, version has to be 0
// If object doesn't exist, version has to be 0 if provided
if (!$obj) {
if ($version !== 0) {
$this->e404(ucwords($objectType) . " doesn't exist (to create, use version 0)");
// PATCH is only allowed for missing objects with version 0
if ($this->method == "PATCH" && $version === false) {
$this->e404(ucwords($objectType) . " not found "
. "(to create, use If-Unmodified-Since-Version: 0, JSON 'version' 0, or PUT method)");
}
if ($version > 0) {
$this->e404(ucwords($objectType) . " not found (expected version $version)");
}
return true;
}
Expand Down
6 changes: 1 addition & 5 deletions model/API.inc.php
Expand Up @@ -1246,12 +1246,8 @@ public static function checkJSONObjectVersion($object, $json, $requestParams, $r
}
// If a version is specified, the object has to exist
else if ($json->$versionProp > 0 && !$object->version) {
$code = 412;
if ($requestParams['v'] == 2) {
$code = 404;
}
throw new HTTPException(ucwords($objectType)
. " doesn't exist (expected version {$json->$versionProp}; use 0 instead)", $code);
. " doesn't exist (expected version {$json->$versionProp}; use 0 instead)", 404);
}
}
else {
Expand Down
12 changes: 6 additions & 6 deletions tests/remote/tests/API/3/VersionTest.php
Expand Up @@ -490,7 +490,7 @@ private function _testPatchExistingObjectWithoutVersion($objectType) {
}


// PATCH to a missing object with version header > 0 is a 412
// PATCH with version header > 0 to a missing object is a 404
public function testPatchMissingObjectWithVersionHeader() {
$this->_testPatchMissingObjectWithVersionHeader('collection');
$this->_testPatchMissingObjectWithVersionHeader('item');
Expand All @@ -511,11 +511,11 @@ private function _testPatchMissingObjectWithVersionHeader($objectType) {
"If-Unmodified-Since-Version: 123"
]
);
$this->assert412($response);
$this->assert404($response);
}


// PATCH to a missing object with version property > 0 is a 412
// PATCH with version property > 0 to a missing object is a 404
public function testPatchMissingObjectWithVersionProperty() {
$this->_testPatchMissingObjectWithVersionProperty('collection');
$this->_testPatchMissingObjectWithVersionProperty('item');
Expand All @@ -536,7 +536,7 @@ private function _testPatchMissingObjectWithVersionProperty($objectType) {
"Content-Type: application/json"
]
);
$this->assert412($response);
$this->assert404($response);
}


Expand Down Expand Up @@ -746,7 +746,7 @@ private function _testPatchMissingObjectsWithVersion0Property($objectType) {
}


// POST to a missing object with version > 0 is a 412 for that object
// POST with version > 0 to a missing object is a 404 for that object
public function testPatchMissingObjectsWithVersion() {
$this->_testPatchMissingObjectsWithVersion('collection');
$this->_testPatchMissingObjectsWithVersion('item');
Expand All @@ -766,7 +766,7 @@ private function _testPatchMissingObjectsWithVersion($objectType) {
json_encode([$json]),
array("Content-Type: application/json")
);
$this->assert412ForObject($response, ucwords($objectType)
$this->assert404ForObject($response, ucwords($objectType)
. " doesn't exist (expected version 123; use 0 instead)");
}

Expand Down

0 comments on commit aafda6d

Please sign in to comment.