[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1263: diff with 9_1

Mon, 02 Jan 2023 15:00:52 -0800 


sonatype-lift[bot] commented on code in PR #1263:
URL: https://github.com/apache/solr/pull/1263#discussion_r1060206625


##########
solr/core/src/java/org/apache/solr/security/MultiAuthPlugin.java:
##########
@@ -185,7 +210,8 @@ public boolean doAuthenticate(HttpServletRequest request, 
HttpServletResponse re
     final String scheme = getSchemeFromAuthHeader(authHeader);
     final AuthenticationPlugin plugin = pluginMap.get(scheme);
     if (plugin == null) {
-      response.sendError(ErrorCode.UNAUTHORIZED.code, "Authorization scheme '" 
+ scheme + "' not supported!");
+      response.sendError(

Review Comment:
   đŸ’Ŧ 4 similar findings have been found in this PR
   
   ---
   
   *[XSS_SERVLET](https://find-sec-bugs.github.io/bugs.htm#XSS_SERVLET):*  This 
use of javax/servlet/http/HttpServletResponse.sendError(ILjava/lang/String;)V 
could be vulnerable to XSS in the Servlet
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this 
finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java | 
[204](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java#L204)
 |
   | 
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java 
| 
[790](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java#L790)
 |
   | solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java | 
[854](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L854)
 |
   | solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java 
| 
[890](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java#L890)
 |
   <p><a 
href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|XSS_SERVLET"
 target="_blank">Visit the Lift Web Console</a> to find more details in your 
report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></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>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=365234167&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234167&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234167&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234167&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=365234167&lift_comment_rating=5)
 ]



##########
solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java:
##########
@@ -123,9 +117,16 @@ public void addRepository(String repoName, String uri) 
throws Exception {
     List<PackageRepository> repos = 
getMapper().readValue(existingRepositoriesJson, List.class);
     repos.add(new DefaultPackageRepository(repoName, uri));
     if (packageManager.zkClient.exists(PackageUtils.REPOSITORIES_ZK_PATH, 
true) == false) {
-      packageManager.zkClient.create(PackageUtils.REPOSITORIES_ZK_PATH, 
getMapper().writeValueAsString(repos).getBytes("UTF-8"), CreateMode.PERSISTENT, 
true);
+      packageManager.zkClient.create(
+          PackageUtils.REPOSITORIES_ZK_PATH,
+          getMapper().writeValueAsString(repos).getBytes("UTF-8"),
+          CreateMode.PERSISTENT,
+          true);
     } else {
-      packageManager.zkClient.setData(PackageUtils.REPOSITORIES_ZK_PATH, 
getMapper().writeValueAsString(repos).getBytes("UTF-8"), true);
+      packageManager.zkClient.setData(
+          PackageUtils.REPOSITORIES_ZK_PATH,
+          getMapper().writeValueAsString(repos).getBytes("UTF-8"),
+          true);
     }
 
     addKey(IOUtils.toByteArray(new URL(uri + "/publickey.der").openStream()), 
repoName + ".der");

Review Comment:
   <picture><img alt="22% of developers fix this issue" 
src="https://lift.sonatype.com/api/commentimage/fixrate/22/display.svg";></picture>
   
   đŸ’Ŧ 8 similar findings have been found in this PR
   
   ---
   
   
*[URLCONNECTION_SSRF_FD](https://find-sec-bugs.github.io/bugs.htm#URLCONNECTION_SSRF_FD):*
  This web server request could be used by an attacker to expose internal 
services and filesystem.
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this 
finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | 
solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java | 
[41](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java#L41)
 |
   | solr/core/src/java/org/apache/solr/util/SimplePostTool.java | 
[999](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L999)
 |
   | 
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java
 | 
[465](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java#L465)
 |
   | 
solr/test-framework/src/java/org/apache/solr/handler/BackupRestoreUtils.java | 
[116](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/handler/BackupRestoreUtils.java#L116)
 |
   | solr/core/src/java/org/apache/solr/util/SimplePostTool.java | 
[1235](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L1235)
 |
   | solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java | 
[150](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java#L150)
 |
   | solr/core/src/java/org/apache/solr/util/CryptoKeys.java | 
[271](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/CryptoKeys.java#L271)
 |
   | solr/core/src/java/org/apache/solr/util/SimplePostTool.java | 
[975](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L975)
 |
   <p><a 
href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|URLCONNECTION_SSRF_FD"
 target="_blank">Visit the Lift Web Console</a> to find more details in your 
report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></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>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=365234179&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234179&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234179&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234179&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=365234179&lift_comment_rating=5)
 ]



##########
solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java:
##########
@@ -116,14 +122,17 @@ public void load(SolrQueryRequest req, SolrQueryResponse 
rsp, ContentStream stre
         // TODO: The charset may be wrong, as the real charset is later
         // determined by the XML parser, the content-type is only used as a 
hint!
         if (log.isTraceEnabled()) {
-          log.trace("body: {}", new String(body, (charset == null) ?
-              ContentStreamBase.DEFAULT_CHARSET : charset));
+          log.trace(
+              "body: {}",
+              new String(body, (charset == null) ? 
ContentStreamBase.DEFAULT_CHARSET : charset));
         }
         IOUtils.closeQuietly(is);
         is = new ByteArrayInputStream(body);
       }
-      parser = (charset == null) ?
-        inputFactory.createXMLStreamReader(is) : 
inputFactory.createXMLStreamReader(is, charset);
+      parser =
+          (charset == null)
+              ? inputFactory.createXMLStreamReader(is)

Review Comment:
   đŸ’Ŧ 7 similar findings have been found in this PR
   
   ---
   
   
*[XXE_XMLSTREAMREADER](https://find-sec-bugs.github.io/bugs.htm#XXE_XMLSTREAMREADER):*
  The XML parsing is vulnerable to XML External Entity attacks
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this 
finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | 
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLResponseParser.java | 
[92](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLResponseParser.java#L92)
 |
   | 
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLResponseParser.java | 
[104](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLResponseParser.java#L104)
 |
   | 
solr/modules/scripting/src/java/org/apache/solr/scripting/xslt/XSLTUpdateRequestHandler.java
 | 
[125](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/scripting/src/java/org/apache/solr/scripting/xslt/XSLTUpdateRequestHandler.java#L125)
 |
   | 
solr/core/src/java/org/apache/solr/handler/DocumentAnalysisRequestHandler.java 
| 
[156](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/DocumentAnalysisRequestHandler.java#L156)
 |
   | 
solr/core/src/java/org/apache/solr/handler/DocumentAnalysisRequestHandler.java 
| 
[157](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/DocumentAnalysisRequestHandler.java#L157)
 |
   | solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java | 
[135](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java#L135)
 |
   | 
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLResponseParser.java | 
[578](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLResponseParser.java#L578)
 |
   <p><a 
href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|XXE_XMLSTREAMREADER"
 target="_blank">Visit the Lift Web Console</a> to find more details in your 
report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></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>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=365234189&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234189&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234189&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234189&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=365234189&lift_comment_rating=5)
 ]



##########
solr/core/src/java/org/apache/solr/util/ExportTool.java:
##########
@@ -257,12 +251,12 @@ public JsonSink(Info info) {
     @Override
     public void start() throws IOException {
       fos = new FileOutputStream(info.out);

Review Comment:
   đŸ’Ŧ 3 similar findings have been found in this PR
   
   ---
   
   
*[PATH_TRAVERSAL_OUT](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_OUT):*
  This API (java/io/FileOutputStream.<init>(Ljava/lang/String;)V) writes to a 
file whose location might be specified by user input
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this 
finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java | 
[67](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java#L67)
 |
   | solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java | 
[193](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java#L193)
 |
   | solr/core/src/java/org/apache/solr/util/ExportTool.java | 
[327](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/ExportTool.java#L327)
 |
   <p><a 
href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|PATH_TRAVERSAL_OUT"
 target="_blank">Visit the Lift Web Console</a> to find more details in your 
report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></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>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=365234217&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234217&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234217&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234217&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=365234217&lift_comment_rating=5)
 ]



##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -113,81 +149,159 @@ public boolean doAuthenticate(HttpServletRequest 
request, HttpServletResponse re
       return true;
     }
 
-    String header = request.getHeader(HEADER);
-    assert header != null : "Should have been checked by 
SolrDispatchFilter.authenticateRequest";
+    PKIHeaderData headerData = null;
+    String headerV2 = request.getHeader(HEADER_V2);
+    String headerV1 = request.getHeader(HEADER);
+    if (headerV2 != null) {
+      // Try V2 first
+      int nodeNameEnd = headerV2.indexOf(' ');
+      if (nodeNameEnd <= 0) {
+        // Do not log the value as it is likely gibberish
+        return sendError(response, true, "Could not parse node name from 
SolrAuthV2 header.");
+      }
 
-    List<String> authInfo = StrUtils.splitWS(header, false);
-    if (authInfo.size() != 2) {
-      numErrors.mark();
-      log.error("Invalid SolrAuth header: {}", header);
-      response.setHeader(HttpHeaders.WWW_AUTHENTICATE, HEADER);
-      response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid 
SolrAuth header");
-      return false;
+      headerData = decipherHeaderV2(headerV2, headerV2.substring(0, 
nodeNameEnd));
+    } else if (headerV1 != null && acceptPkiV1) {
+      List<String> authInfo = StrUtils.splitWS(headerV1, false);
+      if (authInfo.size() != 2) {
+        // We really shouldn't be logging and returning this, but we did it 
before so keep that
+        return sendError(response, false, "Invalid SolrAuth header: " + 
headerV1);
+      }
+      headerData = decipherHeader(authInfo.get(0), authInfo.get(1));
     }
 
-    String nodeName = authInfo.get(0);
-    String cipher = authInfo.get(1);
-
-    PKIHeaderData decipher = decipherHeader(nodeName, cipher);
-    if (decipher == null) {
-      numMissingCredentials.inc();
-      log.error("Could not load principal from SolrAuth header.");
-      response.setHeader(HttpHeaders.WWW_AUTHENTICATE, HEADER);
-      response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Could not load 
principal from SolrAuth header.");
-      return false;
+    if (headerData == null) {
+      return sendError(response, true, "Could not load principal from 
SolrAuthV2 header.");
     }
-    long elapsed = receivedTime - decipher.timestamp;
+    long elapsed = receivedTime - headerData.timestamp;
     if (elapsed > MAX_VALIDITY) {
-      numErrors.mark();
-      log.error("Expired key request timestamp, elapsed={}, TTL={}", elapsed, 
MAX_VALIDITY);
-      response.setHeader(HttpHeaders.WWW_AUTHENTICATE, HEADER);
-      response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Expired key 
request timestamp");
-      return false;
+      return sendError(response, true, "Expired key request timestamp, 
elapsed=" + elapsed);
     }
 
-    final Principal principal = "$".equals(decipher.userName) ?
-        SU :
-        new BasicUserPrincipal(decipher.userName);
+    final Principal principal =
+        "$".equals(headerData.userName) ? SU : new 
BasicUserPrincipal(headerData.userName);
 
     numAuthenticated.inc();
     filterChain.doFilter(wrapWithPrincipal(request, principal), response);
     return true;
   }
 
+  /**
+   * Set the response header errors, possibly log something and return false 
for failed
+   * authentication
+   *
+   * @param response the response to set error status with
+   * @param v2 whether this authentication used the v1 or v2 header (true if 
v2)
+   * @param message the message to log and send back to client. do not include 
anyhting sensitive
+   *     here about server state
+   * @return false to chain with calls from authenticate
+   */
+  private boolean sendError(HttpServletResponse response, boolean v2, String 
message)
+      throws IOException {
+    numErrors.mark();
+    log.error(message);
+    response.setHeader(HttpHeaders.WWW_AUTHENTICATE, v2 ? HEADER_V2 : HEADER);
+    response.sendError(HttpServletResponse.SC_UNAUTHORIZED, message);
+    return false;
+  }
+
   public static class PKIHeaderData {
     String userName;
     long timestamp;
+
+    @Override
+    public String toString() {
+      return "PKIHeaderData{" + "userName='" + userName + '\'' + ", 
timestamp=" + timestamp + '}';
+    }
+  }
+
+  private PKIHeaderData decipherHeaderV2(String header, String nodeName) {
+    PublicKey key = keyCache.get(nodeName);
+    if (key == null) {
+      log.debug("No key available for node: {} fetching now ", nodeName);

Review Comment:
   <picture><img alt="17% of developers fix this issue" 
src="https://lift.sonatype.com/api/commentimage/fixrate/17/display.svg";></picture>
   
   đŸ’Ŧ 9 similar findings have been found in this PR
   
   ---
   
   
*[CRLF_INJECTION_LOGS](https://find-sec-bugs.github.io/bugs.htm#CRLF_INJECTION_LOGS):*
  This use of org/slf4j/Logger.debug(Ljava/lang/String;Ljava/lang/Object;)V 
might be used to include CRLF characters into log messages
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this 
finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | 
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java
 | 
[237](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java#L237)
 |
   | 
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java
 | 
[227](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java#L227)
 |
   | 
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java
 | 
[228](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java#L228)
 |
   | solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java | 
[334](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java#L334)
 |
   | 
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/KerberosFilter.java
 | 
[98](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/KerberosFilter.java#L98)
 |
   | 
solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java | 
[148](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java#L148)
 |
   | solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java | 
[102](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java#L102)
 |
   | 
solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java | 
[146](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java#L146)
 |
   | solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java | 
[249](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java#L249)
 |
   <p><a 
href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|CRLF_INJECTION_LOGS"
 target="_blank">Visit the Lift Web Console</a> to find more details in your 
report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></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>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=365234220&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234220&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234220&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234220&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=365234220&lift_comment_rating=5)
 ]



##########
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##########
@@ -1229,43 +1282,56 @@ protected void returnFields(ResponseBuilder rb, 
ShardRequest sreq) {
       for (ShardResponse srsp : sreq.responses) {
         if (srsp.getException() != null) {
           // Don't try to get the documents if there was an exception in the 
shard
-          if(rb.req.getParams().getBool(ShardParams.SHARDS_INFO, false)) {
+          if (rb.req.getParams().getBool(ShardParams.SHARDS_INFO, false)) {
             @SuppressWarnings("unchecked")
-            NamedList<Object> shardInfo = (NamedList<Object>) 
rb.rsp.getValues().get(ShardParams.SHARDS_INFO);
+            NamedList<Object> shardInfo =
+                (NamedList<Object>) 
rb.rsp.getValues().get(ShardParams.SHARDS_INFO);
             @SuppressWarnings("unchecked")
             SimpleOrderedMap<Object> nl = (SimpleOrderedMap<Object>) 
shardInfo.get(srsp.getShard());
             if (nl.get("error") == null) {
               // Add the error to the shards info section if it wasn't added 
before
               Throwable t = srsp.getException();
-              if(t instanceof SolrServerException) {
-                t = ((SolrServerException)t).getCause();
+              if (t instanceof SolrServerException) {
+                t = ((SolrServerException) t).getCause();
               }
-              nl.add("error", t.toString() );
+              nl.add("error", t.toString());
               StringWriter trace = new StringWriter();
               t.printStackTrace(new PrintWriter(trace));

Review Comment:
   đŸ’Ŧ 8 similar findings have been found in this PR
   
   ---
   
   
*[INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE](https://find-sec-bugs.github.io/bugs.htm#INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE):*
  Possible information exposure through an error message
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this 
finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java | 
[910](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java#L910)
 |
   | solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java | 
[43](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java#L43)
 |
   | solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java | 
[589](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java#L589)
 |
   | solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/Tuple.java | 
[381](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/Tuple.java#L381)
 |
   | solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java | 
[52](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java#L52)
 |
   | 
solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java
 | 
[93](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java#L93)
 |
   | solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java | 
[74](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java#L74)
 |
   | solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java | 
[124](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java#L124)
 |
   <p><a 
href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE"
 target="_blank">Visit the Lift Web Console</a> to find more details in your 
report.</p></div></details>
   
   
   
   ---
   
   <details><summary><b>ℹī¸ Learn about @sonatype-lift commands</b></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>
   
   
   
   ---
   
   Was this a good recommendation?
   [ [🙁 Not 
relevant](https://www.sonatype.com/lift-comment-rating?comment=365234223&lift_comment_rating=1)
 ] - [ [😕 Won't 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234223&lift_comment_rating=2)
 ] - [ [😑 Not critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234223&lift_comment_rating=3)
 ] - [ [🙂 Critical, will 
fix](https://www.sonatype.com/lift-comment-rating?comment=365234223&lift_comment_rating=4)
 ] - [ [😊 Critical, fixing 
now](https://www.sonatype.com/lift-comment-rating?comment=365234223&lift_comment_rating=5)
 ]



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to