feat(jsonrpc): add resource restirct for jsonrpc#69
feat(jsonrpc): add resource restirct for jsonrpc#69
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java">
<violation number="1" location="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java:129">
P2: Race condition after timeout: `future.cancel(true)` only sends an interrupt; the RPC handler may still be running. Since `BufferedResponseWrapper` doesn't override header methods (`setStatus`, `setContentType`, etc.), the handler thread writes headers directly to the underlying `resp` concurrently with `writeJsonRpcError`. This can corrupt the HTTP response. Consider either wrapping header methods in `BufferedResponseWrapper` too, or awaiting actual thread termination before writing the error.</violation>
<violation number="2" location="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java:155">
P1: No limit on request body size. `readBody` loads the entire input stream into memory before any validation runs. A malicious client can send a multi-gigabyte body to cause OOM. Add a maximum request body size check (e.g., reject if total bytes read exceeds a configured limit).</violation>
<violation number="3" location="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java:168">
P1: The `message` string is concatenated into the JSON response without JSON-escaping. If `message` contains `"`, `\`, or control characters, the output is malformed JSON. Use the `ObjectMapper` (already available as `MAPPER`) to build the JSON safely, or at minimum escape the string.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| resp.getOutputStream().flush(); | ||
| } | ||
|
|
||
| private byte[] readBody(InputStream in) throws IOException { |
There was a problem hiding this comment.
P1: No limit on request body size. readBody loads the entire input stream into memory before any validation runs. A malicious client can send a multi-gigabyte body to cause OOM. Add a maximum request body size check (e.g., reject if total bytes read exceeds a configured limit).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java, line 155:
<comment>No limit on request body size. `readBody` loads the entire input stream into memory before any validation runs. A malicious client can send a multi-gigabyte body to cause OOM. Add a maximum request body size check (e.g., reject if total bytes read exceeds a configured limit).</comment>
<file context>
@@ -66,6 +95,83 @@ public Integer getJsonRpcCode(int httpStatusCode) {
+ resp.getOutputStream().flush();
+ }
+
+ private byte[] readBody(InputStream in) throws IOException {
+ ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+ byte[] tmp = new byte[4096];
</file context>
| String body = "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":" + error.code | ||
| + ",\"message\":\"" + message + "\"},\"id\":" + idStr + "}"; |
There was a problem hiding this comment.
P1: The message string is concatenated into the JSON response without JSON-escaping. If message contains ", \, or control characters, the output is malformed JSON. Use the ObjectMapper (already available as MAPPER) to build the JSON safely, or at minimum escape the string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java, line 168:
<comment>The `message` string is concatenated into the JSON response without JSON-escaping. If `message` contains `"`, `\`, or control characters, the output is malformed JSON. Use the `ObjectMapper` (already available as `MAPPER`) to build the JSON safely, or at minimum escape the string.</comment>
<file context>
@@ -66,6 +95,83 @@ public Integer getJsonRpcCode(int httpStatusCode) {
+ private void writeJsonRpcError(HttpServletResponse resp, JsonRpcError error, String message,
+ JsonNode id) throws IOException {
+ String idStr = (id != null && !id.isNull() && !id.isMissingNode()) ? id.toString() : "null";
+ String body = "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":" + error.code
+ + ",\"message\":\"" + message + "\"},\"id\":" + idStr + "}";
+ byte[] bytes = body.getBytes(StandardCharsets.UTF_8);
</file context>
| String body = "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":" + error.code | |
| + ",\"message\":\"" + message + "\"},\"id\":" + idStr + "}"; | |
| String escapedMessage = MAPPER.writeValueAsString(message); | |
| String body = "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":" + error.code | |
| + ",\"message\":" + escapedMessage + "},\"id\":" + idStr + "}"; |
| try { | ||
| future.get(timeoutSec, TimeUnit.SECONDS); | ||
| } catch (TimeoutException e) { | ||
| future.cancel(true); |
There was a problem hiding this comment.
P2: Race condition after timeout: future.cancel(true) only sends an interrupt; the RPC handler may still be running. Since BufferedResponseWrapper doesn't override header methods (setStatus, setContentType, etc.), the handler thread writes headers directly to the underlying resp concurrently with writeJsonRpcError. This can corrupt the HTTP response. Consider either wrapping header methods in BufferedResponseWrapper too, or awaiting actual thread termination before writing the error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java, line 129:
<comment>Race condition after timeout: `future.cancel(true)` only sends an interrupt; the RPC handler may still be running. Since `BufferedResponseWrapper` doesn't override header methods (`setStatus`, `setContentType`, etc.), the handler thread writes headers directly to the underlying `resp` concurrently with `writeJsonRpcError`. This can corrupt the HTTP response. Consider either wrapping header methods in `BufferedResponseWrapper` too, or awaiting actual thread termination before writing the error.</comment>
<file context>
@@ -66,6 +95,83 @@ public Integer getJsonRpcCode(int httpStatusCode) {
+ try {
+ future.get(timeoutSec, TimeUnit.SECONDS);
+ } catch (TimeoutException e) {
+ future.cancel(true);
+ JsonNode idNode = (!rootNode.isArray()) ? rootNode.get("id") : null;
+ writeJsonRpcError(resp, JsonRpcError.TIMEOUT, "Request timeout after " + timeoutSec + "s",
</file context>
1 participant
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
Summary by cubic
Add configurable resource limits to JSON-RPC to cap batch size, response size, request time, and address filter size. This reduces memory use and protects nodes from oversized or slow requests.
node.jsonrpc.maxBatchSize(10),node.jsonrpc.maxResponseSize(25 MB),node.jsonrpc.maxRequestTimeout(30s),node.jsonrpc.maxAddressSize(1000).JsonRpcServlet: buffered response with early size checks, cached request body, and per-request timeout.-32005(limit exceeded),-32003(response too large),-32002(timeout). Defaults set inCommonParameterand loaded viaArgs; keys added toconfig.conf.Written for commit 7b33166. Summary will update on new commits.