imbajin commented on code in PR #2945:
URL:
https://github.com/apache/incubator-hugegraph/pull/2945#discussion_r2731182687
##########
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java:
##########
@@ -189,6 +192,16 @@ public Map<String, Object>
cleanPartition(@PathVariable(value = "id") int id) th
@GetMapping(value = "/arthasstart", produces = "application/json")
public Map<String, Object> arthasstart(
@RequestParam(required = false, defaultValue = "") String flags) {
+ String remoteAddr = ((ServletRequestAttributes) Objects.requireNonNull(
+
RequestContextHolder.getRequestAttributes())).getRequest().getRemoteAddr();
+
+ boolean isLocalRequest = "127.0.0.1".equals(remoteAddr) ||
Review Comment:
⚠️ **Security: IP check can be bypassed via reverse proxy**
Using only `getRemoteAddr()` is insufficient when the server is behind a
reverse proxy (like Nginx). The remote address will be the proxy's IP (often
127.0.0.1), not the actual client's IP.
Consider checking `X-Forwarded-For` or `X-Real-IP` headers as well, or
document that this endpoint should not be exposed behind a proxy:
```java
private boolean isLocalRequest(HttpServletRequest request) {
String remoteAddr = request.getRemoteAddr();
// Also check forwarded headers when behind proxy
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor \!= null && \!forwardedFor.isEmpty()) {
// Take the first IP (original client)
remoteAddr = forwardedFor.split(",")[0].trim();
}
return "127.0.0.1".equals(remoteAddr) ||
"::1".equals(remoteAddr) ||
"[0:0:0:0:0:0:0:1]".equals(remoteAddr);
}
```
Note: The check for IPv6 localhost should also include `::1` (the standard
short form).
##########
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java:
##########
@@ -189,6 +192,16 @@ public Map<String, Object>
cleanPartition(@PathVariable(value = "id") int id) th
@GetMapping(value = "/arthasstart", produces = "application/json")
public Map<String, Object> arthasstart(
@RequestParam(required = false, defaultValue = "") String flags) {
+ String remoteAddr = ((ServletRequestAttributes) Objects.requireNonNull(
Review Comment:
⚠️ **Potential NPE risk with Objects.requireNonNull**
If `RequestContextHolder.getRequestAttributes()` returns null (e.g., in
non-web contexts or async threads), this will throw a `NullPointerException`
with no helpful message.
Consider using a cleaner approach with Spring's `HttpServletRequest`
injection:
```java
@GetMapping(value = "/arthasstart", produces = "application/json")
public Map<String, Object> arthasstart(
HttpServletRequest request,
@RequestParam(required = false, defaultValue = "") String flags) {
String remoteAddr = request.getRemoteAddr();
// ...
}
```
This is more idiomatic for Spring controllers and avoids the convoluted
`RequestContextHolder` usage.
--
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]
