You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2023/01/02 22:01:35 UTC

[GitHub] [solr] vatsalpatel3689 opened a new pull request, #1263: diff with 9_1

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

   https://issues.apache.org/jira/browse/SOLR-XXXXX
   
   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Solr:
   
   * https://issues.apache.org/jira/projects/SOLR
   
   For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA. 
   To create a Jira issue, you will need to create an account there first.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-####: <short description of problem or changes>
   
   SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # Description
   
   Please provide a short description of the changes you're making with this pull request.
   
   # Solution
   
   Please provide a short description of the approach taken to implement your solution.
   
   # Tests
   
   Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
   
   # 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] sonatype-lift[bot] commented on a diff in pull request #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
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: 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 #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1263:
URL: https://github.com/apache/solr/pull/1263#discussion_r1060206666


##########
solr/core/src/java/org/apache/solr/update/UpdateLog.java:
##########
@@ -2222,7 +2357,9 @@ protected Long seedBucketsWithHighestVersion(SolrIndexSearcher newSearcher, Vers

Review Comment:
   <picture><img alt="19% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/19/display.svg"></picture>
   
   đŸ’Ŧ 396 similar findings have been found in this PR
   
   ---
   
   *THREAD_SAFETY_VIOLATION:*  Unprotected write. Non-private method `UpdateLog.seedBucketsWithHighestVersion(...)` indirectly mutates container `util.ObjectReleaseTracker.OBJECTS` via call to `Map.remove(...)` outside of synchronization.
    Reporting because another access to the same memory occurs on a background thread, although this access may not.
   
   ---
   
   <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/logging/CircularList.java | [73](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/logging/CircularList.java#L73) |
   | solr/modules/hdfs/src/java/org/apache/solr/hdfs/update/HdfsUpdateLog.java | [142](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hdfs/src/java/org/apache/solr/hdfs/update/HdfsUpdateLog.java#L142) |
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [319](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L319) |
   | solr/core/src/java/org/apache/solr/uninverting/FieldCacheImpl.java | [640](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/uninverting/FieldCacheImpl.java#L640) |
   | solr/core/src/java/org/apache/solr/update/UpdateLog.java | [2373](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L2373) |
   | solr/modules/hdfs/src/java/org/apache/solr/hdfs/update/HdfsUpdateLog.java | [352](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hdfs/src/java/org/apache/solr/hdfs/update/HdfsUpdateLog.java#L352) |
   | solr/core/src/java/org/apache/solr/metrics/reporters/ReporterClientCache.java | [74](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/metrics/reporters/ReporterClientCache.java#L74) |
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [2514](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L2514) |
   | solr/core/src/java/org/apache/solr/schema/IndexSchema.java | [1475](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/schema/IndexSchema.java#L1475) |
   | solr/core/src/java/org/apache/solr/rest/RestManager.java | [537](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/rest/RestManager.java#L537) |
   <p> Showing <b>10</b> of <b> 396 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=Infer|THREAD_SAFETY_VIOLATION" target="_blank">Visit the Lift Web Console</a> to see all.</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=365234406&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234406&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234406&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234406&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234406&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/util/PackageTool.java:
##########
@@ -95,97 +99,137 @@ protected void runImpl(CommandLine cli) throws Exception {
                 break;
               case "add-key":
                 String keyFilename = cli.getArgs()[1];
-                repositoryManager.addKey(FileUtils.readFileToByteArray(new File(keyFilename)), Paths.get(keyFilename).getFileName().toString());
+                Path path = Path.of(keyFilename);
+                repositoryManager.addKey(Files.readAllBytes(path), path.getFileName().toString());
                 break;
               case "list-installed":
                 PackageUtils.printGreen("Installed packages:\n-----");
-                for (SolrPackageInstance pkg: packageManager.fetchInstalledPackageInstances()) {
+                for (SolrPackageInstance pkg : packageManager.fetchInstalledPackageInstances()) {
                   PackageUtils.printGreen(pkg);
                 }
                 break;
               case "list-available":
                 PackageUtils.printGreen("Available packages:\n-----");
-                for (SolrPackage pkg: repositoryManager.getPackages()) {
-                  PackageUtils.printGreen(pkg.name + " \t\t"+pkg.description);
-                  for (SolrPackageRelease version: pkg.versions) {
-                    PackageUtils.printGreen("\tVersion: "+version.version);
+                for (SolrPackage pkg : repositoryManager.getPackages()) {
+                  PackageUtils.printGreen(pkg.name + " \t\t" + pkg.description);
+                  for (SolrPackageRelease version : pkg.versions) {
+                    PackageUtils.printGreen("\tVersion: " + version.version);
                   }
                 }
                 break;
               case "list-deployed":
                 if (cli.hasOption('c')) {
                   String collection = cli.getArgs()[1];
-                  Map<String, SolrPackageInstance> packages = packageManager.getPackagesDeployed(collection);
+                  Map<String, SolrPackageInstance> packages =
+                      packageManager.getPackagesDeployed(collection);
                   PackageUtils.printGreen("Packages deployed on " + collection + ":");
-                  for (String packageName: packages.keySet()) {
+                  for (String packageName : packages.keySet()) {
                     PackageUtils.printGreen("\t" + packages.get(packageName));
                   }
                 } else {
                   String packageName = cli.getArgs()[1];
-                  Map<String, String> deployedCollections = packageManager.getDeployedCollections(packageName);
+                  Map<String, String> deployedCollections =
+                      packageManager.getDeployedCollections(packageName);
                   if (deployedCollections.isEmpty() == false) {
-                    PackageUtils.printGreen("Collections on which package " + packageName + " was deployed:");
-                    for (String collection: deployedCollections.keySet()) {
-                      PackageUtils.printGreen("\t" + collection + "("+packageName+":"+deployedCollections.get(collection)+")");
+                    PackageUtils.printGreen(
+                        "Collections on which package " + packageName + " was deployed:");
+                    for (String collection : deployedCollections.keySet()) {
+                      PackageUtils.printGreen(
+                          "\t"
+                              + collection
+                              + "("
+                              + packageName
+                              + ":"
+                              + deployedCollections.get(collection)

Review Comment:
   <picture><img alt="44% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/44/display.svg"></picture>
   
   đŸ’Ŧ 23 similar findings have been found in this PR
   
   ---
   
   *INEFFICIENT_KEYSET_ITERATOR:*  Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.
   
   ---
   
   <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/ClusterAPI.java | [152](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java#L152) |
   | solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java | [206](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java#L206) |
   | solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [191](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L191) |
   | solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymGraphFilterFactory.java | [219](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymGraphFilterFactory.java#L219) |
   | solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [314](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L314) |
   | solr/core/src/java/org/apache/solr/cluster/placement/impl/AttributeFetcherImpl.java | [165](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/cluster/placement/impl/AttributeFetcherImpl.java#L165) |
   | solr/core/src/java/org/apache/solr/handler/ClusterAPI.java | [153](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java#L153) |
   | solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [108](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L108) |
   | solr/core/src/java/org/apache/solr/core/backup/ShardBackupMetadata.java | [146](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/core/backup/ShardBackupMetadata.java#L146) |
   | solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [251](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L251) |
   <p> Showing <b>10</b> of <b> 23 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=Infer|INEFFICIENT_KEYSET_ITERATOR" target="_blank">Visit the Lift Web Console</a> to see all.</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=365234409&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234409&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234409&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234409&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234409&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/handler/BlobHandler.java:
##########
@@ -282,15 +322,15 @@ public void init(PluginInfo info) {
           maxSize = maxSize * 1024 * 1024;
         }
       }
-
     }
   }
 
   // This does not work for the general case of forwarding requests.  It probably currently
   // works OK for real-time get (which is all that BlobHandler uses it for).
-  private static void forward(SolrQueryRequest req, String handler ,SolrParams params, SolrQueryResponse rsp){
+  private static void forward(
+      SolrQueryRequest req, String handler, SolrParams params, SolrQueryResponse rsp) {
     LocalSolrQueryRequest r = new LocalSolrQueryRequest(req.getCore(), params);
-    SolrRequestInfo.getRequestInfo().addCloseHook( r );  // Close as late as possible...
+    SolrRequestInfo.getRequestInfo().addCloseHook(r); // Close as late as possible...
     req.getCore().getRequestHandler(handler).handleRequest(r, rsp);
   }

Review Comment:
   <picture><img alt="21% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/21/display.svg"></picture>
   
   đŸ’Ŧ 23 similar findings have been found in this PR
   
   ---
   
   *RESOURCE_LEAK:*  resource of type `org.apache.solr.request.LocalSolrQueryRequest` acquired to `r` by call to `LocalSolrQueryRequest(...)` at line 332 is not released after line 335.
   **Note**: potential exception at line 333
   
   ---
   
   <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/servlet/CoordinatorHttpSolrCall.java | [125](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java#L125) |
   | solr/core/src/java/org/apache/solr/util/SimplePostTool.java | [1250](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L1250) |
   | solr/core/src/java/org/apache/solr/util/FileUtils.java | [49](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/FileUtils.java#L49) |
   | solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessorChain.java | [184](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessorChain.java#L184) |
   | solr/core/src/java/org/apache/solr/util/FileUtils.java | [49](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/FileUtils.java#L49) |
   | solr/core/src/java/org/apache/solr/update/UpdateLog.java | [1451](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L1451) |
   | solr/core/src/java/org/apache/solr/handler/GraphHandler.java | [124](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/GraphHandler.java#L124) |
   | solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java | [136](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java#L136) |
   | solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java | [218](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java#L218) |
   | solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java | [91](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java#L91) |
   <p> Showing <b>10</b> of <b> 23 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=Infer|RESOURCE_LEAK" target="_blank">Visit the Lift Web Console</a> to see all.</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=365234416&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234416&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234416&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234416&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234416&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * 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.security;
+
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionParams.CollectionAction.CREATE;
+import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETE;
+import static org.apache.solr.common.params.CollectionParams.CollectionAction.RELOAD;
+import static org.apache.solr.servlet.HttpSolrCall.shouldAudit;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.http.HttpStatus;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.core.CoreContainer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AuthorizationUtils {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  private AuthorizationUtils() {
+    /* Private ctor prevents instantiation */
+  }
+
+  public static class AuthorizationFailure {
+    private final int statusCode;
+    private final String message;
+
+    public AuthorizationFailure(int statusCode, String message) {
+      this.statusCode = statusCode;
+      this.message = message;
+    }
+
+    public int getStatusCode() {
+      return statusCode;
+    }
+
+    public String getMessage() {
+      return message;
+    }
+  }
+
+  public static AuthorizationFailure authorize(
+      HttpServletRequest servletReq,
+      HttpServletResponse response,
+      CoreContainer cores,
+      AuthorizationContext context)
+      throws IOException {
+    log.debug("AuthorizationContext : {}", context);
+    final AuthorizationPlugin authzPlugin = cores.getAuthorizationPlugin();
+    if (authzPlugin == null) {
+      return null; // A 'null' failure retval indicates success
+    }
+    AuthorizationResponse authResponse = authzPlugin.authorize(context);
+    int statusCode = authResponse.statusCode;
+
+    if (statusCode == AuthorizationResponse.PROMPT.statusCode) {
+      @SuppressWarnings({"unchecked"})
+      Map<String, String> headers =
+          (Map<String, String>) servletReq.getAttribute(AuthenticationPlugin.class.getName());
+      if (headers != null) {
+        for (Map.Entry<String, String> e : headers.entrySet())
+          response.setHeader(e.getKey(), e.getValue());

Review Comment:
   <picture><img alt="0% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/0/display.svg"></picture>
   
   đŸ’Ŧ 3 similar findings have been found in this PR
   
   ---
   
   *CROSS_SITE_SCRIPTING:*  UserControlledString(HttpServletRequest.getAttribute(...)) at line 81 ~> HTML(HttpServletResponse.setHeader(...)) at line 84.
   
   ---
   
   <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/servlet/HttpSolrCall.java | [545](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L545) |
   | solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java | [785](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L785) |
   | solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java | [785](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L785) |
   <p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=Infer|CROSS_SITE_SCRIPTING" 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=365234461&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234461&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234461&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234461&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234461&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: 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 #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1263:
URL: https://github.com/apache/solr/pull/1263#discussion_r1060206704


##########
dev-tools/scripts/cherrypick.sh:
##########
@@ -0,0 +1,215 @@
+#!/bin/bash
+# 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.
+
+# Forked and adapted from https://github.com/factorial-io/cherrypicker - MIT license
+# Copyright (c) 2017 Shibin Das - @d34dman
+
+function LOG() {
+  local STATUS=$1
+  local MESSAGE=$2
+  echo "[$(date)] ${STATUS} ${MESSAGE}"
+}
+
+function usage() {
+  cat << EOF
+Usage: dev-tools/scripts/cherrypick.sh [<options>] <commit-hash> [<commit-hash>...]
+ -b <branch> Sets the branch(es) to cherry-pick to, typically branch_Nx or branch_x_y
+ -s          Skips precommit test. WARNING: Always run precommit for code- and doc changes
+ -t          Run the full test suite during check, not only precommit
+ -n          Skips git pull of target branch. Useful if you are without internet access
+ -a          Enters automated mode. Aborts cherry-pick and exits on error
+ -r <remote> Specify remote to push to. Defaults to whatever the branch is tracking.
+ -p          Push to remote. Only done if both cherry-pick and tests succeeded
+    WARNING: Never push changes to a remote branch before a thorough local test
+
+Simple script for aiding in back-porting one or more (trivial) commits to other branches.
+On merge conflict the script will run 'git mergetool'. See 'git mergetool --help'
+for help on configuring your favourite merge tool. Check out Sublime Merge (smerge).
+
+Example:
+  # Backport two commits to both stable and release branches
+  dev-tools/scripts/cherrypick.sh -b branch_9x -b branch_9_0 deadbeef0000 cafebabe1111
+EOF
+}
+
+function yesno() {
+  question=$1
+  unset answer
+  echo "$question"
+  while [[ "$answer" != "y" ]] && [[ "$answer" != "n" ]]; do
+    read -r answer
+    if [[ "$answer" == "y" ]]; then
+      true
+    else
+      false
+    fi
+  done
+}
+
+GIT_COMMAND=git
+PRECOMMIT="true"
+TESTARG="-x test"
+TEST=
+TESTS_PASSED=
+PUSH=
+REMOTE=
+NOPULL=
+AUTO_MODE=
+unset BRANCHES
+
+while getopts ":b:phstnar:" opt; do
+  case ${opt} in
+    b)
+      BRANCHES+=("$OPTARG")
+      ;;
+    r)
+      REMOTE=$OPTARG
+      ;;
+    p)
+      PUSH=true
+      ;;
+    s)
+      PRECOMMIT=
+      ;;
+    a)
+      AUTO_MODE="true"
+      ;;
+    n)
+      NOPULL="true"
+      ;;
+    t)
+      TEST="true"
+      TESTARG=""
+      ;;
+    h)
+      usage
+      exit 0
+      ;;
+   \?)
+      echo "Unknown option $OPTARG"
+      usage
+      exit 1
+   esac
+done
+shift $((OPTIND -1))
+
+COMMITS=( "$@" )
+
+if [ ${#BRANCHES[@]} -eq 0 ]; then
+  LOG INFO "Lacking -b option, must specify target branch. Supports multiple -b options too"
+  usage
+  exit
+fi
+
+if [ ${#COMMITS[@]} -eq 0 ]; then
+  LOG ERROR "Please specify one or more commits"
+  usage
+  exit
+fi
+
+## Loop over branches
+for BRANCH in "${BRANCHES[@]}"; do
+  echo ""
+  LOG "INFO" "============================================";
+  LOG "INFO" "Git branch: $BRANCH"
+  LOG "INFO" "============================================";
+  echo ""
+  LOG INFO "Checking out target branch $BRANCH"
+  # shellcheck disable=SC2086
+  $GIT_COMMAND checkout $BRANCH
+  if [ $? -gt 0 ]; then

Review Comment:
   <picture><img alt="5% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/5/display.svg"></picture>
   
   đŸ’Ŧ 3 similar findings have been found in this PR
   
   ---
   
   *[SC2181](https://github.com/koalaman/shellcheck/wiki/SC2181):*  Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | dev-tools/scripts/cherrypick.sh | [154](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/dev-tools/scripts/cherrypick.sh#L154) |
   | dev-tools/scripts/cherrypick.sh | [185](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/dev-tools/scripts/cherrypick.sh#L185) |
   | dev-tools/scripts/cherrypick.sh | [207](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/dev-tools/scripts/cherrypick.sh#L207) |
   <p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=Shellcheck|SC2181" 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=365235666&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365235666&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365235666&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365235666&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365235666&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: 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 #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1263:
URL: https://github.com/apache/solr/pull/1263#discussion_r1060206585


##########
solr/core/src/java/org/apache/solr/schema/EnumFieldType.java:
##########
@@ -45,19 +44,23 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/**
- * Field type for support of string values with custom sort order.
- */
+/** Field type for support of string values with custom sort order. */
 public class EnumFieldType extends AbstractEnumField {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   @Override
   public Type getUninversionType(SchemaField sf) {

Review Comment:
   <picture><img alt="32% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/32/display.svg"></picture>
   
   đŸ’Ŧ 2 similar findings have been found in this PR
   
   ---
   
   *[BadImport](https://errorprone.info/bugpattern/BadImport):*  Importing nested classes/static methods/static fields with commonly-used names can make code harder to read, because it may not be clear from the context exactly which type is being referred to. Qualifying the name with that of the containing class can make the code clearer. Here we recommend using qualified class: UninvertingReader.
   
   ---
   
   
   ```suggestion
     public UninvertingReader.Type getUninversionType(SchemaField sf) {
   ```
   
   
   ---
   
   <details><summary><b>🔎 Expand here to view all instances of this finding</b></summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | ------------- | ------------- |
   | solr/modules/analytics/src/java/org/apache/solr/analytics/AnalyticsRequestParser.java | [104](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/analytics/src/java/org/apache/solr/analytics/AnalyticsRequestParser.java#L104) |
   | solr/modules/analysis-extras/src/java/org/apache/solr/schema/ICUCollationField.java | [259](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/analysis-extras/src/java/org/apache/solr/schema/ICUCollationField.java#L259) |
   <p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=ErrorProne|BadImport" 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=365234098&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234098&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234098&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234098&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234098&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/core/DirectoryFactory.java:
##########
@@ -342,36 +338,45 @@ public String getDataHome(CoreDescriptor cd) throws IOException {
     return dataDir.toString();
   }
 
-  public void cleanupOldIndexDirectories(final String dataDirPath, final String currentIndexDirPath, boolean afterCoreReload) {
+  public void cleanupOldIndexDirectories(
+      final String dataDirPath, final String currentIndexDirPath, boolean afterCoreReload) {
     File dataDir = new File(dataDirPath);
     if (!dataDir.isDirectory()) {
-      log.debug("{} does not point to a valid data directory; skipping clean-up of old index directories.", dataDirPath);
+      log.debug(
+          "{} does not point to a valid data directory; skipping clean-up of old index directories.",
+          dataDirPath);
       return;
     }
 
     final File currentIndexDir = new File(currentIndexDirPath);

Review Comment:
   <picture><img alt="10% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/10/display.svg"></picture>
   
   đŸ’Ŧ 100 similar findings have been found in this PR
   
   ---
   
   *[PATH_TRAVERSAL_IN](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN):*  This API (java/io/File.<init>(Ljava/lang/String;)V) reads 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/rest/ManagedResourceStorage.java | [183](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java#L183) |
   | solr/modules/hdfs/src/java/org/apache/solr/hdfs/store/blockcache/BlockDirectory.java | [294](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/modules/hdfs/src/java/org/apache/solr/hdfs/store/blockcache/BlockDirectory.java#L294) |
   | solr/test-framework/src/java/org/apache/solr/SolrTestCase.java | [108](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java#L108) |
   | solr/core/src/java/org/apache/solr/util/VersionedFile.java | [68](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/VersionedFile.java#L68) |
   | solr/core/src/java/org/apache/solr/handler/SnapShooter.java | [79](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/SnapShooter.java#L79) |
   | solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java | [188](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java#L188) |
   | solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java | [178](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java#L178) |
   | solr/core/src/java/org/apache/solr/util/SolrCLI.java | [3189](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/SolrCLI.java#L3189) |
   | solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java | [144](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java#L144) |
   | solr/core/src/java/org/apache/solr/core/ZkContainer.java | [96](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/core/ZkContainer.java#L96) |
   <p> Showing <b>10</b> of <b> 100 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|PATH_TRAVERSAL_IN" target="_blank">Visit the Lift Web Console</a> to see all.</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=365234122&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234122&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234122&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234122&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234122&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java:
##########
@@ -155,45 +156,67 @@ public static String getJsonStringFromUrl(HttpClient client, String url) {
   }
 
   /**
-   * Fetches a manifest file from the File Store / Package Store. A SHA512 check is enforced after fetching.
+   * Fetches a manifest file from the File Store / Package Store. A SHA512 check is enforced after
+   * fetching.
    */
-  public static Manifest fetchManifest(HttpSolrClient solrClient, String solrBaseUrl, String manifestFilePath, String expectedSHA512) throws MalformedURLException, IOException {
-    String manifestJson = PackageUtils.getJsonStringFromUrl(solrClient.getHttpClient(), solrBaseUrl + "/api/node/files" + manifestFilePath);
-    String calculatedSHA512 = BlobRepository.sha512Digest(ByteBuffer.wrap(manifestJson.getBytes("UTF-8")));
+  public static Manifest fetchManifest(
+      HttpSolrClient solrClient, String solrBaseUrl, String manifestFilePath, String expectedSHA512)
+      throws MalformedURLException, IOException {
+    String manifestJson =
+        PackageUtils.getJsonStringFromUrl(
+            solrClient.getHttpClient(), solrBaseUrl + "/api/node/files" + manifestFilePath);
+    String calculatedSHA512 =
+        BlobRepository.sha512Digest(ByteBuffer.wrap(manifestJson.getBytes("UTF-8")));
     if (expectedSHA512.equals(calculatedSHA512) == false) {

Review Comment:
   đŸ’Ŧ 9 similar findings have been found in this PR
   
   ---
   
   *[UNSAFE_HASH_EQUALS](https://find-sec-bugs.github.io/bugs.htm#UNSAFE_HASH_EQUALS):*  Unsafe comparison of hash that are susceptible to timing attack
   
   ---
   
   <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/filestore/DistribPackageStore.java | [204](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java#L204) |
   | solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java | [129](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java#L129) |
   | solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java | [1399](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java#L1399) |
   | solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java | [260](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java#L260) |
   | solr/core/src/java/org/apache/solr/core/BlobRepository.java | [187](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/core/BlobRepository.java#L187) |
   | solr/core/src/java/org/apache/solr/rest/RestManager.java | [317](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/rest/RestManager.java#L317) |
   | solr/core/src/java/org/apache/solr/search/facet/FacetModule.java | [216](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java#L216) |
   | solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java | [300](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java#L300) |
   | solr/solrj/src/java/org/apache/solr/common/params/ShardParams.java | [118](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/solrj/src/java/org/apache/solr/common/params/ShardParams.java#L118) |
   <p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|UNSAFE_HASH_EQUALS" 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=365234132&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234132&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234132&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234132&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234132&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/core/XmlConfigFile.java:
##########
@@ -237,32 +219,34 @@ public Node getNode(String path, Document doc, boolean errIfMissing) {
     String xstr = normalize(path);
 
     try {
-      NodeList nodes = (NodeList)xpath.evaluate(xstr, doc, 
-                                                XPathConstants.NODESET);
-      if (nodes==null || 0 == nodes.getLength() ) {
+      NodeList nodes = (NodeList) xpath.evaluate(xstr, doc, XPathConstants.NODESET);

Review Comment:
   đŸ’Ŧ 6 similar findings have been found in this PR
   
   ---
   
   *[XPATH_INJECTION](https://find-sec-bugs.github.io/bugs.htm#XPATH_INJECTION):*  This use of javax/xml/xpath/XPath.evaluate(Ljava/lang/String;Ljava/lang/Object;Ljavax/xml/namespace/QName;)Ljava/lang/Object; can be vulnerable to XPath Injection
   
   ---
   
   <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/core/XmlConfigFile.java | [204](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/core/XmlConfigFile.java#L204) |
   | solr/core/src/java/org/apache/solr/util/SimplePostTool.java | [1146](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L1146) |
   | solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java | [91](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java#L91) |
   | solr/test-framework/src/java/org/apache/solr/util/DOMUtilTestBase.java | [47](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/test-framework/src/java/org/apache/solr/util/DOMUtilTestBase.java#L47) |
   | solr/core/src/java/org/apache/solr/core/XmlConfigFile.java | [258](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/core/XmlConfigFile.java#L258) |
   | solr/core/src/java/org/apache/solr/schema/AbstractEnumField.java | [115](https://github.com/apache/solr/blob/5884467f49024e2fa15eb51f77f04d21f626e367/solr/core/src/java/org/apache/solr/schema/AbstractEnumField.java#L115) |
   <p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?t=FindSecBugs|XPATH_INJECTION" 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=365234161&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234161&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234161&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234161&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234161&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: 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 #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1263:
URL: https://github.com/apache/solr/pull/1263#discussion_r1060206517


##########
solr/core/build.gradle:
##########
@@ -59,38 +100,50 @@ dependencies {
   implementation "org.apache.lucene:lucene-spatial-extras"
   implementation "org.apache.lucene:lucene-suggest"
 
+
   // Collections & lang utilities
-  implementation ('com.google.guava:guava') { transitive = false }
+  implementation 'com.google.guava:guava'
   implementation 'org.apache.commons:commons-lang3'
   implementation 'org.apache.commons:commons-math3'
   implementation 'commons-io:commons-io'
   implementation 'com.carrotsearch:hppc'
   implementation 'org.apache.commons:commons-collections4'
-  runtimeOnly 'commons-collections:commons-collections' // for Hadoop and...?
 
   implementation('com.github.ben-manes.caffeine:caffeine') { transitive = false }
 
-  implementation 'org.slf4j:slf4j-api'
-
   implementation 'commons-codec:commons-codec'
 
   implementation 'commons-cli:commons-cli'
 
+  implementation 'org.locationtech.spatial4j:spatial4j'
+
+  implementation 'com.fasterxml.jackson.core:jackson-annotations'
+  implementation 'com.fasterxml.jackson.core:jackson-core'
   implementation 'com.fasterxml.jackson.core:jackson-databind'
   implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-smile'

Review Comment:
   <picture><img alt="52% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/52/display.svg"></picture>
   
   *High Vulnerability:*
   ### maven : com.fasterxml.jackson.dataformat/jackson-dataformat-smile : 2.14.1
   0 Critical, 1 High, 0 Medium, 0 Low vulnerabilities have been found across 1 dependencies.
   [View the Lift console](https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?tab=dependencies&component=pkg%3Amaven%2Fcom.fasterxml.jackson.dataformat%2Fjackson-dataformat-smile%402.14.1) for details about these vulnerabilities.
   
   ---
   
   <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=365233899&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365233899&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365233899&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365233899&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365233899&lift_comment_rating=5) ]



##########
solr/core/build.gradle:
##########
@@ -59,38 +100,50 @@ dependencies {
   implementation "org.apache.lucene:lucene-spatial-extras"
   implementation "org.apache.lucene:lucene-suggest"
 
+
   // Collections & lang utilities
-  implementation ('com.google.guava:guava') { transitive = false }
+  implementation 'com.google.guava:guava'

Review Comment:
   <picture><img alt="47% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/47/display.svg"></picture>
   
   *Medium Vulnerability:*
   ### maven : com.google.guava/guava : 31.1-jre
   0 Critical, 0 High, 1 Medium, 0 Low vulnerabilities have been found across 1 dependencies.
   [View the Lift console](https://lift.sonatype.com/results/github.com/apache/solr/01GNT825EMV63AWZ6P5BHSYYB2?tab=dependencies&component=pkg%3Amaven%2Fcom.google.guava%2Fguava%4031.1-jre) for details about these vulnerabilities.
   
   ---
   
   <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=365233902&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365233902&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365233902&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365233902&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365233902&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: 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 #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1263:
URL: https://github.com/apache/solr/pull/1263#discussion_r1060206538


##########
solr/core/src/java/org/apache/solr/handler/admin/ZookeeperStatusHandler.java:
##########
@@ -292,42 +333,62 @@ protected List<String> getZkRawResponse(String zkHostPort, String fourLetterWord
       port = Integer.parseInt(hostPort[1]);
     }
 
-    try (
-        Socket socket = new Socket(host, port);
+    try (Socket socket = new Socket(host, port);

Review Comment:
   *[UNENCRYPTED_SOCKET](https://find-sec-bugs.github.io/bugs.htm#UNENCRYPTED_SOCKET):*  Unencrypted socket to org.apache.solr.handler.admin.ZookeeperStatusHandler (instead of SSLSocket)
   
   ---
   
   <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=365234367&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234367&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234367&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234367&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234367&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/update/processor/MD5Signature.java:
##########
@@ -15,21 +15,23 @@
  * limitations under the License.
  */
 package org.apache.solr.update.processor;
+
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
 public class MD5Signature extends Signature {
-  private static ThreadLocal<MessageDigest> DIGESTER_FACTORY = new ThreadLocal<MessageDigest>() {
-    @Override
-    protected MessageDigest initialValue() {
-      try {
-        return MessageDigest.getInstance("MD5");
-      } catch (NoSuchAlgorithmException e) {
-        throw new RuntimeException(e);
-      }
-    }
-  };
+  private static ThreadLocal<MessageDigest> DIGESTER_FACTORY =
+      new ThreadLocal<MessageDigest>() {
+        @Override
+        protected MessageDigest initialValue() {
+          try {
+            return MessageDigest.getInstance("MD5");

Review Comment:
   *[WEAK_MESSAGE_DIGEST_MD5](https://find-sec-bugs.github.io/bugs.htm#WEAK_MESSAGE_DIGEST_MD5):*  This API MD5 (MDX) is not a recommended cryptographic hash function
   
   ---
   
   <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=365234348&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234348&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234348&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234348&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234348&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/util/CryptoKeys.java:
##########
@@ -129,21 +115,21 @@ public static PublicKey getX509PublicKey(byte[] buf)
    * Verify the signature of a file
    *
    * @param publicKey the public key used to sign this
-   * @param sig       the signature
-   * @param data      The data tha is signed
+   * @param sig the signature
+   * @param data The data tha is signed
    */
-  public static boolean verify(PublicKey publicKey, byte[] sig, ByteBuffer data) throws InvalidKeyException, SignatureException {
+  public static boolean verify(PublicKey publicKey, byte[] sig, ByteBuffer data)
+      throws InvalidKeyException, SignatureException {
     data = ByteBuffer.wrap(data.array(), data.arrayOffset(), data.limit());
     try {
       Signature signature = Signature.getInstance("SHA1withRSA");

Review Comment:
   *[WEAK_MESSAGE_DIGEST_SHA1](https://find-sec-bugs.github.io/bugs.htm#WEAK_MESSAGE_DIGEST_SHA1):*  This API SHA1 (SHA-1) is not a recommended cryptographic hash function
   
   ---
   
   <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=365234368&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234368&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234368&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234368&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234368&lift_comment_rating=5) ]



##########
solr/core/src/java/org/apache/solr/servlet/RedirectServlet.java:
##########
@@ -17,47 +17,43 @@
 package org.apache.solr.servlet;
 
 import java.io.IOException;
-
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-/**
- * A Simple redirection servlet to help us deprecate old UI elements
- */
+/** A Simple redirection servlet to help us deprecate old UI elements */
 public class RedirectServlet extends BaseSolrServlet {
-  
+
   static final String CONTEXT_KEY = "${context}";
-  
+
   String destination;
   int code = HttpServletResponse.SC_MOVED_PERMANENTLY;
-  
+
   @Override
   public void init(ServletConfig config) throws ServletException {
     super.init(config);
-    
+
     destination = config.getInitParameter("destination");
-    if(destination==null) {
+    if (destination == null) {
       throw new ServletException("RedirectServlet missing destination configuration");
     }
-    if( "false".equals(config.getInitParameter("permanent") )) {
+    if ("false".equals(config.getInitParameter("permanent"))) {
       code = HttpServletResponse.SC_MOVED_TEMPORARILY;
     }
-    
+
     // Replace the context key
-    if(destination.startsWith(CONTEXT_KEY)) {
-      destination = config.getServletContext().getContextPath()
-          +destination.substring(CONTEXT_KEY.length());
+    if (destination.startsWith(CONTEXT_KEY)) {
+      destination =
+          config.getServletContext().getContextPath() + destination.substring(CONTEXT_KEY.length());
     }
   }
-  
+
   @Override
   public void doGet(HttpServletRequest req, HttpServletResponse res)
-          throws ServletException,IOException {
-      
+      throws ServletException, IOException {
+
     res.setStatus(code);
     res.setHeader("Location", destination);

Review Comment:
   *[UNVALIDATED_REDIRECT](https://find-sec-bugs.github.io/bugs.htm#UNVALIDATED_REDIRECT):*  The following redirection could be used by an attacker to redirect users to a phishing website.
   
   ---
   
   <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=365234405&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=365234405&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234405&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=365234405&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=365234405&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: 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] vatsalpatel3689 closed pull request #1263: diff with 9_1

Posted by GitBox <gi...@apache.org>.
vatsalpatel3689 closed pull request #1263: diff with 9_1
URL: https://github.com/apache/solr/pull/1263


-- 
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