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

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


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: [email protected]

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


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

Reply via email to