You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/03 14:17:45 UTC
[GitHub] [solr] madrob commented on a change in pull request #593: SOLR-15950 Create SOLR_HOME/{filestore,userfiles} lazily on first use
madrob commented on a change in pull request #593:
URL: https://github.com/apache/solr/pull/593#discussion_r798606819
##########
File path: solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
##########
@@ -94,8 +94,18 @@ private static Path _getRealPath(String path, Path solrHome) {
if (!path.isEmpty() && path.charAt(0) != File.separatorChar) {
path = File.separator + path;
}
- // Use concat because path might start with a slash and be incorrectly interpreted as absolute
- return solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY + path);
+ if (path.startsWith("\\\\")) { // Windows absolute UNC
+ throw new SolrException(BAD_REQUEST, "Illegal path " + path);
+ }
+ while (path.startsWith("/")) { // Trim all leading slashes
Review comment:
This is odd because at line 94 we just added leading slashes?
##########
File path: solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
##########
@@ -572,7 +582,16 @@ private void ensurePackageStoreDir(Path solrHome) {
}
public static Path getPackageStoreDirPath(Path solrHome) {
- return solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY);
+ var path = solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY);
+ if (!Files.exists(path)) {
+ try {
+ Files.createDirectories(path);
+ log.info("Created filestore folder {}", path);
+ } catch (IOException e) {
+ throw new SolrException(SERVER_ERROR, "Faild creating 'filestore' folder in SOLR_HOME. Check permissions");
Review comment:
Failed
##########
File path: solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
##########
@@ -572,7 +582,16 @@ private void ensurePackageStoreDir(Path solrHome) {
}
public static Path getPackageStoreDirPath(Path solrHome) {
- return solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY);
+ var path = solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY);
+ if (!Files.exists(path)) {
+ try {
+ Files.createDirectories(path);
+ log.info("Created filestore folder {}", path);
+ } catch (IOException e) {
+ throw new SolrException(SERVER_ERROR, "Faild creating 'filestore' folder in SOLR_HOME. Check permissions");
Review comment:
Should we set the Ioexception as cause?
--
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