Skip to content

Small code improvements in facet component#1307

Closed
renatoh wants to merge 879 commits intoapache:branch_9xfrom
renatoh:small_code_improvements_in_FacetComponent
Closed

Small code improvements in facet component#1307
renatoh wants to merge 879 commits intoapache:branch_9xfrom
renatoh:small_code_improvements_in_FacetComponent

Conversation

@renatoh
Copy link
Copy Markdown
Contributor

@renatoh renatoh commented Jan 23, 2023

backporting "Debugging FacetComponent I noticed two little things which can be improved"

risdenk and others added 30 commits October 20, 2022 00:44
* Simplify assert statements
* remove unused imports
* Address remaining assert simplifications
* Fix bad replacements in TestSQLHandler
* Fix SolrInputDocumentReaderTest setup
* Fix redundant casts in SpellCheckComponentTest
* Fix redundant cast in ArrayEvaluatorTest
* Fix cast in SchemaTest
* Simplify casts in stream/sql tests and fix asserts
* Simplify raw assert in tests

Co-authored-by: Kevin Risden <krisden@apache.org>
Previously added in support of a feature that has since been deprecated
and removed, this internal API can be ripped out without fear of
backcompat violation.
renatoh and others added 5 commits January 22, 2023 19:17
Co-authored-by: Renato Haeberli <>
* setting initial length for facetQueries ArrayList

* Using isNotEmpty-method rather than negating isEmpty

Co-authored-by: Renato Haeberli <>
@mkhludnev
Copy link
Copy Markdown
Member

Files changed 5,000+
Not good. Ok maybe I put it wrong, but it's cherry-picking as described in dev docs.

@mkhludnev mkhludnev closed this Jan 23, 2023
Comment thread solr/core/build.gradle
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'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

46% of developers fix this issue

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 for details about these vulnerabilities.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Comment thread solr/core/build.gradle
// Collections & lang utilities
implementation ('com.google.guava:guava') { transitive = false }
implementation 'com.google.guava:guava'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

36% of developers fix this issue

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 for details about these vulnerabilities.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

/**
* 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) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

29% of developers fix this issue

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.


Suggested change
public Type getUninversionType(SchemaField sf) {
public UninvertingReader.Type getUninversionType(SchemaField sf) {

ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

private interface RestoreRepository {
String[] listAllFiles() throws IOException;

IndexInput openInput(String filename) throws IOException;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

24% of developers fix this issue

UnusedMethod: Method 'openInput' is never used.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@Override
protected MessageDigest initialValue() {
try {
return MessageDigest.getInstance("MD5");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WEAK_MESSAGE_DIGEST_MD5: This API MD5 (MDX) is not a recommended cryptographic hash function


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UNENCRYPTED_SOCKET: Unencrypted socket to org.apache.solr.handler.admin.ZookeeperStatusHandler (instead of SSLSocket)


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

throws ServletException,IOException {
throws ServletException, IOException {

res.setStatus(code);
res.setHeader("Location", destination);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UNVALIDATED_REDIRECT: The following redirection could be used by an attacker to redirect users to a phishing website.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

*/
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");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WEAK_MESSAGE_DIGEST_SHA1: This API SHA1 (SHA-1) is not a recommended cryptographic hash function


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -142,8 +143,7 @@ public EnumFieldValue toObject(SchemaField sf, BytesRef term) {
@Override
public String storedToIndexed(IndexableField f) {
final Number val = f.numericValue();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6% of developers fix this issue

💬 113 similar findings have been found in this PR


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.


Suggested change
final Number val = f.numericValue();
Number val = f.numericValue();

🔎 Expand here to view all instances of this finding
File Path Line Number
solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java 243
solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java 315
solr/core/src/java/org/apache/solr/update/processor/FieldNameMutatingUpdateProcessorFactory.java 57
solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java 219
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java 222
solr/core/src/java/org/apache/solr/search/join/HashRangeQuery.java 143
solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java 241
solr/modules/analytics/src/java/org/apache/solr/analytics/util/FacetRangeGenerator.java 94
solr/modules/hdfs/src/java/org/apache/solr/hdfs/HdfsDirectoryFactory.java 237
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java 728

Showing 10 of 113 findings. Visit the Lift Web Console to see all.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

}

/**
*
* @param forceFetch get a fresh value and not cached value
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

45% of developers fix this issue

💬 6 similar findings have been found in this PR


MissingSummary: A summary line is required on public/protected Javadocs.


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java 60
solr/core/src/java/org/apache/solr/request/LocalSolrQueryRequest.java 34
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/cloud/AlreadyExistsException.java 20
solr/solrj/src/java/org/apache/solr/common/AlreadyClosedException.java 20
solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java 26
solr/modules/clustering/src/java/org/apache/solr/handler/clustering/ClusteringComponent.java 499

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

}
}
}
}

if (managedResource == null) {
final String method = getSolrRequest().getHttpMethod();
if ("PUT".equals(method) || "POST".equals(method)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 9 similar findings have been found in this PR


UNSAFE_HASH_EQUALS: Unsafe comparison of hash that are susceptible to timing attack


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/search/facet/FacetModule.java 216
solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java 1398
solr/core/src/java/org/apache/solr/core/BlobRepository.java 187
solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java 129
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java 300
solr/solrj/src/java/org/apache/solr/common/params/ShardParams.java 118
solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java 204
solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java 170
solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java 260

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -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(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

15% of developers fix this issue

💬 9 similar findings have been found in this PR


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


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/update/processor/RegexpBoostProcessor.java 146
solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java 102
solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java 335
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/KerberosFilter.java 98
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java 227
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java 237
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/HadoopAuthPlugin.java 228
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java 221
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java 249

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

*/
public static Document parseConfigXML(Logger log, ResourceLoader loader, InputSource is)
throws SAXException, IOException {
return configDocumentBuilder(loader, log).parse(is);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 9 similar findings have been found in this PR


XXE_DOCUMENT: The use of DocumentBuilder.parse(...) (DocumentBuilder) is vulnerable to XML External Entity attacks


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java 109
solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java 82
solr/test-framework/src/java/org/apache/solr/util/DOMUtilTestBase.java 51
solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java 62
solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsConfiguration.java 98
solr/core/src/java/org/apache/solr/util/SimplePostTool.java 1175
solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java 108
solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java 125
solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsConfiguration.java 105

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

}

@Override
public OutputStream openOutputStream(String storedResourceId) throws IOException {
return new FileOutputStream(storageDir+"/"+storedResourceId);
return new FileOutputStream(storageDir + "/" + storedResourceId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 3 similar findings have been found in this PR


PATH_TRAVERSAL_OUT: This API (java/io/FileOutputStream.(Ljava/lang/String;)V) writes to a file whose location might be specified by user input


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java 67
solr/core/src/java/org/apache/solr/util/ExportTool.java 253
solr/core/src/java/org/apache/solr/util/ExportTool.java 327

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -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();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

13% of developers fix this issue

💬 8 similar findings have been found in this PR


URLCONNECTION_SSRF_FD: This web server request could be used by an attacker to expose internal services and filesystem.


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/util/CryptoKeys.java 278
solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java 150
solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java 132
solr/core/src/java/org/apache/solr/util/SimplePostTool.java 999
solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java 41
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java 465
solr/core/src/java/org/apache/solr/util/SimplePostTool.java 975
solr/test-framework/src/java/org/apache/solr/handler/BackupRestoreUtils.java 116

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

// 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*[+-]+)+");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 4 similar findings have been found in this PR


REDOS: The regular expression "\s++-+" is vulnerable to a denial of service attack (ReDOS)


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsQueryTemplate.java 41
solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java 428
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkDynamicConfig.java 39
solr/core/src/java/org/apache/solr/handler/admin/SolrEnvironment.java 36

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -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(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 4 similar findings have been found in this PR


XSS_SERVLET: This use of javax/servlet/http/HttpServletResponse.sendError(ILjava/lang/String;)V could be vulnerable to XSS in the Servlet


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java 854
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java 204
solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java 890
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java 818

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

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));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0% of developers fix this issue

💬 8 similar findings have been found in this PR


INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE: Possible information exposure through an error message


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java 589
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/Tuple.java 381
solr/core/src/java/org/apache/solr/response/GraphMLResponseWriter.java 43
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java 910
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java 1299
solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java 93
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java 124
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java 74

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -161,15 +151,13 @@ protected boolean updateInitArgs(NamedList<?> updatedArgs) {
}
boolean madeChanges = false;
if (!managedInitArgs.equals(updatedArgs)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6% of developers fix this issue

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


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/core/CoreContainer.java 1235
solr/modules/analytics/src/java/org/apache/solr/analytics/AnalyticsRequestManager.java 250
solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java 484
solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java 412
solr/core/src/java/org/apache/solr/schema/IndexSchema.java 1494
solr/core/src/java/org/apache/solr/update/TransactionLog.java 275
solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymGraphFilterFactory.java 303
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java 2150
solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymFilterFactory.java 316
solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java 254

Showing 10 of 395 findings. Visit the Lift Web Console to see all.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

*/
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();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11% of developers fix this issue

💬 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


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/update/UpdateLog.java 1402
solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessorChain.java 184
solr/core/src/java/org/apache/solr/handler/GraphHandler.java 124
solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java 218
solr/core/src/java/org/apache/solr/core/SolrCore.java 2423
solr/core/src/java/org/apache/solr/update/UpdateLog.java 1409
solr/core/src/java/org/apache/solr/core/PluginBag.java 148
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java 362
solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java 125
solr/core/src/java/org/apache/solr/handler/BlobHandler.java 335

Showing 10 of 23 findings. Visit the Lift Web Console to see all.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Map<String, SolrPackageInstance> clusterPackages = getPackagesDeployedAsClusterLevelPlugins();
for (String clusterPackageName: clusterPackages.keySet()) {
for (String clusterPackageName : clusterPackages.keySet()) {
SolrPackageInstance clusterPackageInstance = clusterPackages.get(clusterPackageName);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

16% of developers fix this issue

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


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java 253
solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java 202
solr/core/src/java/org/apache/solr/util/PackageTool.java 127
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java 314
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java 108
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java 295
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java 312
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java 151
solr/core/src/java/org/apache/solr/rest/schema/analysis/ManagedSynonymFilterFactory.java 223
solr/core/src/java/org/apache/solr/util/PackageTool.java 143

Showing 10 of 23 findings. Visit the Lift Web Console to see all.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

}


static LongFunction<Object> bitsToValue(FieldType fieldType) {
switch (fieldType.getNumberType()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java 175
solr/core/src/java/org/apache/solr/search/facet/PercentileAgg.java 328
solr/solrj/src/java/org/apache/solr/client/solrj/routing/NodePreferenceRulesComparator.java 113

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -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(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0% of developers fix this issue

💬 3 similar findings have been found in this PR


CROSS_SITE_SCRIPTING: UserControlledString(HttpServletRequest.getHeader(...)) at line 749 ~> HTML(HttpServletResponse.setHeader(...)) at line 785.


🔎 Expand here to view all instances of this finding
File Path Line Number
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java 785
solr/core/src/java/org/apache/solr/security/AuthorizationUtils.java 84
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java 545

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

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.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.