Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Change undocumented behaviour in case of invalid rev in If-Match headers. 

An invalid rev should lead to a 412 PRECONDITION FAILED rather than a
404 BAD error. This is more intuitive, in particular since we have
never documented what valid rev strings are. Also adjust tests and
CHANGELOG.
This commit is contained in:
Max Neunhoeffer 2017-02-07 22:37:38 +01:00
parent db9c244266
commit cbcda7932c
5 changed files with 22 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGELOG
View File

@ -1,6 +1,9 @@
devel devel
----- -----
* change undocumented behaviour in case of invalid revision ids in
If-Match and If-None-Match headers from 400 (BAD) to 412 (PRECONDITION
FAILED).
v3.2.alpha1 (2017-02-05) v3.2.alpha1 (2017-02-05)
------------------------ ------------------------

12
UnitTests/HttpInterface/api-document-delete-spec.rb
View File

@ -190,20 +190,20 @@ describe ArangoDB do
hdr = { "if-match" => "\"*abcd\"" } hdr = { "if-match" => "\"*abcd\"" }
doc = ArangoDB.log_delete("#{prefix}-rev-invalid", cmd, :headers => hdr ) doc = ArangoDB.log_delete("#{prefix}-rev-invalid", cmd, :headers => hdr )
doc.code.should eq(400) doc.code.should eq(412)
doc.parsed_response['error'].should eq(true) doc.parsed_response['error'].should eq(true)
doc.parsed_response['errorNum'].should eq(400) doc.parsed_response['errorNum'].should eq(1200)
doc.parsed_response['code'].should eq(400) doc.parsed_response['code'].should eq(412)
# delete document, invalid revision # delete document, invalid revision
cmd = "/_api/document/#{did}" cmd = "/_api/document/#{did}"
hdr = { "if-match" => "'*abcd'" } hdr = { "if-match" => "'*abcd'" }
doc = ArangoDB.log_delete("#{prefix}-rev-invalid", cmd, :headers => hdr) doc = ArangoDB.log_delete("#{prefix}-rev-invalid", cmd, :headers => hdr)
doc.code.should eq(400) doc.code.should eq(412)
doc.parsed_response['error'].should eq(true) doc.parsed_response['error'].should eq(true)
doc.parsed_response['errorNum'].should eq(400) doc.parsed_response['errorNum'].should eq(1200)
doc.parsed_response['code'].should eq(400) doc.parsed_response['code'].should eq(412)
# delete document, correct revision # delete document, correct revision
cmd = "/_api/document/#{did}" cmd = "/_api/document/#{did}"

2
UnitTests/HttpInterface/api-document-read-spec.rb
View File

@ -567,7 +567,7 @@ describe ArangoDB do
hdr = { "if-match" => "'*abcd'" } hdr = { "if-match" => "'*abcd'" }
doc = ArangoDB.log_head("#{prefix}-head-rev-invalid", cmd, :headers => hdr) doc = ArangoDB.log_head("#{prefix}-head-rev-invalid", cmd, :headers => hdr)
doc.code.should eq(400) doc.code.should eq(412)
end end
end end

12
UnitTests/HttpInterface/api-document-update-spec.rb
View File

@ -266,20 +266,20 @@ describe ArangoDB do
hdr = { "if-match" => "\"*abcd\"" } hdr = { "if-match" => "\"*abcd\"" }
doc = ArangoDB.log_put("#{prefix}-rev-invalid", cmd, :headers => hdr, :body => body) doc = ArangoDB.log_put("#{prefix}-rev-invalid", cmd, :headers => hdr, :body => body)
doc.code.should eq(400) doc.code.should eq(412)
doc.parsed_response['error'].should eq(true) doc.parsed_response['error'].should eq(true)
doc.parsed_response['errorNum'].should eq(400) doc.parsed_response['errorNum'].should eq(1200)
doc.parsed_response['code'].should eq(400) doc.parsed_response['code'].should eq(412)
# update document, invalid revision # update document, invalid revision
cmd = "/_api/document/#{did}" cmd = "/_api/document/#{did}"
hdr = { "if-match" => "'*abcd'" } hdr = { "if-match" => "'*abcd'" }
doc = ArangoDB.log_put("#{prefix}-rev-invalid", cmd, :headers => hdr, :body => body) doc = ArangoDB.log_put("#{prefix}-rev-invalid", cmd, :headers => hdr, :body => body)
doc.code.should eq(400) doc.code.should eq(412)
doc.parsed_response['error'].should eq(true) doc.parsed_response['error'].should eq(true)
doc.parsed_response['errorNum'].should eq(400) doc.parsed_response['errorNum'].should eq(1200)
doc.parsed_response['code'].should eq(400) doc.parsed_response['code'].should eq(412)
# update document, correct revision # update document, correct revision
cmd = "/_api/document/#{did}" cmd = "/_api/document/#{did}"

20
arangod/RestHandler/RestDocumentHandler.cpp
View File

@ -203,23 +203,19 @@ bool RestDocumentHandler::readSingleDocument(bool generateBody) {
// check for an etag // check for an etag
bool isValidRevision; bool isValidRevision;
TRI_voc_rid_t const ifNoneRid = TRI_voc_rid_t ifNoneRid =
extractRevision("if-none-match", isValidRevision); extractRevision("if-none-match", isValidRevision);
if (!isValidRevision) { if (!isValidRevision) {
generateError(rest::ResponseCode::BAD, ifNoneRid = 1; // an impossible rev, so precondition failed will happen
TRI_ERROR_HTTP_BAD_PARAMETER, "invalid revision number");
return false;
} }
OperationOptions options; OperationOptions options;
options.ignoreRevs = true; options.ignoreRevs = true;
TRI_voc_rid_t const ifRid = TRI_voc_rid_t ifRid =
extractRevision("if-match", isValidRevision); extractRevision("if-match", isValidRevision);
if (!isValidRevision) { if (!isValidRevision) {
generateError(rest::ResponseCode::BAD, ifRid = 1; // an impossible rev, so precondition failed will happen
TRI_ERROR_HTTP_BAD_PARAMETER, "invalid revision number");
return false;
} }
VPackBuilder builder; VPackBuilder builder;
@ -396,9 +392,7 @@ bool RestDocumentHandler::modifyDocument(bool isPatch) {
bool isValidRevision; bool isValidRevision;
revision = extractRevision("if-match", isValidRevision); revision = extractRevision("if-match", isValidRevision);
if (!isValidRevision) { if (!isValidRevision) {
generateError(rest::ResponseCode::BAD, revision = 1; // an impossible revision, so precondition failed
TRI_ERROR_HTTP_BAD_PARAMETER, "invalid revision number");
return false;
} }
VPackSlice keyInBody = body.get(StaticStrings::KeyString); VPackSlice keyInBody = body.get(StaticStrings::KeyString);
if ((revision != 0 && TRI_ExtractRevisionId(body) != revision) || if ((revision != 0 && TRI_ExtractRevisionId(body) != revision) ||
@ -502,9 +496,7 @@ bool RestDocumentHandler::deleteDocument() {
bool isValidRevision = false; bool isValidRevision = false;
revision = extractRevision("if-match", isValidRevision); revision = extractRevision("if-match", isValidRevision);
if (!isValidRevision) { if (!isValidRevision) {
generateError(rest::ResponseCode::BAD, revision = 1; // an impossible revision, so precondition failed
TRI_ERROR_HTTP_BAD_PARAMETER, "invalid revision number");
return false;
} }
} }