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 JS document ops. 

An invalid rev should lead to a 1200 ("conflict") error rather than a
1239 ("illegal document revision") error. This is more intuitive and
in line with the corresponding change in the HTTP API. No tests needed
adjustment.
This commit is contained in:
Max Neunhoeffer 2017-02-08 10:59:04 +01:00
parent 2202f7b296
commit bde48d524d
12 changed files with 61 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG
View File

@ -14,6 +14,10 @@ devel
If-Match and If-None-Match headers from 400 (BAD) to 412 (PRECONDITION
FAILED).
* change undocumented behaviour in case of invalid revision ids in
JavaScript document operations from 1239 ("illegal document revision")
to 1200 ("conflict").
v3.2.alpha1 (2017-02-05)
------------------------

2
arangod/Cluster/ClusterMethods.cpp
View File

@ -612,7 +612,7 @@ int revisionOnCoordinator(std::string const& dbname,
char const* p = r.getString(len);
TRI_voc_rid_t cmp = TRI_StringToRid(p, len, false);
if (cmp > rid) {
if (cmp != UINT64_MAX && cmp > rid) {
// get the maximum value
rid = cmp;
}

5
arangod/GeneralServer/HttpCommTask.cpp
View File

@ -589,9 +589,8 @@ void HttpCommTask::processRequest(std::unique_ptr<HttpRequest> request) {
if (found) {
uint64_t timeStampInt =
arangodb::basics::HybridLogicalClock::decodeTimeStampWithCheck(
timeStamp);
if (timeStampInt != 0) {
arangodb::basics::HybridLogicalClock::decodeTimeStamp(timeStamp);
if (timeStampInt != 0 && timeStampInt != UINT64_MAX) {
TRI_HybridLogicalClock(timeStampInt);
}
}

2
arangod/MMFiles/MMFilesWalRecoverState.cpp
View File

@ -351,7 +351,7 @@ bool MMFilesWalRecoverState::InitialScanMarker(TRI_df_marker_t const* marker,
if (payloadSlice.isObject()) {
TRI_voc_rid_t revisionId =
Transaction::extractRevFromDocument(payloadSlice);
if (revisionId > state->maxRevisionId) {
if (revisionId != UINT64_MAX && revisionId > state->maxRevisionId) {
state->maxRevisionId = revisionId;
}
}

8
arangod/RestHandler/RestDocumentHandler.cpp
View File

@ -206,7 +206,7 @@ bool RestDocumentHandler::readSingleDocument(bool generateBody) {
TRI_voc_rid_t ifNoneRid =
extractRevision("if-none-match", isValidRevision);
if (!isValidRevision) {
ifNoneRid = 1; // an impossible rev, so precondition failed will happen
ifNoneRid = UINT64_MAX; // an impossible rev, so precondition failed will happen
}
OperationOptions options;
@ -215,7 +215,7 @@ bool RestDocumentHandler::readSingleDocument(bool generateBody) {
TRI_voc_rid_t ifRid =
extractRevision("if-match", isValidRevision);
if (!isValidRevision) {
ifRid = 1; // an impossible rev, so precondition failed will happen
ifRid = UINT64_MAX; // an impossible rev, so precondition failed will happen
}
VPackBuilder builder;
@ -392,7 +392,7 @@ bool RestDocumentHandler::modifyDocument(bool isPatch) {
bool isValidRevision;
revision = extractRevision("if-match", isValidRevision);
if (!isValidRevision) {
revision = 1; // an impossible revision, so precondition failed
revision = UINT64_MAX; // an impossible revision, so precondition failed
}
VPackSlice keyInBody = body.get(StaticStrings::KeyString);
if ((revision != 0 && TRI_ExtractRevisionId(body) != revision) ||
@ -496,7 +496,7 @@ bool RestDocumentHandler::deleteDocument() {
bool isValidRevision = false;
revision = extractRevision("if-match", isValidRevision);
if (!isValidRevision) {
revision = 1; // an impossible revision, so precondition failed
revision = UINT64_MAX; // an impossible revision, so precondition failed
}
}

4
arangod/RestHandler/RestVocbaseBaseHandler.cpp
View File

@ -595,8 +595,8 @@ TRI_voc_rid_t RestVocbaseBaseHandler::extractRevision(char const* header,
TRI_voc_rid_t rid = 0;
bool isOld;
rid = TRI_StringToRidWithCheck(s, e - s, isOld, false);
isValid = (rid != 0);
rid = TRI_StringToRid(s, e - s, isOld, false);
isValid = (rid != 0 && rid != UINT64_MAX);
return rid;
}

2
arangod/V8Server/v8-util.cpp
View File

@ -172,7 +172,7 @@ bool ExtractDocumentHandle(v8::Isolate* isolate,
}
v8::String::Utf8Value str(revObj);
bool isOld;
uint64_t rid = TRI_StringToRidWithCheck(*str, str.length(), isOld, false);
uint64_t rid = TRI_StringToRid(*str, str.length(), isOld, false);
if (rid == 0) {
return false;

51
arangod/V8Server/v8-vocbase.cpp
View File

@ -2757,29 +2757,36 @@ static void JS_DecodeRev(v8::FunctionCallbackInfo<v8::Value> const& args) {
std::string rev = TRI_ObjectToString(args[0]);
uint64_t revInt = HybridLogicalClock::decodeTimeStamp(rev);
uint64_t timeMilli = HybridLogicalClock::extractTime(revInt);
uint64_t count = HybridLogicalClock::extractCount(revInt);
time_t timeSeconds = timeMilli / 1000;
uint64_t millis = timeMilli % 1000;
struct tm date;
#ifdef _WIN32
gmtime_s(&date, &timeSeconds);
#else
gmtime_r(&timeSeconds, &date);
#endif
char buffer[32];
strftime(buffer, 32, "%Y-%m-%dT%H:%M:%S.000Z", &date);
buffer[20] = static_cast<char>(millis / 100) + '0';
buffer[21] = ((millis / 10) % 10) + '0';
buffer[22] = (millis % 10) + '0';
buffer[24] = 0;
v8::Handle<v8::Object> result = v8::Object::New(isolate);
result->Set(TRI_V8_ASCII_STRING("date"),
TRI_V8_ASCII_STRING(buffer));
result->Set(TRI_V8_ASCII_STRING("count"),
v8::Number::New(isolate, static_cast<double>(count)));
if (revInt == UINT64_MAX) {
result->Set(TRI_V8_ASCII_STRING("date"),
TRI_V8_ASCII_STRING("illegal"));
result->Set(TRI_V8_ASCII_STRING("count"),
v8::Number::New(isolate, 0.0));
} else {
uint64_t timeMilli = HybridLogicalClock::extractTime(revInt);
uint64_t count = HybridLogicalClock::extractCount(revInt);
time_t timeSeconds = timeMilli / 1000;
uint64_t millis = timeMilli % 1000;
struct tm date;
#ifdef _WIN32
gmtime_s(&date, &timeSeconds);
#else
gmtime_r(&timeSeconds, &date);
#endif
char buffer[32];
strftime(buffer, 32, "%Y-%m-%dT%H:%M:%S.000Z", &date);
buffer[20] = static_cast<char>(millis / 100) + '0';
buffer[21] = ((millis / 10) % 10) + '0';
buffer[22] = (millis % 10) + '0';
buffer[24] = 0;
result->Set(TRI_V8_ASCII_STRING("date"),
TRI_V8_ASCII_STRING(buffer));
result->Set(TRI_V8_ASCII_STRING("count"),
v8::Number::New(isolate, static_cast<double>(count)));
}
TRI_V8_RETURN(result);

10
arangod/VocBase/LogicalCollection.cpp
View File

@ -2354,8 +2354,8 @@ int LogicalCollection::replace(Transaction* trx, VPackSlice const newSlice,
VPackValueLength l;
char const* p = oldRev.getString(l);
revisionId = TRI_StringToRid(p, l, isOld, false);
if (isOld) {
// Do not tolerate old revision ticks:
if (isOld || revisionId == UINT64_MAX) {
// Do not tolerate old revision ticks or invalid ones:
revisionId = TRI_HybridLogicalClock();
}
} else {
@ -2499,8 +2499,8 @@ int LogicalCollection::remove(arangodb::Transaction* trx,
VPackValueLength l;
char const* p = oldRev.getString(l);
revisionId = TRI_StringToRid(p, l, isOld, false);
if (isOld) {
// Do not tolerate old revisions
if (isOld || revisionId == UINT64_MAX) {
// Do not tolerate old revisions or illegal ones
revisionId = TRI_HybridLogicalClock();
}
}
@ -3364,7 +3364,7 @@ int LogicalCollection::newObjectForInsert(
VPackValueLength l;
char const* p = oldRev.getString(l);
TRI_voc_rid_t oldRevision = TRI_StringToRid(p, l, isOld, false);
if (isOld) {
if (isOld || oldRevision == UINT64_MAX) {
oldRevision = TRI_HybridLogicalClock();
}
newRevSt = TRI_RidToString(oldRevision);

10
arangod/VocBase/voc-types.h
View File

@ -62,21 +62,15 @@ typedef uint64_t TRI_server_id_t;
/// @brief Convert a revision ID to a string
std::string TRI_RidToString(TRI_voc_rid_t rid);
/// @brief Convert a string into a revision ID, no check variant
/// @brief Convert a string into a revision ID, returns UINT64_MAX if invalid
TRI_voc_rid_t TRI_StringToRid(std::string const& ridStr, bool& isOld, bool warn);
/// @brief Convert a string into a revision ID, no check variant
TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool warn);
/// @brief Convert a string into a revision ID, no check variant
/// @brief Convert a string into a revision ID, returns UINT64_MAX if invalid
TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool& isOld, bool warn);
/// @brief Convert a string into a revision ID, returns 0 if format invalid
TRI_voc_rid_t TRI_StringToRidWithCheck(std::string const& ridStr, bool& isOld, bool warn);
/// @brief Convert a string into a revision ID, returns 0 if format invalid
TRI_voc_rid_t TRI_StringToRidWithCheck(char const* p, size_t len, bool& isOld, bool warn);
/// @brief enum for write operations
enum TRI_voc_document_operation_e : uint8_t {
TRI_VOC_DOCUMENT_OPERATION_UNKNOWN = 0,

47
arangod/VocBase/vocbase.cpp
View File

@ -1314,31 +1314,21 @@ std::string TRI_RidToString(TRI_voc_rid_t rid) {
return HybridLogicalClock::encodeTimeStamp(rid);
}
/// @brief Convert a string into a revision ID, no check variant
TRI_voc_rid_t TRI_StringToRid(std::string const& ridStr, bool& isOld, bool warn) {
return TRI_StringToRid(ridStr.c_str(), ridStr.size(), isOld, warn);
}
/// @brief Convert a string into a revision ID, no check variant
TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool warn) {
bool isOld;
return TRI_StringToRid(p, len, isOld, warn);
}
/// @brief Convert a string into a revision ID, no check variant
/// @brief Convert a string into a revision ID, returns 0 if format invalid
TRI_voc_rid_t TRI_StringToRid(std::string const& ridStr, bool& isOld, bool warn) {
return TRI_StringToRid(ridStr.c_str(), ridStr.size(), isOld, warn);
}
/// @brief Convert a string into a revision ID, returns 0 if format invalid
TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool& isOld, bool warn) {
if (len > 0 && *p >= '1' && *p <= '9') {
// Remove this case before the year 3887 AD because then it will
// start to clash with encoded timestamps:
char const* e = p + len;
TRI_voc_rid_t r = 0;
do {
r += *p - '0';
if (++p == e) {
break;
}
r *= 10;
} while (true);
TRI_voc_rid_t r = arangodb::basics::StringUtils::uint64_check(p, len);
if (warn && r > tickLimit) {
// An old tick value that could be confused with a time stamp
LOG(WARN)
@ -1351,26 +1341,3 @@ TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool& isOld, bool warn)
return HybridLogicalClock::decodeTimeStamp(p, len);
}
/// @brief Convert a string into a revision ID, returns 0 if format invalid
TRI_voc_rid_t TRI_StringToRidWithCheck(std::string const& ridStr, bool& isOld, bool warn) {
return TRI_StringToRidWithCheck(ridStr.c_str(), ridStr.size(), isOld, warn);
}
/// @brief Convert a string into a revision ID, returns 0 if format invalid
TRI_voc_rid_t TRI_StringToRidWithCheck(char const* p, size_t len, bool& isOld, bool warn) {
if (len > 0 && *p >= '1' && *p <= '9') {
// Remove this case before the year 3887 AD because then it will
// start to clash with encoded timestamps:
TRI_voc_rid_t r = arangodb::basics::StringUtils::uint64_check(p, len);
if (warn && r > tickLimit) {
// An old tick value that could be confused with a time stamp
LOG(WARN)
<< "Saw old _rev value that could be confused with a time stamp!";
}
isOld = true;
return r;
}
isOld = false;
return HybridLogicalClock::decodeTimeStampWithCheck(p, len);
}

19
lib/Basics/HybridLogicalClock.h
View File

@ -116,28 +116,15 @@ class HybridLogicalClock {
}
static uint64_t decodeTimeStamp(char const* p, size_t len) {
uint64_t r = 0;
for (size_t i = 0; i < len; i++) {
r = (r << 6) |
static_cast<uint8_t>(decodeTable[static_cast<uint8_t>(p[i])]);
}
return r;
}
static uint64_t decodeTimeStampWithCheck(std::string const& s) {
return decodeTimeStampWithCheck(s.c_str(), s.size());
}
static uint64_t decodeTimeStampWithCheck(char const* p, size_t len) {
// Returns 0 if format is not valid
// Returns UINT64_MAX if format is not valid
if (len > 11) {
return 0;
return UINT64_MAX;
}
uint64_t r = 0;
for (size_t i = 0; i < len; i++) {
signed char c = decodeTable[static_cast<uint8_t>(p[i])];
if (c < 0) {
return 0;
return UINT64_MAX;
}
r = (r << 6) | static_cast<uint8_t>(c);
}