dsmiley commented on code in PR #2154:
URL: https://github.com/apache/solr/pull/2154#discussion_r1427316295
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1645,6 +1650,34 @@ public Set<Path> getAllowPaths() {
return allowPaths;
}
+ /**
+ * Checks that the given path is not protected (i.e. can accept writes).
Protected paths generally
+ * contain library directories that Solr loads jars and classes from.
+ *
+ * @param pathToAssert path to check
+ * @throws SolrException if path is contained inside a library folder
+ */
+ public void assertPathNotProtected(Path pathToAssert) throws SolrException {
Review Comment:
Please move this to SolrPaths. CoreContainer has a ton of responsibilities;
it doesn't need yet another one.
##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -455,7 +460,32 @@ private void setupSharedLib() {
libDirs.addAll(sharedLibs);
}
- addFoldersToSharedLib(libDirs);
+ List<Path> libDirPaths = new ArrayList<>(libDirs.size());
+ for (String libDir : libDirs) {
+ libDirPaths.add(getSolrHome().resolve(libDir).toAbsolutePath());
+ }
+ return libDirPaths;
+ }
+
+ // Get the list of protected directories, that cannot have files created
within them
+ public List<Path> getProtectedDirectories() {
+ List<Path> protectedDirectories = new ArrayList<>(sharedLibPaths.size() +
4);
+
+ // Start with the shared lib directories
+ protectedDirectories.addAll(sharedLibPaths);
+
+ if (getSolrInstallDir() != null) {
+ // Add in modules and tools
+
protectedDirectories.add(ModuleUtils.getModulesPath(getSolrInstallDir()).toAbsolutePath());
+
protectedDirectories.add(getSolrInstallDir().resolve("prometheus-exporter").toAbsolutePath());
+
+ // Add in server library folders
+ protectedDirectories.add(
+
getSolrInstallDir().resolve("server").resolve("lib").toAbsolutePath());
+ protectedDirectories.add(
+
getSolrInstallDir().resolve("server").resolve("solr-webapp").toAbsolutePath());
+ }
Review Comment:
maybe assert that these actually exist? If one day we change a name or move
something, the assert will help us keep this up to date.
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1645,6 +1650,34 @@ public Set<Path> getAllowPaths() {
return allowPaths;
}
+ /**
+ * Checks that the given path is not protected (i.e. can accept writes).
Protected paths generally
+ * contain library directories that Solr loads jars and classes from.
+ *
+ * @param pathToAssert path to check
+ * @throws SolrException if path is contained inside a library folder
+ */
+ public void assertPathNotProtected(Path pathToAssert) throws SolrException {
+ pathToAssert = pathToAssert.toAbsolutePath();
+ List<Path> protectedPathWithCores =
+ new ArrayList<>(protectedPaths.size() + solrCores.getNumAllCores());
+ protectedPathWithCores.addAll(protectedPaths);
+ solrCores.getCoreDescriptors().stream()
+ .map(CoreDescriptor::getInstanceDir)
+ .map(p -> p.resolve("lib").toAbsolutePath())
+ .forEach(protectedPathWithCores::add);
+
+ for (Path path : protectedPathWithCores) {
Review Comment:
Why the separation here with intermediate "protectedPathWithCores list; why
not end with forEach and do the check?
--
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]
