wwbmmm commented on code in PR #3659:
URL: https://github.com/apache/brpc/pull/3659#discussion_r4500548116
##########
src/brpc/couchbase.cpp:
##########
@@ -50,6 +51,7 @@ constexpr uint32_t APPLE_VBUCKET_COUNT ALLOW_UNUSED = 64;
constexpr uint32_t DEFAULT_VBUCKET_COUNT ALLOW_UNUSED = 1024;
constexpr int CONNECTION_ID_SIZE = 33;
constexpr size_t RANDOM_ID_HEX_SIZE = 67; // 33 bytes * 2 + null terminator
+
} // namespace
Review Comment:
[replied by brpc-oncall robot] Leftover blank line from removing the SAX
handler; please drop it so the anonymous namespace stays tight.
##########
src/brpc/couchbase.cpp:
##########
@@ -124,11 +126,8 @@ bool CouchbaseManifestManager::jsonToCollectionManifest(
return false;
}
- // Parse JSON using RapidJSON
BUTIL_RAPIDJSON_NAMESPACE::Document document;
- document.Parse(json.c_str());
-
- if (document.HasParseError()) {
+ if (!ParseJsonWithDepthLimit(json, &document)) {
DEBUG_PRINT("Failed to parse JSON: " << document.GetParseError());
Review Comment:
[replied by brpc-oncall robot] When ParseJsonWithDepthLimit() rejects the
input because of the depth limit or an embedded NUL, no parsing was attempted,
so `document.GetParseError()` is `kParseErrorNone` and this log reads "Failed
to parse JSON: No error", which is misleading when debugging a rejected
manifest. Consider logging these rejections distinctly, e.g. have the helper
report the rejection reason (depth/NUL vs. syntax error), or print the
rejection cause here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
