Skip to content

Commit b2d1900

Browse files
authored
fix(framework): handle admin IPC failures and validate HTTP media types (#7085)
1 parent 35658ae commit b2d1900

8 files changed

Lines changed: 248 additions & 18 deletions

File tree

‎framework/src/main/java/org/tron/core/services/admin/ipc/client/IpcClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ int runExec(Socket socket, String commandLine) throws IOException {
124124
System.err.println("Disconnected from server before receiving a response.");
125125
return EXIT_FAILURE;
126126
}
127-
IpcResponse parsedResponse = IpcResponse.parse(
127+
IpcResponse parsedResponse = IpcResponse.parseForExec(
128128
response, command.getRequestId(), pendingOutputFormats);
129129
if (parsedResponse.isSuccessful()) {
130130
System.out.println(parsedResponse.getFormatted());

‎framework/src/main/java/org/tron/core/services/admin/ipc/client/IpcResponse.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,14 @@ static IpcResponse parse(String response, Map<Integer, OutputFormat> pendingOutp
3434
return parse(response, null, pendingOutputFormats);
3535
}
3636

37-
/** Also verifies the numeric request ID when called for single-command execution. */
37+
/** Also verifies the numeric request ID without changing JSON-RPC result semantics. */
3838
static IpcResponse parse(String response, Integer expectedId,
3939
Map<Integer, OutputFormat> pendingOutputFormats) {
40+
return parse(response, expectedId, pendingOutputFormats, false);
41+
}
42+
43+
private static IpcResponse parse(String response, Integer expectedId,
44+
Map<Integer, OutputFormat> pendingOutputFormats, boolean checkBusinessResult) {
4045
try {
4146
JsonNode root = OBJECT_MAPPER.reader()
4247
.with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).readTree(response);
@@ -53,19 +58,32 @@ static IpcResponse parse(String response, Integer expectedId,
5358
if (root.has("error")) {
5459
return parseError(root.get("error"));
5560
}
61+
JsonNode result = root.get("result");
62+
if (checkBusinessResult && result.isObject() && result.path("success").isBoolean()
63+
&& !result.get("success").booleanValue()) {
64+
JsonNode message = result.path("errorMessage");
65+
return new IpcResponse(message.isTextual() && !message.textValue().trim().isEmpty()
66+
? message.textValue() : "Admin operation failed.", false);
67+
}
5668
if (outputFormat == OutputFormat.TEXT) {
57-
String table = new ActivePeerOutputFormatter().formatText(root.get("result"));
69+
String table = new ActivePeerOutputFormatter().formatText(result);
5870
if (table != null) {
5971
return new IpcResponse(table, true);
6072
}
6173
}
6274
// only output column "result" of jsonrpc and ignore other columns
63-
return new IpcResponse(formatJsonValue(root.get("result")), true);
75+
return new IpcResponse(formatJsonValue(result), true);
6476
} catch (JsonProcessingException e) {
6577
return invalidResponse();
6678
}
6779
}
6880

81+
/** Formats a single-command response, treating a business rejection as a command failure. */
82+
static IpcResponse parseForExec(String response, int expectedId,
83+
Map<Integer, OutputFormat> pendingOutputFormats) {
84+
return parse(response, expectedId, pendingOutputFormats, true);
85+
}
86+
6987
private static boolean isValidEnvelope(JsonNode root) {
7088
if (root == null || !root.isObject()
7189
|| !"2.0".equals(root.path("jsonrpc").textValue())

‎framework/src/main/java/org/tron/core/services/admin/ipc/server/IpcService.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public class IpcService extends AbstractService {
5555

5656
private final Set<AFUNIXSocket> activeClientSockets = ConcurrentHashMap.newKeySet();
5757
private AFUNIXServerSocket unixServerSocket;
58-
// Record paths only after this instance successfully creates the corresponding resource.
58+
// Track paths only after this instance exclusively creates the private IPC directory.
5959
private Path socketDirectoryPath;
6060
private Path socketFilePath;
6161

@@ -90,15 +90,23 @@ public void innerStart() throws Exception {
9090
try {
9191
socketFiles.createSocketDirectory(socketDirectory);
9292
socketDirectoryPath = socketDirectory;
93+
// A failed bind may already have created the socket file in our private directory.
94+
socketFilePath = resolvedSocketFile;
9395
File socketFile = resolvedSocketFile.toFile();
9496
AFUNIXSocketAddress address = AFUNIXSocketAddress.of(socketFile);
95-
unixServerSocket = AFUNIXServerSocket.bindOn(address);
96-
socketFilePath = resolvedSocketFile;
97+
unixServerSocket = AFUNIXServerSocket.newInstance();
98+
unixServerSocket.bind(address);
9799
socketFiles.setOwnerOnlyPermissions(socketFilePath);
98100
unixServerSocket.setShutdownOnClose(true);
99101

100102
logger.info("IpcService started, listening on {}", socketFile.getAbsolutePath());
101103
} catch (IOException | RuntimeException e) {
104+
logger.error("Admin IPC cannot start at {}: {}. Check socket-directory access permissions "
105+
+ "and filesystem support for Unix sockets and POSIX permission changes. Configure "
106+
+ "node.admin.ipc.socketDirectory to an existing absolute directory on a compatible "
107+
+ "local filesystem (in Docker, use a container-local directory such as /tmp). "
108+
+ "Alternatively, set node.admin.ipc.enable=false to start without Admin IPC.",
109+
resolvedSocketFile, e.toString());
102110
throw rollbackStartup(e);
103111
}
104112
Runnable runnable = () -> {

‎framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcMediaType.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package org.tron.core.services.jsonrpc;
22

33
import java.util.Locale;
4+
import java.util.regex.Pattern;
45

56
public final class JsonRpcMediaType {
67

78
private static final String APPLICATION_JSON = "application/json";
89
private static final String APPLICATION_JSON_RPC = "application/json-rpc";
10+
// RFC 6838 subtype names start with an ASCII letter or digit and contain at most 127 characters.
11+
private static final Pattern APPLICATION_JSON_SUFFIX =
12+
Pattern.compile("application/[a-z0-9][a-z0-9!#$&^_.+-]{0,121}\\+json");
913

1014
private JsonRpcMediaType() {
1115
}
@@ -21,6 +25,6 @@ public static boolean isSupported(String contentType) {
2125
.toLowerCase(Locale.ROOT);
2226
return APPLICATION_JSON.equals(mediaType)
2327
|| APPLICATION_JSON_RPC.equals(mediaType)
24-
|| mediaType.startsWith("application/") && mediaType.endsWith("+json");
28+
|| APPLICATION_JSON_SUFFIX.matcher(mediaType).matches();
2529
}
2630
}

‎framework/src/test/java/org/tron/core/services/admin/http/AdminRpcServletTest.java‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,16 +300,44 @@ public void missingContentTypeIsRejected() throws Exception {
300300
assertEquals(0, response.getContentAsByteArray().length);
301301
}
302302

303+
@Test(timeout = 30_000)
304+
public void malformedJsonContentTypesAreRejectedBeforeInvocation() throws Exception {
305+
TestAdminRpcHttpService service = new TestAdminRpcHttpService();
306+
service.setMaxRequestSize(REQUEST_LIMIT);
307+
ReflectionTestUtils.setField(service, "adminRpcServlet", servlet);
308+
String body = "{\"jsonrpc\":\"2.0\",\"method\":\"admin_listBlockedIps\",\"id\":1}";
309+
try {
310+
service.start().get(5, TimeUnit.SECONDS);
311+
for (String contentType : new String[] {"application/+json",
312+
"APPLICATION/+JSON; charset=UTF-8", "application/ +json", "application/a b+json",
313+
"application/a/b+json", "application/*+json", "application/.vendor+json",
314+
"application/vnd.tron@+json", "application/vnd.troné+json",
315+
"application/" + Strings.repeat("a", 123) + "+json"}) {
316+
String response = service.postWithContentType(body, contentType);
317+
318+
assertTrue(contentType + ": " + response,
319+
response.startsWith("HTTP/1.1 415 "));
320+
assertTrue(response, response.contains("\r\nContent-Length: 0\r\n"));
321+
assertEquals("", response.substring(response.indexOf("\r\n\r\n") + 4));
322+
verifyNoInteractions(adminJsonRpc);
323+
}
324+
} finally {
325+
service.stop().get(5, TimeUnit.SECONDS);
326+
}
327+
}
328+
303329
@Test
304330
public void jsonContentTypesAreAccepted() throws Exception {
305331
String body = "{\"jsonrpc\":\"2.0\",\"method\":\"admin_listBlockedIps\",\"id\":1}";
306-
307-
assertEquals(HttpServletResponse.SC_OK,
308-
doPost(body, "application/json; charset=UTF-8").getStatus());
309-
assertEquals(HttpServletResponse.SC_OK,
310-
doPost(body, "application/json-rpc").getStatus());
311-
assertEquals(HttpServletResponse.SC_OK,
312-
doPost(body, "application/vnd.tron+json").getStatus());
332+
String[] contentTypes = {"application/json; charset=UTF-8", "application/json-rpc",
333+
"application/vnd.tron+json", "Application/Vnd.Tron+Json; charset=UTF-8",
334+
" application/vnd.tron+json ", "application/a+json", "application/1+json",
335+
"application/a!#$&^_.+-+json", "application/" + Strings.repeat("a", 122) + "+json"};
336+
for (String contentType : contentTypes) {
337+
assertEquals(contentType, HttpServletResponse.SC_OK,
338+
doPost(body, contentType).getStatus());
339+
}
340+
verify(adminJsonRpc, times(contentTypes.length)).listBlockedIps();
313341
}
314342

315343
@Test
@@ -706,17 +734,26 @@ String post(String body, boolean chunked) throws Exception {
706734
}
707735

708736
String post(String body, boolean chunked, String accept) throws Exception {
737+
return post(body, chunked, accept, "application/json");
738+
}
739+
740+
private String post(String body, boolean chunked, String accept, String contentType)
741+
throws Exception {
709742
String framing = chunked ? "Transfer-Encoding: chunked\r\n"
710743
: "Content-Length: " + body.length() + "\r\n";
711744
String content = chunked ? Integer.toHexString(body.length()) + "\r\n"
712745
+ body + "\r\n0\r\n\r\n" : body;
713746
String request = "POST /admin HTTP/1.1\r\n"
714747
+ "Host: 127.0.0.1\r\n"
715-
+ "Content-Type: application/json\r\n"
748+
+ "Content-Type: " + contentType + "\r\n"
716749
+ (accept == null ? "" : "Accept: " + accept + "\r\n")
717750
+ "Connection: close\r\n" + framing + "\r\n" + content;
718751
return connector.getResponse(request, 5, TimeUnit.SECONDS);
719752
}
753+
754+
String postWithContentType(String body, String contentType) throws Exception {
755+
return post(body, false, null, contentType);
756+
}
720757
}
721758

722759

‎framework/src/test/java/org/tron/core/services/admin/ipc/client/IpcClientTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,78 @@ public void testExecRejectsMismatchedResponseIds() throws Exception {
309309
}
310310
}
311311

312+
@Test
313+
public void testExecReturnsFailureForBusinessRejection() throws Exception {
314+
String dynamicConfigMessage = "Peer modification is unavailable while "
315+
+ "node.dynamicConfig.enable is true; "
316+
+ "update node.active in the configuration file instead.";
317+
for (String[] scenario : new String[][] {
318+
{"admin_blockIp 192.0.2.20", "Blocked IP limit of 10000 has been reached"},
319+
{"admin_addPeer 192.0.2.20:18888", dynamicConfigMessage},
320+
{"admin_removePeer 192.0.2.20:18888", dynamicConfigMessage},
321+
{"admin_addPeer 192.0.2.20:18888",
322+
"Cannot add active node because IP 192.0.2.20 is manually blocked"}}) {
323+
ObjectNode response = OBJECT_MAPPER.createObjectNode();
324+
response.put("jsonrpc", "2.0");
325+
response.put("id", 1);
326+
response.putObject("result").put("success", false).put("changed", false)
327+
.put("disconnectedCount", 0).put("errorMessage", scenario[1]);
328+
329+
ExecResult execution = executeCommand(scenario[0], response.toString());
330+
331+
Assert.assertEquals(scenario[0], IpcClient.EXIT_FAILURE, execution.exitCode);
332+
Assert.assertEquals("", execution.standardOutput);
333+
Assert.assertEquals(scenario[1] + System.lineSeparator(), execution.errorOutput);
334+
}
335+
}
336+
337+
@Test
338+
public void testExecKeepsUnchangedOperationsSuccessful() throws Exception {
339+
String result = "{\"success\":true,\"changed\":false,\"disconnectedCount\":0,"
340+
+ "\"errorMessage\":\"\"}";
341+
String response = "{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":" + result + "}";
342+
for (String command : new String[] {"admin_blockIp 192.0.2.20",
343+
"admin_unblockIp 192.0.2.20"}) {
344+
ExecResult execution = executeCommand(command, response);
345+
346+
Assert.assertEquals(IpcClient.EXIT_SUCCESS, execution.exitCode);
347+
Assert.assertEquals(OBJECT_MAPPER.writerWithDefaultPrettyPrinter()
348+
.writeValueAsString(OBJECT_MAPPER.readTree(result)) + System.lineSeparator(),
349+
execution.standardOutput);
350+
Assert.assertEquals("", execution.errorOutput);
351+
}
352+
}
353+
354+
@Test
355+
public void testExecDoesNotCoerceBusinessSuccess() throws Exception {
356+
for (String result : new String[] {"{}", "{\"success\":null}", "{\"success\":0}",
357+
"{\"success\":\"false\"}"}) {
358+
String response = "{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":" + result + "}";
359+
ExecResult execution = executeCommand("admin_listBlockedIps", response);
360+
361+
Assert.assertEquals(result, IpcClient.EXIT_SUCCESS, execution.exitCode);
362+
Assert.assertEquals(OBJECT_MAPPER.writerWithDefaultPrettyPrinter()
363+
.writeValueAsString(OBJECT_MAPPER.readTree(result)) + System.lineSeparator(),
364+
execution.standardOutput);
365+
Assert.assertEquals("", execution.errorOutput);
366+
}
367+
}
368+
369+
@Test
370+
public void testExecBusinessRejectionHasFallbackMessage() throws Exception {
371+
for (String message : new String[] {"", ",\"errorMessage\":null",
372+
",\"errorMessage\":\" \"", ",\"errorMessage\":7"}) {
373+
String response = "{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"success\":false"
374+
+ message + "}}";
375+
ExecResult execution = executeCommand("admin_blockIp 192.0.2.20", response);
376+
377+
Assert.assertEquals(IpcClient.EXIT_FAILURE, execution.exitCode);
378+
Assert.assertEquals("", execution.standardOutput);
379+
Assert.assertEquals("Admin operation failed." + System.lineSeparator(),
380+
execution.errorOutput);
381+
}
382+
}
383+
312384
@Test
313385
public void testExecAcceptsNullAndFalseResults() throws Exception {
314386
for (String value : new String[] {"null", "false"}) {

‎framework/src/test/java/org/tron/core/services/admin/ipc/client/IpcResponseTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,34 @@ public void testStructuredResultIsPrettyPrinted() {
102102
"{", " \"height\" : 10,", " \"ready\" : true", "}"), response.getFormatted());
103103
}
104104

105+
@Test
106+
public void testBusinessRejectionOnlyFailsExec() {
107+
String response = RESPONSE_PREFIX + "\"result\":{\"success\":false,\"changed\":false,"
108+
+ "\"disconnectedCount\":0,\"errorMessage\":\"Operation rejected\"}}";
109+
110+
IpcResponse interactive = IpcResponse.parse(response, Collections.emptyMap());
111+
112+
Assert.assertTrue(interactive.isSuccessful());
113+
Assert.assertTrue(interactive.getFormatted().contains("\"success\" : false"));
114+
Assert.assertTrue(interactive.getFormatted()
115+
.contains("\"errorMessage\" : \"Operation rejected\""));
116+
117+
IpcResponse exec = IpcResponse.parseForExec(response, 1, Collections.emptyMap());
118+
119+
Assert.assertFalse(exec.isSuccessful());
120+
Assert.assertEquals("Operation rejected", exec.getFormatted());
121+
}
122+
123+
@Test
124+
public void testExecChecksResponseIdBeforeBusinessRejection() {
125+
IpcResponse response = IpcResponse.parseForExec(
126+
RESPONSE_PREFIX + "\"result\":{\"success\":false,\"errorMessage\":\"Operation rejected\"}}",
127+
2, Collections.emptyMap());
128+
129+
Assert.assertFalse(response.isSuccessful());
130+
Assert.assertEquals("IPC response ID does not match request.", response.getFormatted());
131+
}
132+
105133
@Test
106134
public void testNullAndScalarResultsAreSuccessful() {
107135
assertResponse(RESPONSE_PREFIX + "\"result\":null}", "null", true);

0 commit comments

Comments
 (0)