Skip to content

Commit 5e5063d

Browse files
committed
fix: unify the return value and block non-web request
1 parent 339117d commit 5e5063d

1 file changed

Lines changed: 28 additions & 48 deletions

File tree

  • hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller

hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ public ResponseEntity<Map<String, Object>> getPartitions(
113113
rafts.add(raft);
114114
}
115115

116-
return ResponseEntity.status(HttpStatus.OK).body(okMap("partitions", rafts));
116+
return ok("partitions", rafts);
117117
}
118118

119119
@GetMapping(value = "/partition/{id}", produces = "application/json")
120-
public Raft getPartition(@PathVariable(value = "id") int id) {
120+
public ResponseEntity<Map<String, Object>> getPartition(@PathVariable(value = "id") int id) {
121121

122122
HgStoreEngine storeEngine = nodeService.getStoreEngine();
123123

@@ -142,8 +142,7 @@ public Raft getPartition(@PathVariable(value = "id") int id) {
142142
raft.getPartitions().add(partition);
143143
}
144144

145-
return raft;
146-
//return okMap("partition", rafts);
145+
return ok("raft", raft);
147146
}
148147

149148
/**
@@ -172,7 +171,7 @@ public ResponseEntity<Map<String, Object>> dumpPartition(
172171
}
173172
cfIterator.close();
174173
});
175-
return ResponseEntity.status(HttpStatus.OK).body(okMap("ok", null));
174+
return ok("ok", null);
176175
}
177176

178177
/**
@@ -187,29 +186,24 @@ public ResponseEntity<Map<String, Object>> cleanPartition(
187186
storeEngine.getPartitionEngine(id).getPartitions().forEach((graph, partition) -> {
188187
handler.cleanPartition(graph, id);
189188
});
190-
return ResponseEntity.status(HttpStatus.OK).body(okMap("ok", null));
189+
return ok("ok", null);
191190
}
192191

193192
@GetMapping(value = "/arthasstart", produces = "application/json")
194193
public ResponseEntity<Map<String, Object>> arthasstart(
195194
@RequestParam(required = false, defaultValue = "") String flags,
196195
HttpServletRequest request) {
197-
if (request == null) {
198-
log.error("Security Alert: HttpServletRequest is NULL when accessing /arthasstart. " +
199-
"Access denied.");
200-
Map<String, Object> err = new HashMap<>();
201-
err.put("status", "ERROR");
202-
err.put("message", "Internal Error: Cannot determine client IP.");
203-
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(err);
204-
}
205-
String remoteAddr = getClientIp(request);
196+
// Ignore proxy headers to prevent IP spoofing.
197+
// NOTE: If behind a reverse proxy (e.g., Nginx), getRemoteAddr() returns the proxy's IP.
198+
// Ensure the proxy is configured to block untrusted external access.
199+
String remoteAddr = getCleanIp(request.getRemoteAddr());
206200
boolean isLocalRequest =
207-
"127.0.0.1".equals(remoteAddr) || "[0:0:0:0:0:0:0:1]".equals(remoteAddr);
201+
"127.0.0.1".equals(remoteAddr) || "0:0:0:0:0:0:0:1".equals(remoteAddr) ||
202+
"::1".equals(remoteAddr);
208203
if (!isLocalRequest) {
209204
List<String> ret = new ArrayList<>();
210205
ret.add("Arthas start is ONLY allowed from localhost.");
211-
return ResponseEntity.status(HttpStatus.FORBIDDEN)
212-
.body(forbiddenMap("arthasstart", ret));
206+
return forbidden("arthasstart", ret);
213207
}
214208
HashMap<String, String> configMap = new HashMap<>();
215209
configMap.put("arthas.telnetPort", appConfig.getArthasConfig().getTelnetPort());
@@ -220,7 +214,7 @@ public ResponseEntity<Map<String, Object>> arthasstart(
220214
// DashResponse retPose = new DashResponse();
221215
List<String> ret = new ArrayList<>();
222216
ret.add("Arthas started successfully");
223-
return ResponseEntity.status(HttpStatus.OK).body(okMap("arthasstart", ret));
217+
return ok("arthasstart", ret);
224218
}
225219

226220
@PostMapping("/compat")
@@ -237,42 +231,28 @@ public ResponseEntity<Map<String, Object>> compact(@RequestParam(value = "id") i
237231
map.put("msg",
238232
"compaction task fail to submit, and there could be another task in progress");
239233
}
240-
return ResponseEntity.status(HttpStatus.OK).body(map);
234+
return ok("body", map);
241235
}
242236

243-
public Map<String, Object> okMap(String k, Object v) {
244-
Map<String, Object> map = new HashMap<>();
245-
map.put("status", 0);
246-
map.put(k, v);
247-
return map;
237+
public ResponseEntity<Map<String, Object>> ok(String k, Object v) {
238+
return ResponseEntity.ok(Map.of("status", 200, k, v));
248239
}
249240

250-
public Map<String, Object> forbiddenMap(String k, Object v) {
251-
HashMap<String, Object> map = new HashMap<>();
252-
map.put("status", 403);
253-
map.put(k, v);
254-
return map;
241+
public ResponseEntity<Map<String, Object>> forbidden(String k, Object v) {
242+
return ResponseEntity.status(HttpStatus.FORBIDDEN)
243+
.body(Map.of("status", 403, k, v));
255244
}
256245

257-
private String getClientIp(HttpServletRequest request) {
258-
String ip = request.getHeader("X-Forwarded-For");
259-
if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) {
260-
ip = request.getHeader("Proxy-Client-IP");
261-
}
262-
if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) {
263-
ip = request.getHeader("WL-Proxy-Client-IP");
264-
}
265-
if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) {
266-
ip = request.getHeader("X-Real-IP");
267-
}
268-
if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) {
269-
ip = request.getRemoteAddr();
270-
}
271-
272-
if (ip != null && ip.contains(",")) {
273-
ip = ip.split(",")[0].trim();
246+
private String getCleanIp(String remoteAddr) {
247+
if (remoteAddr != null) {
248+
if (remoteAddr.startsWith("[")) {
249+
remoteAddr = remoteAddr.substring(1);
250+
}
251+
if (remoteAddr.endsWith("]")) {
252+
remoteAddr = remoteAddr.substring(0, remoteAddr.length() - 1);
253+
}
274254
}
275-
return ip;
255+
return remoteAddr;
276256
}
277257

278258
@Data

0 commit comments

Comments
 (0)