gerlowskija commented on code in PR #2154:
URL: https://github.com/apache/solr/pull/2154#discussion_r1427948954
##########
solr/core/src/java/org/apache/solr/handler/SnapShooter.java:
##########
@@ -104,6 +104,7 @@ private void initialize(
this.snapshotName = snapshotName;
if ("file".equals(location.getScheme())) {
solrCore.getCoreContainer().assertPathAllowed(Paths.get(location));
+ solrCore.getCoreContainer().assertPathNotProtected(Paths.get(location));
Review Comment:
[0] I worry a bit that folks might forget one or the other of these calls.
Maybe it'd be worth refactoring into a single utility method that optionally
invokes the not-protected logic in the case of writes. e.g.
```
public enum AccessType { READ, WRITE, UNKNOWN; }
public void assertPathAllowed(Path toCheck, AccessType access) {
...
if (access == AccessType.WRITE) {
assertPathNotProtected(...);
}
}
// Note the 'private' here.
private void assertPathNotProtected(Path toCheck) {
}
```
##########
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(
Review Comment:
[0] I'm a little surprised we don't have static constants for these
directories somewhere. Maybe we should...
--
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]
