VGalaxies commented on code in PR #2466:
URL: https://github.com/apache/hugegraph/pull/2466#discussion_r3371530844
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:
##########
@@ -128,6 +134,16 @@ public void filter(ContainerRequestContext requestContext,
}
}
+ private String getClientIP(ContainerRequestContext requestContext) {
+ try {
+ UriInfo uriInfo = requestContext.getUriInfo();
+ String host = uriInfo.getRequestUri().getHost();
Review Comment:
**Medium: Slow logs resolve the request URI host, not the client IP**
`hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java:140`
**Evidence**
- `getClientIP()` uses
`requestContext.getUriInfo().getRequestUri().getHost()`, which is the host in
the server URL requested by the client. Existing surrounding code in
`AuthenticationFilter` uses `Request.getRemoteAddr()` for the actual peer
address.
**Impact**
- Slow-query logs will attribute requests to the HugeGraph server/listener
hostname or proxy target instead of the originating client, making the new
client-IP field misleading.
**Requested fix**
- Inject/use the servlet request and log `getRemoteAddr()` or a validated
forwarded header only when HugeGraph is behind a trusted proxy.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -32,27 +39,37 @@ public class PathFilter implements ContainerRequestFilter {
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ public static final int MAX_SLOW_LOG_BODY_LENGTH = 512;
@Override
public void filter(ContainerRequestContext context) throws IOException {
- context.setProperty(REQUEST_TIME, System.currentTimeMillis());
+ long startTime = System.currentTimeMillis();
+
+ context.setProperty(REQUEST_TIME, startTime);
+
+ collectRequestParams(context);
+ }
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
+ private void collectRequestParams(ContainerRequestContext context) throws
IOException {
String method = context.getMethod();
- String requestParamsJson = "";
- if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson,
Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
+ if (method.equals(HttpMethod.POST) || method.equals(HttpMethod.PUT) ||
+ method.equals(HttpMethod.DELETE)) {
+ BufferedInputStream bufferedStream = new
BufferedInputStream(context.getEntityStream());
+
+ bufferedStream.mark(Integer.MAX_VALUE);
+ String body = IOUtils.toString(bufferedStream,
Review Comment:
**High: Slow-log capture reads full request bodies before truncating**
`hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:60`
**Evidence**
- `collectRequestParams()` wraps every POST/PUT/DELETE entity stream, calls
`bufferedStream.mark(Integer.MAX_VALUE)`, then
`IOUtils.toString(bufferedStream, ...)`, and only truncates the resulting
`String` afterward. Batch loader paths such as `VertexAPI`/`EdgeAPI` batch
imports are POST/PUT request bodies and can be large.
**Impact**
- Large loader imports are fully read and retained by the filter before the
resource method runs, causing avoidable latency and possible heap exhaustion.
The 512-byte limit does not limit memory usage because truncation happens after
the full body is loaded.
**Requested fix**
- Read at most `MAX_SLOW_LOG_BODY_LENGTH` bytes/chars for the log preview
and replay the consumed prefix plus the remaining original stream, or skip body
capture for large/batch/import endpoints and non-slow-loggable paths.
--
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]
