wwbmmm opened a new pull request, #3592: URL: https://github.com/apache/brpc/pull/3592
### What problem does this PR solve? Problem Summary: The consul, nacos and discovery naming services parsed registry replies with rapidjson's default recursive parser. A deeply nested JSON reply from the naming-service backend crashed the consuming process with stack exhaustion (SIGSEGV). Note that iterative parsing alone is not sufficient for this input class: rapidjson also builds, visits (Value::Accept) and destroys (~GenericValue) its DOM recursively, so any unbounded nesting must be rejected before parsing. json2pb was already hardened against the same input with kParseIterativeFlag plus a recursion-depth cap; the naming-service parsers were missed by that change. ### What is changed and the side effects? Changed: - Added a shared helper `src/brpc/policy/naming_service_json.h`: `ParseNamingServiceJson()` parses replies iteratively (`kParseIterativeFlag`) and rejects, before parsing, replies nested deeper than `kMaxNamingServiceJsonDepth` (100, the same default as `json2pb_max_recursion_depth`). Depth is counted by a string-aware linear scan, so brackets inside string literals don't count and the scan can stop as soon as the limit is exceeded. - All six `Document::Parse` call sites now go through the helper and fail cleanly: consul_naming_service.cpp (service list), nacos_naming_service.cpp (auth and instance-list replies), discovery_naming_service.cpp (nodes, common result and fetchs replies). - With the nesting bounded, the remaining recursions over the document (DOM destruction, consul's error stringification, discovery's metadata conversion) are bounded to 100 levels and safe. Side effects: - Performance effects: one extra linear scan over the reply before parsing; negligible. Overly deep replies are rejected early without building a large DOM. - Breaking backward compatibility: No. Legit consul/nacos/discovery replies are only a few levels deep; replies nested deeper than 100 levels are now treated as malformed and rejected with an error log (they previously crashed the process). Tests: `test/brpc_naming_service_unittest.cpp` adds `naming_service_json_depth_limit` (valid/malformed replies, brackets inside strings, boundary at depth 100/101, 140000-deep reply) plus end-to-end regressions `consul/nacos/discovery_deeply_nested_reply` that serve a 140000-deep reply over real HTTP; `GetServers()` now returns -1 instead of crashing. Verified these tests segfault with the previous parsing behavior and pass after the fix; the rest of the suite is unaffected (the only failures in this container are pre-existing, caused by no external DNS/network, and fail identically on the base commit). --- 🤖 This PR was automatically created by brpc-oncall -- 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]
