SOLR-14886 : suppress stack traces in query response (#1632)

Added a property "hideStackTrace" to solr.xml or system property 'solr.hideStackTrace' to not include error stack trace in the response. --------- Co-authored-by: Christine Poerschke <[email protected]> Co-authored-by: Alex Deparvu <[email protected]> (cherry picked from commit cf30265)
apache · Sep 12, 2023 · 2f69584 · 2f69584
1 parent 79b9c80
commit 2f69584
Show file tree

Hide file tree

Showing 18 changed files with 317 additions and 26 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -61,6 +61,7 @@ Improvements
 
 * SOLR-16970: SOLR_OPTS is now able to override options set by the Solr control scripts, "bin/solr" and "bin/solr.cmd". (Houston Putman)
 
+* SOLR-14886: Suppress stack traces in query response (Isabelle Giguere via Alex Deparvu)
 
 Optimizations
 ---------------------

diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -2447,6 +2447,10 @@ public long getStatus() {
     return status;
   }
 
+  public boolean hideStackTrace() {
+    return cfg.hideStackTraces();
+  }
+
   /**
    * Retrieve the aliases from zookeeper. This is typically cached and does not hit zookeeper after
    * the first use.

diff --git a/solr/core/src/java/org/apache/solr/core/NodeConfig.java b/solr/core/src/java/org/apache/solr/core/NodeConfig.java
@@ -67,6 +67,8 @@ public class NodeConfig {
 
   private final List<String> allowUrls;
 
+  private final boolean hideStackTraces;
+
   private final String sharedLibDirectory;
 
   private final String modules;
@@ -153,6 +155,7 @@ private NodeConfig(
       String defaultZkHost,
       Set<Path> allowPaths,
       List<String> allowUrls,
+      boolean hideStackTraces,
       String configSetServiceClass,
       String modules,
       Set<String> hiddenSysProps) {
@@ -189,6 +192,7 @@ private NodeConfig(
     this.defaultZkHost = defaultZkHost;
     this.allowPaths = allowPaths;
     this.allowUrls = allowUrls;
+    this.hideStackTraces = hideStackTraces;
     this.configSetServiceClass = configSetServiceClass;
     this.modules = modules;
     this.hiddenSysProps = hiddenSysProps;
@@ -452,6 +456,10 @@ public List<String> getAllowUrls() {
     return allowUrls;
   }
 
+  public boolean hideStackTraces() {
+    return hideStackTraces;
+  }
+
   // Configures SOLR_HOME/lib to the shared class loader
   private void setupSharedLib() {
     // Always add $SOLR_HOME/lib to the shared resource loader
@@ -615,6 +623,7 @@ public static class NodeConfigBuilder {
     private String defaultZkHost;
     private Set<Path> allowPaths = Collections.emptySet();
     private List<String> allowUrls = Collections.emptyList();
+    private boolean hideStackTrace = Boolean.getBoolean("solr.hideStackTrace");
 
     private final Path solrHome;
     private final String nodeName;
@@ -809,6 +818,11 @@ public NodeConfigBuilder setAllowUrls(List<String> urls) {
       return this;
     }
 
+    public NodeConfigBuilder setHideStackTrace(boolean hide) {
+      this.hideStackTrace = hide;
+      return this;
+    }
+
     public NodeConfigBuilder setConfigSetServiceClass(String configSetServiceClass) {
       this.configSetServiceClass = configSetServiceClass;
       return this;
@@ -902,6 +916,7 @@ public NodeConfig build() {
           defaultZkHost,
           allowPaths,
           allowUrls,
+          hideStackTrace,
           configSetServiceClass,
           modules,
           resolveHiddenSysPropsFromSysPropOrEnvOrDefault(hiddenSysProps));

diff --git a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
@@ -372,6 +372,9 @@ private static NodeConfig fillSolrSection(NodeConfig.NodeConfigBuilder builder,
               case "allowPaths":
                 builder.setAllowPaths(separatePaths(it.txt()));
                 break;
+              case "hideStackTrace":
+                builder.setHideStackTrace(it.boolVal(false));
+                break;
               case "configSetBaseDir":
                 builder.setConfigSetBaseDirectory(it.txt());
                 break;

diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
@@ -906,9 +906,11 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {
             t = ((SolrServerException) t).getCause();
           }
           nl.add("error", t.toString());
-          StringWriter trace = new StringWriter();
-          t.printStackTrace(new PrintWriter(trace));
-          nl.add("trace", trace.toString());
+          if (!rb.req.getCore().getCoreContainer().hideStackTrace()) {
+            StringWriter trace = new StringWriter();
+            t.printStackTrace(new PrintWriter(trace));
+            nl.add("trace", trace.toString());
+          }
           if (srsp.getShardAddress() != null) {
             nl.add("shardAddress", srsp.getShardAddress());
           }
@@ -1297,9 +1299,11 @@ protected void returnFields(ResponseBuilder rb, ShardRequest sreq) {
                 t = ((SolrServerException) t).getCause();
               }
               nl.add("error", t.toString());
-              StringWriter trace = new StringWriter();
-              t.printStackTrace(new PrintWriter(trace));
-              nl.add("trace", trace.toString());
+              if (!rb.req.getCore().getCoreContainer().hideStackTrace()) {
+                StringWriter trace = new StringWriter();
+                t.printStackTrace(new PrintWriter(trace));
+                nl.add("trace", trace.toString());
+              }
             }
           }
 

diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
@@ -601,9 +601,11 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
           }
         }
         nl.add("error", cause.toString());
-        StringWriter trace = new StringWriter();
-        cause.printStackTrace(new PrintWriter(trace));
-        nl.add("trace", trace.toString());
+        if (!core.getCoreContainer().hideStackTrace()) {
+          StringWriter trace = new StringWriter();
+          cause.printStackTrace(new PrintWriter(trace));
+          nl.add("trace", trace.toString());
+        }
       } else if (rb.getResults() != null) {
         nl.add("numFound", rb.getResults().docList.matches());
         nl.add(

diff --git a/solr/core/src/java/org/apache/solr/jersey/CatchAllExceptionMapper.java b/solr/core/src/java/org/apache/solr/jersey/CatchAllExceptionMapper.java
@@ -108,8 +108,12 @@ public static Response buildExceptionResponse(
         containerRequestContext.getProperty(SOLR_JERSEY_RESPONSE) == null
             ? new SolrJerseyResponse()
             : (SolrJerseyResponse) containerRequestContext.getProperty(SOLR_JERSEY_RESPONSE);
-
-    response.error = ResponseUtils.getTypedErrorInfo(normalizedException, log);
+    response.error =
+        ResponseUtils.getTypedErrorInfo(
+            normalizedException,
+            log,
+            solrQueryRequest.getCore() != null
+                && solrQueryRequest.getCore().getCoreContainer().hideStackTrace());
     response.responseHeader.status = response.error.code;
     final String mediaType =
         V2ApiUtils.getMediaTypeFromWtParam(solrQueryRequest, MediaType.APPLICATION_JSON);

diff --git a/solr/core/src/java/org/apache/solr/rest/BaseSolrResource.java b/solr/core/src/java/org/apache/solr/rest/BaseSolrResource.java
@@ -143,7 +143,12 @@ protected void handleException(Logger log) {
     Exception exception = getSolrResponse().getException();
     if (null != exception) {
       NamedList<Object> info = new SimpleOrderedMap<>();
-      this.statusCode = ResponseUtils.getErrorInfo(exception, info, log);
+      this.statusCode =
+          ResponseUtils.getErrorInfo(
+              exception,
+              info,
+              log,
+              solrCore != null && solrCore.getCoreContainer().hideStackTrace());
       getSolrResponse().add("error", info);
       String message = (String) info.get("msg");
       if (null != message && !message.trim().isEmpty()) {

diff --git a/...solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java b/...solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java
@@ -89,9 +89,11 @@ public void process(ResponseBuilder rb, ShardRequest shardRequest) {
             t = t.getCause();
           }
           nl.add("error", t.toString());
-          StringWriter trace = new StringWriter();
-          t.printStackTrace(new PrintWriter(trace));
-          nl.add("trace", trace.toString());
+          if (!rb.req.getCore().getCoreContainer().hideStackTrace()) {
+            StringWriter trace = new StringWriter();
+            t.printStackTrace(new PrintWriter(trace));
+            nl.add("trace", trace.toString());
+          }
         } else {
           nl.add("numFound", response.get("totalHitCount"));
         }

diff --git a/...e/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java b/...e/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java
@@ -101,9 +101,11 @@ public void process(ResponseBuilder rb, ShardRequest shardRequest) {
             t = ((SolrServerException) t).getCause();
           }
           individualShardInfo.add("error", t.toString());
-          StringWriter trace = new StringWriter();
-          t.printStackTrace(new PrintWriter(trace));
-          individualShardInfo.add("trace", trace.toString());
+          if (!rb.req.getCore().getCoreContainer().hideStackTrace()) {
+            StringWriter trace = new StringWriter();
+            t.printStackTrace(new PrintWriter(trace));
+            individualShardInfo.add("trace", trace.toString());
+          }
         } else {
           // summary for successful shard response is added down below
         }

diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -843,7 +843,12 @@ protected void sendError(Throwable ex) throws IOException {
       try {
         if (exp != null) {
           SimpleOrderedMap<Object> info = new SimpleOrderedMap<>();
-          int code = ResponseUtils.getErrorInfo(ex, info, log);
+          int code =
+              ResponseUtils.getErrorInfo(
+                  ex,
+                  info,
+                  log,
+                  localCore != null && localCore.getCoreContainer().hideStackTrace());
           sendError(code, info.toString());
         }
       } finally {
@@ -977,7 +982,12 @@ protected void writeResponse(
 
       if (solrRsp.getException() != null) {
         NamedList<Object> info = new SimpleOrderedMap<>();
-        int code = ResponseUtils.getErrorInfo(solrRsp.getException(), info, log);
+        int code =
+            ResponseUtils.getErrorInfo(
+                solrRsp.getException(),
+                info,
+                log,
+                core != null && core.getCoreContainer().hideStackTrace());
         solrRsp.add("error", info);
         response.setStatus(code);
       }

diff --git a/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java b/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
@@ -28,6 +28,10 @@
 public class ResponseUtils {
   private ResponseUtils() {}
 
+  // System property to use if the Solr core does not exist or solr.hideStackTrace is not
+  // configured. (i.e.: a lot of unit test).
+  private static final boolean SYSTEM_HIDE_STACK_TRACES = Boolean.getBoolean("solr.hideStackTrace");
+
   /**
    * Adds the given Throwable's message to the given NamedList.
    *
@@ -42,6 +46,27 @@ private ResponseUtils() {}
    * @see #getTypedErrorInfo(Throwable, Logger)
    */
   public static int getErrorInfo(Throwable ex, NamedList<Object> info, Logger log) {
+    return getErrorInfo(ex, info, log, false);
+  }
+
+  /**
+   * Adds the given Throwable's message to the given NamedList.
+   *
+   * <p>Primarily used by v1 code; v2 endpoints or dispatch code should call {@link
+   * #getTypedErrorInfo(Throwable, Logger)}
+   *
+   * <p>If the response code is not a regular code, the Throwable's stack trace is both logged and
+   * added to the given NamedList.
+   *
+   * <p>Status codes less than 100 are adjusted to be 500.
+   *
+   * <p>Stack trace will not be output if hideTrace=true OR system property
+   * solr.hideStackTrace=true.
+   *
+   * @see #getTypedErrorInfo(Throwable, Logger)
+   */
+  public static int getErrorInfo(
+      Throwable ex, NamedList<Object> info, Logger log, boolean hideTrace) {
     int code = 500;
     if (ex instanceof SolrException) {
       SolrException solrExc = (SolrException) ex;
@@ -70,10 +95,13 @@ public static int getErrorInfo(Throwable ex, NamedList<Object> info, Logger log)
 
     // For any regular code, don't include the stack trace
     if (code == 500 || code < 100) {
-      StringWriter sw = new StringWriter();
-      ex.printStackTrace(new PrintWriter(sw));
+      // hide all stack traces, as configured
+      if (!hideStackTrace(hideTrace)) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));
+        info.add("trace", sw.toString());
+      }
       log.error("500 Exception", ex);
-      info.add("trace", sw.toString());
 
       // non standard codes have undefined results with various servers
       if (code < 100) {
@@ -96,6 +124,22 @@ public static int getErrorInfo(Throwable ex, NamedList<Object> info, Logger log)
    * @see #getErrorInfo(Throwable, NamedList, Logger)
    */
   public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
+    return getTypedErrorInfo(ex, log, false);
+  }
+
+  /**
+   * Adds information about the given Throwable to a returned {@link ErrorInfo}
+   *
+   * <p>Primarily used by v2 API code, which can handle such typed information.
+   *
+   * <p>Status codes less than 100 are adjusted to be 500.
+   *
+   * <p>Stack trace will not be output if hideTrace=true OR system property
+   * solr.hideStackTrace=true.
+   *
+   * @see #getErrorInfo(Throwable, NamedList, Logger)
+   */
+  public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log, boolean hideTrace) {
     final ErrorInfo errorInfo = new ErrorInfo();
     int code = 500;
     if (ex instanceof SolrException) {
@@ -120,10 +164,12 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
 
     // For any regular code, don't include the stack trace
     if (code == 500 || code < 100) {
-      StringWriter sw = new StringWriter();
-      ex.printStackTrace(new PrintWriter(sw));
+      if (!hideStackTrace(hideTrace)) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));
+        errorInfo.trace = sw.toString();
+      }
       log.error("500 Exception", ex);
-      errorInfo.trace = sw.toString();
 
       // non standard codes have undefined results with various servers
       if (code < 100) {
@@ -135,4 +181,8 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(final boolean hideTrace) {
+    return hideTrace || SYSTEM_HIDE_STACK_TRACES;
+  }
 }
diff --git a/solr/core/src/test-files/solr/solr-50-all.xml b/solr/core/src/test-files/solr/solr-50-all.xml
@@ -29,6 +29,7 @@
   <int name="transientCacheSize">66</int>
   <int name="replayUpdatesThreads">100</int>
   <int name="maxBooleanClauses">42</int>
+  <bool name="hideStackTrace">true</bool>
 
   <coreAdminHandlerActions>
     <str name="action1">testCoreAdminHandlerAction1</str>

diff --git a/solr/core/src/test-files/solr/solr.xml b/solr/core/src/test-files/solr/solr.xml
@@ -28,6 +28,7 @@
   <str name="coreRootDirectory">${coreRootDirectory:.}</str>
   <str name="allowPaths">${solr.allowPaths:}</str>
   <str name="allowUrls">${solr.tests.allowUrls:}</str>
+  <bool name="hideStackTrace">${solr.hideStackTrace:true}</bool>
 
   <shardHandlerFactory name="shardHandlerFactory" class="HttpShardHandlerFactory">
     <str name="urlScheme">${urlScheme:}</str>

diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrXml.java b/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
@@ -136,6 +136,7 @@ public void testAllInfoPresent() throws IOException {
                     : Set.of("/tmp", "/home/john").stream()
                         .map(s -> Path.of(s))
                         .collect(Collectors.toSet())));
+    assertTrue("hideStackTrace", cfg.hideStackTraces());
     System.clearProperty("solr.allowPaths");
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -61,6 +61,7 @@ Improvements @@
     * SOLR-16970: SOLR_OPTS is now able to override options set by the Solr control scripts, "bin/solr" and "bin/solr.cmd". (Houston Putman)
+    * SOLR-14886: Suppress stack traces in query response (Isabelle Giguere via Alex Deparvu)
     Optimizations
     ---------------------
@@ Expand Down @@