Copilot commented on code in PR #13057:
URL: https://github.com/apache/gravitino/pull/13057#discussion_r4002089166
##########
docs/lance-rest-service.md:
##########
@@ -182,6 +183,7 @@ Configure the service by editing
`{GRAVITINO_HOME}/conf/gravitino-lance-rest-ser
| `gravitino.lance-rest.gravitino-metalake` | Gravitino metalake name |
(none) | Yes |
| `gravitino.lance-rest.httpPort` | Service port number |
9101 | No |
| `gravitino.lance-rest.host` | Service hostname |
0.0.0.0 | No |
+| `gravitino.lance-rest.includeErrorStackTrace` | Whether error responses
include stack traces | true | No |
Review Comment:
The standalone Lance documentation now advertises this opt-out, but the
packaged `conf/gravitino-lance-rest-server.conf.template` still stops at
`responseHeaderSize` and omits `includeErrorStackTrace`. Since users start
standalone deployments from that template, add the key (and its
default/comment) there so the documented setting is discoverable in the shipped
configuration.
##########
docs/iceberg-rest-service.md:
##########
@@ -123,6 +123,7 @@ Do not add them to the standalone server configuration.
| `gravitino.iceberg-rest.idleTimeout` | The timeout in ms of idle
connections.
| `30000`
| No |
| `gravitino.iceberg-rest.requestHeaderSize` | The maximum size of an
HTTP request.
| `131072`
| No |
| `gravitino.iceberg-rest.responseHeaderSize` | The maximum size of an
HTTP response.
| `131072`
| No |
+| `gravitino.iceberg-rest.includeErrorStackTrace` | Whether error responses
include server-side stack traces. Set this to `false` in new deployments
because responses can expose internal implementation details. | `true`
| No |
Review Comment:
The shared Iceberg REST configuration table now documents this setting, but
the shipped `conf/gravitino-iceberg-rest-server.conf.template` does not contain
`includeErrorStackTrace`, even though it lists the other Jetty settings. Add
the key and default/comment to the standalone template so the documented
security opt-out is available from the packaged configuration.
##########
server-common/src/main/java/org/apache/gravitino/server/web/JettyServerConfig.java:
##########
@@ -456,6 +474,10 @@ public int getResponseHeaderSize() {
return responseHeaderSize;
}
+ public boolean isIncludeErrorStackTrace() {
+ return includeErrorStackTrace;
+ }
Review Comment:
This is a new public configuration API but the getter has no Javadoc, unlike
the new config entry and constructors. Add a brief contract comment so
generated/API documentation states what the returned flag controls.
--
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]
