You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "igiguere (via GitHub)" <gi...@apache.org> on 2023/05/08 19:15:01 UTC

[GitHub] [solr] igiguere opened a new pull request, #1632: SOLR_14886 : suppress stack traces in query response

igiguere opened a new pull request, #1632:
URL: https://github.com/apache/solr/pull/1632

   
   https://issues.apache.org/jira/browse/SOLR-14886
   
   
   # Description
   
   Stack traces are considered a security risk.  Please refer to OWASP for a full explanation:
   
   https://owasp.org/Top10/A05_2021-Security_Misconfiguration/
   
   
   # Solution
   
   Add a property "hideStackTrace" to solr.xml.  In NodeConfig, the default value is "false", for back-compatibility.
   
   Use the new property in ResponseUtils, to print out, or not, the stack trace.
   
   Adapt code that calls ResponseUtils.
   
   # Tests
   
   org.apache.solr.servlet.HideStackTraceTest : force an exception into the response, and assert that if hideStackTrace=true, the stack trace is not shown.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [* ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [* ] I have created a Jira issue and added the issue ID to my pull request title.
   - [* ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [* ] I have developed this patch against the `main` branch.
   - [* ] I have run `./gradlew check`.
   - [* ] I have added tests for my changes.
   - [* ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1190310363


##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -971,7 +975,11 @@ protected void writeResponse(
 
       if (solrRsp.getException() != null) {
         NamedList<Object> info = new SimpleOrderedMap<>();
-        int code = ResponseUtils.getErrorInfo(solrRsp.getException(), info, log);
+        boolean hideStackTrace =
+            core != null

Review Comment:
   HttpSolrCall.init() is called during initialization.  It will not race with HttpSolrCall.writeResponse(...).  There is no response to be written before or during initialization.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1322296243


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.util.EntityUtils;

Review Comment:
   As a project, we'd like to get away from needless use of Apache HttpClient.  We can use JDK HttpClient here.



##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -301,6 +301,8 @@ && getZkController().getOverseer() != null
 
   private final AllowListUrlChecker allowListUrlChecker;
 
+  private final boolean hideStackTrace;

Review Comment:
   I don't think a simple global boolean needs to be yet another field on CoreContainer.  Even the NodeConfig stuff is overkill for what could be a system property.
   
   If we do keep NodeConfig, we could still do away with this field because the NodeConfig stays around as a field on CoreContainer as `cfg`.



##########
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");

Review Comment:
   This is all we need for internal config IMO.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1714888248

   It's sad to see so much ceremony in NodeConfig :-( -- an issue for another PR.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1632: SOLR_14886 : suppress stack traces in query response

Posted by "sonatype-lift[bot] (via GitHub)" <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1187848947


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -70,10 +90,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));
-      log.error("500 Exception", ex);
-      info.add("trace", sw.toString());
+      // hide all stack traces, as configured
+      if (!hideStackTrace) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));

Review Comment:
   <picture><img alt="2% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/2/display.svg"></picture>
   
   <b>*[INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE](https://find-sec-bugs.github.io/bugs.htm#INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE):</b>*  Possible information exposure through an error message
   
   ---
   
   <details><summary>ℹ️ Expand to see all <b>@sonatype-lift</b> commands</summary>
   
   You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
   
   



##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -971,7 +975,11 @@ protected void writeResponse(
 
       if (solrRsp.getException() != null) {
         NamedList<Object> info = new SimpleOrderedMap<>();
-        int code = ResponseUtils.getErrorInfo(solrRsp.getException(), info, log);
+        boolean hideStackTrace =
+            core != null

Review Comment:
   <picture><img alt="7% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/7/display.svg"></picture>
   
   <b>*THREAD_SAFETY_VIOLATION:</b>*  Read/Write race. Non-private method `HttpSolrCall.writeResponse(...)` reads without synchronization from `this.core`. Potentially races with write in method `HttpSolrCall.init()`.
    Reporting because the current class is annotated `@ThreadSafe`, so we assume that this method can run in parallel with other non-private methods in the class (including itself).
   
   ---
   
   <details><summary>ℹ️ Expand to see all <b>@sonatype-lift</b> commands</summary>
   
   You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
   
   



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303381080


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathFactory;
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.config.RequestConfig.Builder;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.SearchComponent;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+
+/** SOLR-14886 : Suppress stack trace in Query response */
+@SuppressSSL
+public class HideStackTraceTest extends SolrJettyTestBase {

Review Comment:
   I'm having too much trouble with that unit test (on Windows) if SSL is enabled.  And SSL is not needed to demonstrate the functionality.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1714298498

   @dsmiley I am attempting to rewrite the test using thew new `EmbeddedSolrServerTestRule` but I am falling short.
   First it seems to shortcut exceptions in the response via `checkForExceptions(rsp);` method so there is no opportunity for the stacktrace to be populated in the response, what I see now is just the exception being thrown so cannot verify the `hideStackTrace` flag does anything.
   Second it seems to hardcode the response to the binary response (see `BinaryResponseWriter.getParsedResponse`), so not sure if this might need to change to support other types, for this test it is needed to serialize the error to verify the trace is not there.
   please correct if I am missing something obvious here.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1192139537


##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -296,6 +296,8 @@ && getZkController().getOverseer() != null
 
   private final Set<Path> allowPaths;
 
+  private final boolean hideStackTrace;
+
   private final AllowListUrlChecker allowListUrlChecker;

Review Comment:
   minor: consider locating `hideStackTrace` after the two `allow...` to match protected constructor init order
   ```suggestion
     private final AllowListUrlChecker allowListUrlChecker;
   
     private final boolean hideStackTrace;
   ```



##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -70,10 +90,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));
-      log.error("500 Exception", ex);
-      info.add("trace", sw.toString());
+      // hide all stack traces, as configured
+      if (!hideStackTrace) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));
+        log.error("500 Exception", ex);
+        info.add("trace", sw.toString());

Review Comment:
   Wondering if only the `info.add` here could be guarded by the `!hideStackTrace` hide condition i.e. it's logged but not returned in the response?



##########
solr/core/src/java/org/apache/solr/rest/BaseSolrResource.java:
##########
@@ -143,7 +143,11 @@ protected void handleException(Logger log) {
     Exception exception = getSolrResponse().getException();
     if (null != exception) {
       NamedList<Object> info = new SimpleOrderedMap<>();
-      this.statusCode = ResponseUtils.getErrorInfo(exception, info, log);
+      boolean hideStackTrace =
+          solrCore != null
+              ? solrCore.getCoreContainer().hideStackTrace()
+              : Boolean.parseBoolean(System.getProperty("solr.hideStackTrace"));

Review Comment:
   subjective: with multiple places have this logic, perhaps there could be a common utility method somewhere
   ```suggestion
         boolean hideStackTrace = SomeWhereUtils.getHideStrackTrace(solrCore);
   ```



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1298864143


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -96,6 +120,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, null);
+  }
+
+  /**
+   * 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, SolrCore core) {

Review Comment:
   Done in latest commit. Thanks.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1716390290

   having to rebase + force commit to get crave build unstuck is becoming a real pain


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "sonatype-lift[bot] (via GitHub)" <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1192589333


##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -971,7 +971,7 @@ 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);

Review Comment:
   <picture><img alt="7% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/7/display.svg"></picture>
   
   <b>*THREAD_SAFETY_VIOLATION:</b>*  Read/Write race. Non-private method `HttpSolrCall.writeResponse(...)` reads without synchronization from `this.core`. Potentially races with write in method `HttpSolrCall.init()`.
    Reporting because the current class is annotated `@ThreadSafe`, so we assume that this method can run in parallel with other non-private methods in the class (including itself).
   
   ---
   
   <details><summary>ℹ️ Expand to see all <b>@sonatype-lift</b> commands</summary>
   
   You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
   | **Command** | **Usage** |
   | ------------- | ------------- |
   | `@sonatype-lift ignore` | Leave out the above finding from this PR |
   | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
   | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
   
   **Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
   <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
   
   



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303280414


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -28,6 +28,11 @@
 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 =

Review Comment:
   this can be `Boolean.getBoolean("solr.hideStackTrace")`



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1714322303

   Thanks for trying Alex!
   I suppose this test is more sensitive to the serialization, in which case you should probably use SolrJettyTestRule.
   
   EmbeddedSolrServer:  Ideally, a test (or more likely the test infrastructure behind the scenes) could pick the response format if it wants, say, XML or JSON instead of the parsed NamedList, such as if assertJQ (JSON path custom thing) or assertQ (XPath).  The InputStreamResponseParser may be needed, which is a bit of a special thing (see Find-Usages to see for yourself).


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1192534779


##########
solr/core/src/java/org/apache/solr/rest/BaseSolrResource.java:
##########
@@ -143,7 +143,11 @@ protected void handleException(Logger log) {
     Exception exception = getSolrResponse().getException();
     if (null != exception) {
       NamedList<Object> info = new SimpleOrderedMap<>();
-      this.statusCode = ResponseUtils.getErrorInfo(exception, info, log);
+      boolean hideStackTrace =
+          solrCore != null
+              ? solrCore.getCoreContainer().hideStackTrace()
+              : Boolean.parseBoolean(System.getProperty("solr.hideStackTrace"));

Review Comment:
   addressed in commit cd8459605081f993ba6e16e5d0f631b3b4de60be



##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -70,10 +90,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));
-      log.error("500 Exception", ex);
-      info.add("trace", sw.toString());
+      // hide all stack traces, as configured
+      if (!hideStackTrace) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));
+        log.error("500 Exception", ex);
+        info.add("trace", sw.toString());

Review Comment:
   addressed in commit cd8459605081f993ba6e16e5d0f631b3b4de60be



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1693878618

   I'm not seeing a new build, I think you have to force push the rebase so the build can apply correctly


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1693777248

   @stillalex  :  I'm in CHANGES.txt a couple of times with my full name 
   Isabelle Giguere


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1297843290


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -135,4 +177,10 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(SolrCore core) {
+    return core != null
+        ? core.getCoreContainer().hideStackTrace()
+        : Boolean.parseBoolean(System.getProperty("solr.hideStackTrace"));

Review Comment:
   @stillalex  : Thanks for your comments.  For some reason, the unit test in this PR is now failing, before I even try to modify anything.  Is anyone aware of changes in the code path of a response that would prevent going though these methods of ResponseUtils?  I'll keep trying, but, no luck after a couple of hours so far.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex merged pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex merged PR #1632:
URL: https://github.com/apache/solr/pull/1632


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1304351888


##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -567,6 +575,7 @@ public static class NodeConfigBuilder {
     private String defaultZkHost;
     private Set<Path> allowPaths = Collections.emptySet();
     private List<String> allowUrls = Collections.emptyList();
+    private boolean hideStackTrace = false;

Review Comment:
   Wow!  Thanks.  I don't know what I was thinking ;)



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303281456


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -135,4 +182,8 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(final Boolean hideTrace) {

Review Comment:
   `boolean hideTrace` instead of Boolean



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303627276


##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -567,6 +575,7 @@ public static class NodeConfigBuilder {
     private String defaultZkHost;
     private Set<Path> allowPaths = Collections.emptySet();
     private List<String> allowUrls = Collections.emptyList();
+    private boolean hideStackTrace = false;

Review Comment:
   hmm, this doesn't look right :) it will now ignore the builder value. line 616 needs to be 
   
   ```
   private boolean hideStackTrace = Boolean.getBoolean("solr.hideStackTrace");
   ```
   
   and line 190 needs to be as before
   ```
   this.hideStackTraces
   ```
   



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1192621167


##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -971,7 +971,7 @@ 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);

Review Comment:
   @sonatype-lift ignore
   
   HttpSolrCall.init() is called during initialization. It will not race with HttpSolrCall.writeResponse(...). There is no response to be written before or during initialization.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1192381371


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -70,10 +90,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));
-      log.error("500 Exception", ex);
-      info.add("trace", sw.toString());
+      // hide all stack traces, as configured
+      if (!hideStackTrace) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));
+        log.error("500 Exception", ex);
+        info.add("trace", sw.toString());

Review Comment:
   Good catch.  Thanks, I'll do that



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303292779


##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -567,6 +575,7 @@ public static class NodeConfigBuilder {
     private String defaultZkHost;
     private Set<Path> allowPaths = Collections.emptySet();
     private List<String> allowUrls = Collections.emptyList();
+    private boolean hideStackTrace = false;

Review Comment:
   the default should be the system property value I think. this way it's consistent with the ResponseUtils implementation. (if you only set the system property, both code paths should behave the same)



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303284476


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathFactory;
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.config.RequestConfig.Builder;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.SearchComponent;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+
+/** SOLR-14886 : Suppress stack trace in Query response */
+@SuppressSSL
+public class HideStackTraceTest extends SolrJettyTestBase {

Review Comment:
   I don't think you need to write as much code to boot a Solr. take a look at https://github.com/apache/solr/blob/473cea28b55dcf03fb2338399af321f425c90002/solr/core/src/test/org/apache/solr/handler/TestRequestId.java#L36 for example (there are many examples around)
   
   also `@SuppressSSL` is not needed here



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1714390934

   @dsmiley I refactored the test as best I could. please take a look and let me know if more cleanup is needed.
   I had to rebase+force push to get crave build running again


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1298862178


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -42,6 +43,26 @@ private ResponseUtils() {}
    * @see #getTypedErrorInfo(Throwable, Logger)
    */
   public static int getErrorInfo(Throwable ex, NamedList<Object> info, Logger log) {
+    return getErrorInfo(ex, info, log, null);
+  }
+
+  /**
+   * 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, SolrCore core) {

Review Comment:
   Done in latest commit.  Thanks.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1298863878


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -135,4 +177,10 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(SolrCore core) {
+    return core != null
+        ? core.getCoreContainer().hideStackTrace()
+        : Boolean.parseBoolean(System.getProperty("solr.hideStackTrace"));

Review Comment:
   Something similar was done in latest commit.  Thanks.



##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -135,4 +177,10 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(SolrCore core) {
+    return core != null
+        ? core.getCoreContainer().hideStackTrace()
+        : Boolean.parseBoolean(System.getProperty("solr.hideStackTrace"));

Review Comment:
   @stillalex  : Thanks for your comments.  For some reason, the unit test in this PR is now failing, before I even try to modify anything.  Is anyone aware of changes in the code path of a response that would prevent going though these methods of ResponseUtils?  I'll keep trying, but, no luck after a couple of hours so far.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303382150


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathFactory;
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.config.RequestConfig.Builder;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.SearchComponent;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+
+/** SOLR-14886 : Suppress stack trace in Query response */
+@SuppressSSL
+public class HideStackTraceTest extends SolrJettyTestBase {

Review Comment:
   Thanks for the example.  I guess I had never noticed such a simple one.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303381080


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathFactory;
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.config.RequestConfig.Builder;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.SearchComponent;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+
+/** SOLR-14886 : Suppress stack trace in Query response */
+@SuppressSSL
+public class HideStackTraceTest extends SolrJettyTestBase {

Review Comment:
   I'm having too much trouble with that unit test (on Windows + OpenJDK 11.0.20) if SSL is enabled.  And SSL is not needed to demonstrate the functionality.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303627276


##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -567,6 +575,7 @@ public static class NodeConfigBuilder {
     private String defaultZkHost;
     private Set<Path> allowPaths = Collections.emptySet();
     private List<String> allowUrls = Collections.emptyList();
+    private boolean hideStackTrace = false;

Review Comment:
   hmm, this doesn't look right :) it will now ignore the builder value. line 616 needs to be 
   
   ```
   private boolean hideStackTrace = Boolean.getBoolean("solr.hideStackTrace");
   ```
   
   and line 190 needs to be as before
   ```
   this.hideStackTraces = hideStackTraces
   ```
   



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1306186982


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathFactory;
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.config.RequestConfig.Builder;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.SearchComponent;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+
+/** SOLR-14886 : Suppress stack trace in Query response */
+@SuppressSSL
+public class HideStackTraceTest extends SolrJettyTestBase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  private static Path solrHome;
+
+  @BeforeClass
+  public static void before() throws Exception {
+    initCore("solrconfig-errorComponent.xml", "schema.xml");
+    createAndStartJetty(SolrTestCaseJ4.TEST_PATH().toAbsolutePath().toString());
+  }
+
+  @AfterClass
+  public static void after() throws Exception {
+    afterSolrJettyTestBase();
+    if (solrHome != null) {
+      cleanUpJettyHome(solrHome.toFile());
+    }
+  }
+
+  @Test
+  public void testHideStackTrace() throws Exception {
+    final String url = jetty.getBaseUrl().toString() + "/collection1/withError?q=*:*&wt=xml";
+    final HttpGet get = new HttpGet(url);

Review Comment:
   Does this test need to work with HTTP at all?  I believe not; the stack trace should be in the response structure.  See EmbeddedSolrServerTestRule -- a new test mechanism for a new breed of tests that can work with a SolrClient.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1190308248


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -70,10 +90,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));
-      log.error("500 Exception", ex);
-      info.add("trace", sw.toString());
+      // hide all stack traces, as configured
+      if (!hideStackTrace) {
+        StringWriter sw = new StringWriter();
+        ex.printStackTrace(new PrintWriter(sw));

Review Comment:
   This PR aims at mitigating "information exposure through an error message".  With this PR, the stack trace will only be output if Solr is configured to output stack traces.
   The PR does not, however, attempt to fix pre-existing code, such as this call to "printStackTrace"



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303615766


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -135,4 +182,8 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(final Boolean hideTrace) {

Review Comment:
   It took a bit of refactoring elsewhere, but yep, done in latest commit.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303615333


##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -567,6 +575,7 @@ public static class NodeConfigBuilder {
     private String defaultZkHost;
     private Set<Path> allowPaths = Collections.emptySet();
     private List<String> allowUrls = Collections.emptyList();
+    private boolean hideStackTrace = false;

Review Comment:
   done in latest commit



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1296299519


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -96,6 +120,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, null);
+  }
+
+  /**
+   * 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, SolrCore core) {

Review Comment:
   same as above. pass the boolean only



##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -42,6 +43,26 @@ private ResponseUtils() {}
    * @see #getTypedErrorInfo(Throwable, Logger)
    */
   public static int getErrorInfo(Throwable ex, NamedList<Object> info, Logger log) {
+    return getErrorInfo(ex, info, log, null);
+  }
+
+  /**
+   * 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, SolrCore core) {

Review Comment:
   could you replace 'core' instance with just the hideStackTrace flag? there is no reason to pass in the entire core.



##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -135,4 +177,10 @@ public static ErrorInfo getTypedErrorInfo(Throwable ex, Logger log) {
     errorInfo.code = code;
     return errorInfo;
   }
+
+  private static boolean hideStackTrace(SolrCore core) {
+    return core != null
+        ? core.getCoreContainer().hideStackTrace()
+        : Boolean.parseBoolean(System.getProperty("solr.hideStackTrace"));

Review Comment:
   no need to check the system property value every time you need to log an error. you can move this to the init phase, in the NodeConfig maybe? and pass that value to the log method.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] stillalex commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1693721799

   @igiguere could you rebase on top of main branch (as suggested on the dev list)?
   also what name should I use in the CHANGES.txt entry for the credit?
   


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on PR #1632:
URL: https://github.com/apache/solr/pull/1632#issuecomment-1693865778

   I rebased this branch on the fork's main (up to date with /solr main).  There were no changes to push, so I don't know if a build will be triggered.
   Hopefully, it will work.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303615200


##########
solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathFactory;
+import org.apache.http.client.config.CookieSpecs;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.config.RequestConfig.Builder;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.SearchComponent;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+
+/** SOLR-14886 : Suppress stack trace in Query response */
+@SuppressSSL
+public class HideStackTraceTest extends SolrJettyTestBase {

Review Comment:
   test initialization simplified in the latest commit.  But keeping @SuppresSSL: refer to my [previous comment|https://github.com/apache/solr/pull/1632#discussion_r1303381080].



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] igiguere commented on a diff in pull request #1632: SOLR-14886 : suppress stack traces in query response

Posted by "igiguere (via GitHub)" <gi...@apache.org>.
igiguere commented on code in PR #1632:
URL: https://github.com/apache/solr/pull/1632#discussion_r1303614343


##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -28,6 +28,11 @@
 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 =

Review Comment:
   done in latest commit



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org