HoustonPutman commented on code in PR #4193:
URL: https://github.com/apache/solr/pull/4193#discussion_r2954935683
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java:
##########
@@ -345,13 +342,6 @@ public FileVisitResult visitFile(Path file,
BasicFileAttributes attrs)
filenameExclusions);
return FileVisitResult.CONTINUE;
}
- if (isFileForbiddenInConfigSets(filename)) {
Review Comment:
Is this related to
https://github.com/apache/solr/pull/4193/changes#diff-59179ebc496ba57145294b7f952cf8a20df7ec69dd75ca09e59c83f0c558a5c2R175?
If so I understand that this helps us with other configSetService
implementations, and I can get behind that.
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java:
##########
@@ -443,13 +433,9 @@ public static void downloadFromZK(SolrZkClient zkClient,
String zkPath, Path fil
if (children.size() == 0) {
// If we didn't copy data down, then we also didn't create the file.
But we still need a
// marker on the local disk so create an empty file.
- if (isFileForbiddenInConfigSets(zkPath)) {
Review Comment:
I get your point, but I do think it's good to err on the side of caution,
and this also generally just prevents us from having RCE issues with injecting
code in ZK. And in general, I think it's ok to say we never want to download
these kinds of files from ZK. I'd leave this code in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
