You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "renatoh (via GitHub)" <gi...@apache.org> on 2023/01/23 09:29:18 UTC
[GitHub] [solr] renatoh opened a new pull request, #1307: Small code improvements in facet component
renatoh opened a new pull request, #1307:
URL: https://github.com/apache/solr/pull/1307
backporting "Debugging FacetComponent I noticed two little things which can be improved"
--
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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083883602
##########
solr/core/src/java/org/apache/solr/core/ConfigSet.java:
##########
@@ -53,17 +56,17 @@ public String getName() {
}
public SolrConfig getSolrConfig() {
- return solrconfig;
+ return solrConfig;
}
/**
- *
* @param forceFetch get a fresh value and not cached value
Review Comment:
<picture><img alt="45% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/45/display.svg"></picture>
đŦ 6 similar findings have been found in this PR
---
*[MissingSummary](https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment):* A summary line is required on public/protected Javadocs.
---
<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/ResponseParser.java | [60](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java#L60) |
| solr/core/src/java/org/apache/solr/request/LocalSolrQueryRequest.java | [34](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/request/LocalSolrQueryRequest.java#L34) |
| solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/cloud/AlreadyExistsException.java | [20](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/cloud/AlreadyExistsException.java#L20) |
| solr/solrj/src/java/org/apache/solr/common/AlreadyClosedException.java | [20](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj/src/java/org/apache/solr/common/AlreadyClosedException.java#L20) |
| solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java | [26](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java#L26) |
| solr/modules/clustering/src/java/org/apache/solr/handler/clustering/ClusteringComponent.java | [499](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/clustering/src/java/org/apache/solr/handler/clustering/ClusteringComponent.java#L499) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?t=ErrorProne|MissingSummary" 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=371755703&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371755703&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755703&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755703&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371755703&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/rest/RestManager.java:
##########
@@ -289,33 +291,35 @@ public void doInit(SolrQueryRequest solrRequest, SolrQueryResponse solrResponse)
// see if we have a registered endpoint one-level up ...
int lastSlashAt = resourceId.lastIndexOf('/');
if (lastSlashAt != -1) {
- String parentResourceId = resourceId.substring(0,lastSlashAt);
- log.info("Resource not found for {}, looking for parent: {}",
- resourceId, parentResourceId);
+ String parentResourceId = resourceId.substring(0, lastSlashAt);
+ log.info(
+ "Resource not found for {}, looking for parent: {}", resourceId, parentResourceId);
managedResource = restManager.getManagedResourceOrNull(parentResourceId);
if (managedResource != null) {
// verify this resource supports child resources
if (!(managedResource instanceof ManagedResource.ChildResourceSupport)) {
- String errMsg = String.format(Locale.ROOT,
- "%s does not support child resources!", managedResource.getResourceId());
+ String errMsg =
+ String.format(
+ Locale.ROOT,
+ "%s does not support child resources!",
+ managedResource.getResourceId());
throw new SolrException(ErrorCode.BAD_REQUEST, errMsg);
}
-
- childId = resourceId.substring(lastSlashAt+1);
- log.info("Found parent resource {} for child: {}",
- parentResourceId, childId);
+
+ childId = resourceId.substring(lastSlashAt + 1);
+ log.info("Found parent resource {} for child: {}", parentResourceId, childId);
}
}
- }
+ }
if (managedResource == null) {
final String method = getSolrRequest().getHttpMethod();
if ("PUT".equals(method) || "POST".equals(method)) {
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/search/facet/FacetModule.java | [216](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java#L216) |
| solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java | [1398](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java#L1398) |
| solr/core/src/java/org/apache/solr/core/BlobRepository.java | [187](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/core/BlobRepository.java#L187) |
| solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java | [129](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java#L129) |
| solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java | [300](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/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/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj/src/java/org/apache/solr/common/params/ShardParams.java#L118) |
| solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java | [204](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java#L204) |
| solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java | [170](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java#L170) |
| solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java | [260](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java#L260) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756017&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756017&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756017&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756017&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756017&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java:
##########
@@ -143,7 +145,10 @@ private List<BoostEntry> initBoostEntries(InputStream is) throws IOException {
newBoostEntries.add(new BoostEntry(Pattern.compile(regexp), Double.parseDouble(boost)));
log.debug("Read regexp {} with boost {}", regexp, boost);
} else {
- log.warn("Malformed config input line: {} (expected 2 fields, got {} fields). Skipping entry.", line, fields.length);
+ log.warn(
Review Comment:
<picture><img alt="15% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/15/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.warn(Ljava/lang/String;Ljava/lang/Object;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/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java | [146](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java#L146) |
| solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java | [102](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java#L102) |
| solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java | [335](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java#L335) |
| solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/KerberosFilter.java | [98](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/KerberosFilter.java#L98) |
| solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java | [227](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/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 | [237](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/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 | [228](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java#L228) |
| solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java | [221](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java#L221) |
| solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java | [249](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java#L249) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756018&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756018&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756018&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756018&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756018&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java:
##########
@@ -39,57 +38,95 @@
import org.xml.sax.SAXException;
/**
- * Some utility methods for parsing XML in a safe way. This class can be used to parse XML
- * coming from network (completely untrusted) or it can load a config file from a
- * {@link ResourceLoader}. In this case it allows external entities and xincludes, but only
- * referring to files reachable by the loader.
+ * Some utility methods for parsing XML in a safe way. This class can be used to parse XML coming
+ * from network (completely untrusted) or it can load a config file from a {@link ResourceLoader}.
+ * In this case it allows external entities and xincludes, but only referring to files reachable by
+ * the loader.
*/
-@SuppressForbidden(reason = "This class uses XML APIs directly that should not be used anywhere else in Solr code")
-public final class SafeXMLParsing {
-
+@SuppressForbidden(
+ reason = "This class uses XML APIs directly that should not be used anywhere else in Solr code")
+public final class SafeXMLParsing {
+
public static final String SYSTEMID_UNTRUSTED = "untrusted://stream";
private SafeXMLParsing() {}
-
- /** Parses a config file from ResourceLoader. Xinclude and external entities are enabled, but cannot escape the resource loader. */
- public static Document parseConfigXML(Logger log, ResourceLoader loader, String file) throws SAXException, IOException {
+
+ /**
+ * Parses a config file from a Solr config based on ResourceLoader. Xinclude and external entities
+ * are enabled, but cannot escape the resource loader.
+ */
+ public static Document parseConfigXML(Logger log, ResourceLoader loader, String file)
+ throws SAXException, IOException {
try (InputStream in = loader.openResource(file)) {
+ DocumentBuilder db = configDocumentBuilder(loader, log);
+ return db.parse(in, SystemIdResolver.createSystemIdFromResourceName(file));
+ }
+ }
+
+ /**
+ * Parses a config file from a Solr config based on InputSource. Xinclude and external entities
+ * are enabled, but cannot escape the resource loader.
+ */
+ public static Document parseConfigXML(Logger log, ResourceLoader loader, InputSource is)
+ throws SAXException, IOException {
+ return configDocumentBuilder(loader, log).parse(is);
Review Comment:
đŦ 9 similar findings have been found in this PR
---
*[XXE_DOCUMENT](https://find-sec-bugs.github.io/bugs.htm#XXE_DOCUMENT):* The use of DocumentBuilder.parse(...) (DocumentBuilder) 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/core/src/java/org/apache/solr/util/SafeXMLParsing.java | [109](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java#L109) |
| solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java | [82](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java#L82) |
| solr/test-framework/src/java/org/apache/solr/util/DOMUtilTestBase.java | [51](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/util/DOMUtilTestBase.java#L51) |
| solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java | [62](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java#L62) |
| solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsConfiguration.java | [98](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsConfiguration.java#L98) |
| solr/core/src/java/org/apache/solr/util/SimplePostTool.java | [1175](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L1175) |
| solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java | [108](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java#L108) |
| solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java | [125](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java#L125) |
| solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsConfiguration.java | [105](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsConfiguration.java#L105) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?t=FindSecBugs|XXE_DOCUMENT" 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=371756021&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756021&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756021&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756021&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756021&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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083883916
##########
solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java:
##########
@@ -48,145 +50,155 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static org.apache.solr.common.util.Utils.toJSONString;
-
/**
- * Abstract base class that provides most of the functionality needed
- * to store arbitrary data for managed resources. Concrete implementations
- * need to decide the underlying format that data is stored in, such as JSON.
- *
- * The underlying storage I/O layer will be determined by the environment
- * Solr is running in, e.g. in cloud mode, data will be stored and loaded
- * from ZooKeeper.
+ * Abstract base class that provides most of the functionality needed to store arbitrary data for
+ * managed resources. Concrete implementations need to decide the underlying format that data is
+ * stored in, such as JSON.
+ *
+ * <p>The underlying storage I/O layer will be determined by the environment Solr is running in,
+ * e.g. in cloud mode, data will be stored and loaded from ZooKeeper.
*/
public abstract class ManagedResourceStorage {
-
+
/**
- * Hides the underlying storage implementation for data being managed
- * by a ManagedResource. For instance, a ManagedResource may use JSON as
- * the data format and an instance of this class to persist and load
- * the JSON bytes to/from some backing store, such as ZooKeeper.
+ * Hides the underlying storage implementation for data being managed by a ManagedResource. For
+ * instance, a ManagedResource may use JSON as the data format and an instance of this class to
+ * persist and load the JSON bytes to/from some backing store, such as ZooKeeper.
*/
public static interface StorageIO {
String getInfo();
+
void configure(SolrResourceLoader loader, NamedList<String> initArgs) throws SolrException;
+
boolean exists(String storedResourceId) throws IOException;
- InputStream openInputStream(String storedResourceId) throws IOException;
+
+ InputStream openInputStream(String storedResourceId) throws IOException;
+
OutputStream openOutputStream(String storedResourceId) throws IOException;
+
boolean delete(String storedResourceId) throws IOException;
}
-
- public static final String STORAGE_IO_CLASS_INIT_ARG = "storageIO";
+
+ public static final String STORAGE_IO_CLASS_INIT_ARG = "storageIO";
public static final String STORAGE_DIR_INIT_ARG = "storageDir";
-
+
/**
- * Creates a new StorageIO instance for a Solr core, taking into account
- * whether the core is running in cloud mode as well as initArgs.
+ * Creates a new StorageIO instance for a Solr core, taking into account whether the core is
+ * running in cloud mode as well as initArgs.
*/
- public static StorageIO newStorageIO(String collection, SolrResourceLoader resourceLoader, NamedList<String> initArgs) {
+ public static StorageIO newStorageIO(
+ String collection, SolrResourceLoader resourceLoader, NamedList<String> initArgs) {
StorageIO storageIO;
SolrZkClient zkClient = null;
String configName = null;
if (resourceLoader instanceof ZkSolrResourceLoader) {
- zkClient = ((ZkSolrResourceLoader)resourceLoader).getZkController().getZkClient();
+ zkClient = ((ZkSolrResourceLoader) resourceLoader).getZkController().getZkClient();
try {
- final ZkStateReader zkStateReader = ((ZkSolrResourceLoader)resourceLoader).getZkController().getZkStateReader();
+ final ZkStateReader zkStateReader =
+ ((ZkSolrResourceLoader) resourceLoader).getZkController().getZkStateReader();
configName = zkStateReader.getClusterState().getCollection(collection).getConfigName();
} catch (Exception e) {
log.error("Failed to get config name due to", e);
- throw new SolrException(ErrorCode.SERVER_ERROR,
- "Failed to load config name for collection:" + collection + " due to: ", e);
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Failed to load config name for collection:" + collection + " due to: ",
+ e);
}
if (configName == null) {
- throw new SolrException(ErrorCode.SERVER_ERROR,
- "Could not find config name for collection:" + collection);
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR, "Could not find config name for collection:" + collection);
}
}
-
+
if (initArgs.get(STORAGE_IO_CLASS_INIT_ARG) != null) {
- storageIO = resourceLoader.newInstance(initArgs.get(STORAGE_IO_CLASS_INIT_ARG), StorageIO.class);
+ storageIO =
+ resourceLoader.newInstance(initArgs.get(STORAGE_IO_CLASS_INIT_ARG), StorageIO.class);
} else {
if (zkClient != null) {
- String znodeBase = "/configs/"+configName;
- log.debug("Setting up ZooKeeper-based storage for the RestManager with znodeBase: {}", znodeBase);
+ String znodeBase = "/configs/" + configName;
+ log.debug(
+ "Setting up ZooKeeper-based storage for the RestManager with znodeBase: {}", znodeBase);
storageIO = new ManagedResourceStorage.ZooKeeperStorageIO(zkClient, znodeBase);
} else {
- storageIO = new FileStorageIO();
+ storageIO = new FileStorageIO();
}
}
-
+
if (storageIO instanceof FileStorageIO) {
- // using local fs, if storageDir is not set in the solrconfig.xml, assume the configDir for the core
+ // using local fs, if storageDir is not set in the solrconfig.xml, assume the configDir for
+ // the core
if (initArgs.get(STORAGE_DIR_INIT_ARG) == null) {
- File configDir = new File(resourceLoader.getConfigDir());
- boolean hasAccess = false;
+ Path configDir = resourceLoader.getConfigPath();
+ boolean hasAccess;
try {
- hasAccess = configDir.isDirectory() && configDir.canWrite();
- } catch (java.security.AccessControlException ace) {}
-
+ hasAccess = Files.isDirectory(configDir) && Files.isWritable(configDir);
+ } catch (SecurityException noAccess) {
+ hasAccess = false;
+ }
+
if (hasAccess) {
- initArgs.add(STORAGE_DIR_INIT_ARG, configDir.getAbsolutePath());
+ initArgs.add(STORAGE_DIR_INIT_ARG, configDir.toAbsolutePath().toString());
} else {
- // most likely this is because of a unit test
+ // most likely this is because of a unit test
// that doesn't have write-access to the config dir
// while this failover approach is not ideal, it's better
// than causing the core to fail esp. if managed resources aren't being used
- log.warn("Cannot write to config directory {} ; switching to use InMemory storage instead.", configDir.getAbsolutePath());
+ log.warn(
+ "Cannot write to config directory {} ; switching to use InMemory storage instead.",
+ configDir.toAbsolutePath());
storageIO = new ManagedResourceStorage.InMemoryStorageIO();
}
- }
+ }
}
-
- storageIO.configure(resourceLoader, initArgs);
-
+
+ storageIO.configure(resourceLoader, initArgs);
+
return storageIO;
}
-
- /**
- * Local file-based storage implementation.
- */
+
+ /** Local file-based storage implementation. */
public static class FileStorageIO implements StorageIO {
private String storageDir;
-
+
@Override
- public void configure(SolrResourceLoader loader, NamedList<String> initArgs) throws SolrException {
+ public void configure(SolrResourceLoader loader, NamedList<String> initArgs)
+ throws SolrException {
String storageDirArg = initArgs.get(STORAGE_DIR_INIT_ARG);
-
+
if (storageDirArg == null || storageDirArg.trim().length() == 0)
- throw new IllegalArgumentException("Required configuration parameter '"+
- STORAGE_DIR_INIT_ARG+"' not provided!");
-
+ throw new IllegalArgumentException(
+ "Required configuration parameter '" + STORAGE_DIR_INIT_ARG + "' not provided!");
+
File dir = new File(storageDirArg);
- if (!dir.isDirectory())
- dir.mkdirs();
+ if (!dir.isDirectory()) dir.mkdirs();
- storageDir = dir.getAbsolutePath();
+ storageDir = dir.getAbsolutePath();
log.info("File-based storage initialized to use dir: {}", storageDir);
}
-
+
@Override
public boolean exists(String storedResourceId) throws IOException {
return (new File(storageDir, storedResourceId)).exists();
- }
-
+ }
+
@Override
public InputStream openInputStream(String storedResourceId) throws IOException {
- return new FileInputStream(storageDir+"/"+storedResourceId);
+ return new FileInputStream(storageDir + "/" + storedResourceId);
}
@Override
public OutputStream openOutputStream(String storedResourceId) throws IOException {
- return new FileOutputStream(storageDir+"/"+storedResourceId);
+ return new FileOutputStream(storageDir + "/" + storedResourceId);
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/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java#L67) |
| solr/core/src/java/org/apache/solr/util/ExportTool.java | [253](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/ExportTool.java#L253) |
| solr/core/src/java/org/apache/solr/util/ExportTool.java | [327](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/ExportTool.java#L327) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756028&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756028&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756028&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756028&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756028&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/util/SimplePostTool.java:
##########
@@ -1135,30 +1225,33 @@ public PageFetcherResult readPageFromUrl(URL u) throws URISyntaxException {
PageFetcherResult res = new PageFetcherResult();
try {
if (isDisallowedByRobots(u)) {
- warn("The URL "+u+" is disallowed by robots.txt and will not be crawled.");
+ warn("The URL " + u + " is disallowed by robots.txt and will not be crawled.");
res.httpStatus = 403;
URI uri = u.toURI();
visited.add(uri);
return res;
}
res.httpStatus = 404;
HttpURLConnection conn = (HttpURLConnection) u.openConnection();
Review Comment:
<picture><img alt="13% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/13/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/core/src/java/org/apache/solr/util/CryptoKeys.java | [278](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/CryptoKeys.java#L278) |
| solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java | [150](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java#L150) |
| solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java | [132](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java#L132) |
| solr/core/src/java/org/apache/solr/util/SimplePostTool.java | [999](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L999) |
| solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java | [41](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java#L41) |
| solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java | [465](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java#L465) |
| solr/core/src/java/org/apache/solr/util/SimplePostTool.java | [975](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/SimplePostTool.java#L975) |
| solr/test-framework/src/java/org/apache/solr/handler/BackupRestoreUtils.java | [116](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/handler/BackupRestoreUtils.java#L116) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756029&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756029&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756029&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756029&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756029&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java:
##########
@@ -787,36 +730,36 @@ public static CharSequence partialEscape(CharSequence s) {
// Pattern to detect dangling operator(s) at end of query
// \s+[-+\s]+$
- private final static Pattern DANGLING_OP_PATTERN = Pattern.compile( "\\s+[-+\\s]+$" );
+ private static final Pattern DANGLING_OP_PATTERN = Pattern.compile("\\s+[-+\\s]+$");
// Pattern to detect consecutive + and/or - operators
// \s+[+-](?:\s*[+-]+)+
- private final static Pattern CONSECUTIVE_OP_PATTERN = Pattern.compile( "\\s+[+-](?:\\s*[+-]+)+" );
+ private static final Pattern CONSECUTIVE_OP_PATTERN = Pattern.compile("\\s+[+-](?:\\s*[+-]+)+");
Review Comment:
đŦ 4 similar findings have been found in this PR
---
*[REDOS](https://find-sec-bugs.github.io/bugs.htm#REDOS):* The regular expression "\\s+[+-](?:\\s*[+-]+)+" is vulnerable to a denial of service attack (ReDOS)
---
<details><summary><b>đ Expand here to view all instances of this finding</b></summary><br/>
<div align=\"center\">
| **File Path** | **Line Number** |
| ------------- | ------------- |
| solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsQueryTemplate.java | [41](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsQueryTemplate.java#L41) |
| solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java | [428](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java#L428) |
| solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkDynamicConfig.java | [39](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkDynamicConfig.java#L39) |
| solr/core/src/java/org/apache/solr/handler/admin/SolrEnvironment.java | [36](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/handler/admin/SolrEnvironment.java#L36) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?t=FindSecBugs|REDOS" 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=371756051&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756051&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756051&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756051&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756051&lift_comment_rating=5) ]
##########
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/servlet/HttpSolrCall.java | [854](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L854) |
| solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java | [204](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java#L204) |
| solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java | [890](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java#L890) |
| solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java | [818](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java#L818) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756073&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756073&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756073&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756073&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756073&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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083882639
##########
solr/core/src/java/org/apache/solr/handler/RestoreCore.java:
##########
@@ -181,20 +204,20 @@ private void openNewSearcher() throws Exception {
}
}
- /**
- * A minimal version of {@link BackupRepository} used for restoring
- */
+ /** A minimal version of {@link BackupRepository} used for restoring */
private interface RestoreRepository {
String[] listAllFiles() throws IOException;
+
IndexInput openInput(String filename) throws IOException;
Review Comment:
<picture><img alt="24% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/24/display.svg"></picture>
*[UnusedMethod](https://errorprone.info/bugpattern/UnusedMethod):* Method 'openInput' is never used.
---
<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=371755794&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371755794&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755794&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755794&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371755794&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=371756042&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756042&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756042&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756042&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756042&lift_comment_rating=5) ]
##########
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=371756131&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756131&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756131&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756131&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756131&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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083883173
##########
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=371756240&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756240&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756240&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756240&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756240&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/util/CryptoKeys.java:
##########
@@ -129,21 +119,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=371756294&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756294&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756294&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756294&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756294&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/schema/EnumFieldType.java:
##########
@@ -142,8 +143,7 @@ public EnumFieldValue toObject(SchemaField sf, BytesRef term) {
@Override
public String storedToIndexed(IndexableField f) {
final Number val = f.numericValue();
Review Comment:
<picture><img alt="6% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/6/display.svg"></picture>
đŦ 113 similar findings have been found in this PR
---
*[UnnecessaryFinal](https://errorprone.info/bugpattern/UnnecessaryFinal):* Since Java 8, it's been unnecessary to make local variables and parameters `final` for use in lambdas or anonymous classes. Marking them as `final` is weakly discouraged, as it adds a fair amount of noise for minimal benefit.
---
```suggestion
Number val = f.numericValue();
```
---
<details><summary><b>đ Expand here to view all instances of this finding</b></summary><br/>
<div align=\"center\">
| **File Path** | **Line Number** |
| ------------- | ------------- |
| solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java | [243](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java#L243) |
| solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java | [315](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java#L315) |
| solr/core/src/java/org/apache/solr/update/processor/FieldNameMutatingUpdateProcessorFactory.java | [57](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/processor/FieldNameMutatingUpdateProcessorFactory.java#L57) |
| solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java | [219](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java#L219) |
| solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java | [222](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java#L222) |
| solr/core/src/java/org/apache/solr/search/join/HashRangeQuery.java | [143](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/search/join/HashRangeQuery.java#L143) |
| solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java | [241](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java#L241) |
| solr/modules/analytics/src/java/org/apache/solr/analytics/util/FacetRangeGenerator.java | [94](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/analytics/src/java/org/apache/solr/analytics/util/FacetRangeGenerator.java#L94) |
| solr/modules/hdfs/src/java/org/apache/solr/hdfs/HdfsDirectoryFactory.java | [237](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/hdfs/src/java/org/apache/solr/hdfs/HdfsDirectoryFactory.java#L237) |
| solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java | [728](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java#L728) |
<p> Showing <b>10</b> of <b> 113 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?t=ErrorProne|UnnecessaryFinal" 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=371755502&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371755502&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755502&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755502&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371755502&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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083882288
##########
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="46% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/46/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/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371755399&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371755399&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755399&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755399&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371755399&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="36% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/36/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/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371755401&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371755401&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755401&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755401&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371755401&lift_comment_rating=5) ]
##########
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="29% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/29/display.svg"></picture>
*[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>âšī¸ 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=371755970&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371755970&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755970&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371755970&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371755970&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] mkhludnev closed pull request #1307: Small code improvements in facet component
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev closed pull request #1307: Small code improvements in facet component
URL: https://github.com/apache/solr/pull/1307
--
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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083884241
##########
solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java:
##########
@@ -20,41 +20,40 @@
import java.io.IOException;
import java.io.PrintWriter;
import java.io.Writer;
-import java.util.List;
import java.util.ArrayList;
-
+import java.util.List;
+import org.apache.solr.client.solrj.io.Tuple;
import org.apache.solr.client.solrj.io.graph.Traversal;
import org.apache.solr.client.solrj.io.stream.TupleStream;
-import org.apache.solr.client.solrj.io.Tuple;
import org.apache.solr.handler.GraphHandler;
import org.apache.solr.request.SolrQueryRequest;
-
public class GraphMLResponseWriter implements QueryResponseWriter {
+ @Override
public String getContentType(SolrQueryRequest req, SolrQueryResponse res) {
return "application/xml";
}
+ @Override
public void write(Writer writer, SolrQueryRequest req, SolrQueryResponse res) throws IOException {
Exception e1 = res.getException();
- if(e1 != null) {
+ if (e1 != null) {
e1.printStackTrace(new PrintWriter(writer));
return;
}
- TupleStream stream = (TupleStream)req.getContext().get("stream");
+ TupleStream stream = (TupleStream) req.getContext().get("stream");
- if(stream instanceof GraphHandler.DummyErrorStream) {
- GraphHandler.DummyErrorStream d = (GraphHandler.DummyErrorStream)stream;
+ if (stream instanceof GraphHandler.DummyErrorStream) {
+ GraphHandler.DummyErrorStream d = (GraphHandler.DummyErrorStream) stream;
Exception e = d.getException();
e.printStackTrace(new PrintWriter(writer));
Review Comment:
<picture><img alt="0% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/0/display.svg"></picture>
đŦ 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/SearchHandler.java | [589](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/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/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/Tuple.java#L381) |
| solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java | [43](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java#L43) |
| solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java | [910](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java#L910) |
| solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java | [1299](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java#L1299) |
| solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java | [93](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java#L93) |
| solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java | [124](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java#L124) |
| solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java | [74](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java#L74) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756083&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756083&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756083&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756083&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756083&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/rest/ManagedResource.java:
##########
@@ -161,15 +151,13 @@ protected boolean updateInitArgs(NamedList<?> updatedArgs) {
}
boolean madeChanges = false;
if (!managedInitArgs.equals(updatedArgs)) {
Review Comment:
<picture><img alt="6% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/6/display.svg"></picture>
đŦ 395 similar findings have been found in this PR
---
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedResource.updateInitArgs(...)` reads without synchronization from `this.managedInitArgs`. Potentially races with write in method `ManagedResource.reloadFromStorage()`.
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/core/CoreContainer.java | [1235](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/core/CoreContainer.java#L1235) |
| solr/modules/analytics/src/java/org/apache/solr/analytics/AnalyticsRequestManager.java | [250](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/modules/analytics/src/java/org/apache/solr/analytics/AnalyticsRequestManager.java#L250) |
| solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java | [484](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java#L484) |
| solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java | [412](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java#L412) |
| solr/core/src/java/org/apache/solr/schema/IndexSchema.java | [1494](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/schema/IndexSchema.java#L1494) |
| solr/core/src/java/org/apache/solr/update/TransactionLog.java | [275](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/TransactionLog.java#L275) |
| solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymGraphFilterFactory.java | [303](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymGraphFilterFactory.java#L303) |
| solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java | [2150](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java#L2150) |
| solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymFilterFactory.java | [316](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymFilterFactory.java#L316) |
| solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java | [254](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java#L254) |
<p> Showing <b>10</b> of <b> 395 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756302&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756302&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756302&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756302&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756302&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/util/FileUtils.java:
##########
@@ -25,32 +25,26 @@
import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
-
import org.apache.commons.io.FileExistsException;
-/**
- *
- */
+/** */
public class FileUtils {
/**
* Resolves a path relative a base directory.
*
- * <p>
- * This method does what "new File(base,path)" <b>Should</b> do, if it wasn't
- * completely lame: If path is absolute, then a File for that path is returned;
- * if it's not absolute, then a File is returned using "path" as a child
- * of "base")
- * </p>
+ * <p>This method does what "new File(base,path)" <b>Should</b> do, if it wasn't completely lame:
+ * If path is absolute, then a File for that path is returned; if it's not absolute, then a File
+ * is returned using "path" as a child of "base")
*/
public static File resolvePath(File base, String path) {
File r = new File(path);
return r.isAbsolute() ? r : new File(base, path);
}
- public static void copyFile(File src , File destination) throws IOException {
+ public static void copyFile(File src, File destination) throws IOException {
try (FileChannel in = new FileInputStream(src).getChannel();
Review Comment:
<picture><img alt="11% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/11/display.svg"></picture>
đŦ 23 similar findings have been found in this PR
---
*RESOURCE_LEAK:* resource of type `java.io.FileInputStream` acquired by call to `FileInputStream(...)` at line 46 is not released after line 46.
**Note**: potential exception at line 47
---
<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/update/UpdateLog.java | [1402](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L1402) |
| solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessorChain.java | [184](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessorChain.java#L184) |
| solr/core/src/java/org/apache/solr/handler/GraphHandler.java | [124](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/handler/GraphHandler.java#L124) |
| solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java | [218](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java#L218) |
| solr/core/src/java/org/apache/solr/core/SolrCore.java | [2423](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/core/SolrCore.java#L2423) |
| solr/core/src/java/org/apache/solr/update/UpdateLog.java | [1409](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L1409) |
| solr/core/src/java/org/apache/solr/core/PluginBag.java | [148](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/core/PluginBag.java#L148) |
| solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java | [362](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L362) |
| solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java | [125](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java#L125) |
| solr/core/src/java/org/apache/solr/handler/BlobHandler.java | [335](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/handler/BlobHandler.java#L335) |
<p> Showing <b>10</b> of <b> 23 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756313&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756313&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756313&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756313&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756313&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java:
##########
@@ -98,37 +93,55 @@ public void close() throws IOException {
public void uninstall(String packageName, String version) {
SolrPackageInstance packageInstance = getPackageInstance(packageName, version);
if (packageInstance == null) {
- PackageUtils.printRed("Package " + packageName + ":" + version + " doesn't exist. Use the install command to install this package version first.");
+ PackageUtils.printRed(
+ "Package "
+ + packageName
+ + ":"
+ + version
+ + " doesn't exist. Use the install command to install this package version first.");
System.exit(1);
}
// Make sure that this package instance is not deployed on any collection
Map<String, String> collectionsDeployedOn = getDeployedCollections(packageName);
- for (String collection: collectionsDeployedOn.keySet()) {
+ for (String collection : collectionsDeployedOn.keySet()) {
if (version.equals(collectionsDeployedOn.get(collection))) {
- PackageUtils.printRed("Package " + packageName + " is currently deployed on collection: " + collection + ". Undeploy the package with undeploy <package-name> -collections <collection1>[,<collection2>,...] before attempting to uninstall the package.");
+ PackageUtils.printRed(
+ "Package "
+ + packageName
+ + " is currently deployed on collection: "
+ + collection
+ + ". Undeploy the package with undeploy <package-name> -collections <collection1>[,<collection2>,...] before attempting to uninstall the package.");
System.exit(1);
}
}
- // Make sure that no plugin from this package instance has been deployed as cluster level plugins
+ // Make sure that no plugin from this package instance has been deployed as cluster level
+ // plugins
Map<String, SolrPackageInstance> clusterPackages = getPackagesDeployedAsClusterLevelPlugins();
- for (String clusterPackageName: clusterPackages.keySet()) {
+ for (String clusterPackageName : clusterPackages.keySet()) {
SolrPackageInstance clusterPackageInstance = clusterPackages.get(clusterPackageName);
Review Comment:
<picture><img alt="16% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/16/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/packagemanager/PackageManager.java | [253](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L253) |
| solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java | [202](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java#L202) |
| solr/core/src/java/org/apache/solr/util/PackageTool.java | [127](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/PackageTool.java#L127) |
| solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [314](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L314) |
| solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [108](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L108) |
| solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [295](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L295) |
| solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java | [312](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java#L312) |
| solr/core/src/java/org/apache/solr/handler/ClusterAPI.java | [151](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java#L151) |
| solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymFilterFactory.java | [223](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymFilterFactory.java#L223) |
| solr/core/src/java/org/apache/solr/util/PackageTool.java | [143](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/util/PackageTool.java#L143) |
<p> Showing <b>10</b> of <b> 23 </b> findings. <a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756319&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756319&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756319&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756319&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756319&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 #1307: Small code improvements in facet component
Posted by sonatype-lift.
sonatype-lift[bot] commented on code in PR #1307:
URL: https://github.com/apache/solr/pull/1307#discussion_r1083884573
##########
solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java:
##########
@@ -101,35 +110,35 @@ public boolean write(SortDoc sortDoc, LeafReaderContext readerContext, MapWriter
final SortedSetDocValues docVals = vals;
- out.put(this.field,
- (IteratorWriter) w -> {
- long o;
- while((o = docVals.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) {
- BytesRef ref = docVals.lookupOrd(o);
- fieldType.indexedToReadable(ref, cref);
- IndexableField f = fieldType.createField(schemaField, cref.toString());
- if (f == null) w.add(cref.toString());
- else w.add(fieldType.toObject(f));
- }
- });
+ out.put(
+ this.field,
+ (IteratorWriter)
+ w -> {
+ long o;
+ while ((o = docVals.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) {
+ BytesRef ref = docVals.lookupOrd(o);
+ fieldType.indexedToReadable(ref, cref);
+ IndexableField f = fieldType.createField(schemaField, cref.toString());
+ if (f == null) w.add(cref.toString());
+ else w.add(fieldType.toObject(f));
+ }
+ });
return true;
}
-
}
-
static LongFunction<Object> bitsToValue(FieldType fieldType) {
switch (fieldType.getNumberType()) {
Review Comment:
đŦ 3 similar findings have been found in this PR
---
*NULLPTR_DEREFERENCE:* ``NumberType FieldType.getNumberType()`` could be null (from the call to `FieldType.getNumberType()` on line 131) and is dereferenced.
---
<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/search/facet/DocValuesAcc.java | [175](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java#L175) |
| solr/core/src/java/org/apache/solr/search/facet/PercentileAgg.java | [328](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/search/facet/PercentileAgg.java#L328) |
| solr/solrj/src/java/org/apache/solr/client/solrj/routing/NodePreferenceRulesComparator.java | [113](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/solrj/src/java/org/apache/solr/client/solrj/routing/NodePreferenceRulesComparator.java#L113) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?t=Infer|NULLPTR_DEREFERENCE" 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=371756324&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756324&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756324&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756324&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756324&lift_comment_rating=5) ]
##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -747,8 +782,11 @@ private void remoteQuery(String coreUrl, HttpServletResponse resp) throws IOExce
if (httpEntity != null) {
if (httpEntity.getContentEncoding() != null)
- resp.setHeader(httpEntity.getContentEncoding().getName(), httpEntity.getContentEncoding().getValue());
- if (httpEntity.getContentType() != null) resp.setContentType(httpEntity.getContentType().getValue());
+ resp.setHeader(
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.getHeader(...)) at line 749 ~> HTML(HttpServletResponse.setHeader(...)) at line 785.
---
<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 | [785](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L785) |
| solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java | [84](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java#L84) |
| solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java | [545](https://github.com/apache/solr/blob/a33ba6bb6cf2e46fb136f879a6bc5696b8f779d8/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L545) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GQEZBSKB0V8KWDB2MGK5EH8N?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=371756702&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=371756702&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756702&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=371756702&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=371756702&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] mkhludnev commented on pull request #1307: Small code improvements in facet component
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on PR #1307:
URL: https://github.com/apache/solr/pull/1307#issuecomment-1400065983
` Files changed 5,000+`
Not good. Ok maybe I put it wrong, but it's cherry-picking as described in dev docs.
--
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