Remove superfluous condition around logging site and add comments

cbuescher · Oct 23, 2023 · bfe4e4c · bfe4e4c
1 parent 1d3b9fa
commit bfe4e4c
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 6 deletions.
diff --git a/server/src/main/java/org/elasticsearch/rest/RestResponse.java b/server/src/main/java/org/elasticsearch/rest/RestResponse.java
@@ -33,7 +33,6 @@
 
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE;
-import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
 import static org.elasticsearch.rest.RestController.ELASTIC_PRODUCT_HTTP_HEADER;
 
@@ -116,8 +115,7 @@ public RestResponse(RestChannel channel, Exception e) throws IOException {
     public RestResponse(RestChannel channel, RestStatus status, Exception e) throws IOException {
         this.status = status;
         ToXContent.Params params = channel.request();
-        if (params.paramAsBoolean(REST_EXCEPTION_SKIP_STACK_TRACE, REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT) && e != null) {
-            // log exception only if it is not returned in the response
+        if (e != null) {
             Supplier<?> messageSupplier = () -> String.format(
                 Locale.ROOT,
                 "path: %s, params: %s, status: %d",
@@ -131,6 +129,11 @@ public RestResponse(RestChannel channel, RestStatus status, Exception e) throws
                 SUPPRESSED_ERROR_LOGGER.warn(messageSupplier, e);
             }
         }
+        // if "error_trace" is turned on in the request, we want to render it in the rest response
+        // for that the REST_EXCEPTION_SKIP_STACK_TRACE flag that if "true" omits the stack traces is
+        // switched in the xcontent rendering parameters.
+        // For authorization problems (RestStatus.UNAUTHORIZED) we don't want to do this since this could
+        // leak information to the caller who is unauthorized to make this call
         if (params.paramAsBoolean("error_trace", false) && status != RestStatus.UNAUTHORIZED) {
             params = new ToXContent.DelegatingMapParams(singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params);
         }

diff --git a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java
@@ -476,9 +476,6 @@ public void testSupressedLogging() throws IOException {
             "unauthorized"
         );
 
-        // setting "rest.exception.stacktrace.skip" to false should prevent logging to happen
-        request.params().put(REST_EXCEPTION_SKIP_STACK_TRACE, "false");
-        assertLogging(channel, new ElasticsearchException("simulated"), null, null, null);
         // setting "error_trace" to true should not affect logging
         request.params().clear();
         request.params().put("error_trace", "true");