From 2e4f1f7d060729803257c841fe93e8cf78d7e515 Mon Sep 17 00:00:00 2001
From: jameswartell <33641420+jameswartell@users.noreply.github.com>
Date: Wed, 22 Jan 2025 08:33:59 -0600
Subject: [PATCH] XWIKI-22741: Cache path for solr embedded is not properly
escaped on Windows
* Fix writing properties file in improper format, causing '\' to not be properly escaped. Also added regression test.
* Clean up old cache files in wrong place.
---
.../search/solr/internal/EmbeddedSolr.java | 57 ++++++-
.../EmbeddedSolrInitializationTest.java | 154 ++++++++++++++----
2 files changed, 170 insertions(+), 41 deletions(-)
diff --git a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/EmbeddedSolr.java b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/EmbeddedSolr.java
index eaddb2e19a2e..aaab1b940f01 100644
--- a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/EmbeddedSolr.java
+++ b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/EmbeddedSolr.java
@@ -20,6 +20,8 @@
package org.xwiki.search.solr.internal;
import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStream;
@@ -29,6 +31,7 @@
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
+import java.util.Properties;
import java.util.Set;
import java.util.stream.Stream;
import java.util.zip.ZipEntry;
@@ -89,6 +92,10 @@ public class EmbeddedSolr extends AbstractSolr implements Disposable, Initializa
private static final long SEARCH_CORE_SCHEMA_VERSION = AbstractSolrCoreInitializer.SCHEMA_VERSION_16_6;
+ private static final String CORE_PROPERTIES_FILENAME = "core.properties";
+
+ private static final String DATA_DIR_PROPERTY = "dataDir";
+
/**
* Solr configuration.
*/
@@ -126,6 +133,7 @@ public void initialize() throws InitializationException
// Validate and create the search core
if (Files.exists(this.solrSearchCorePath)) {
+
// Make sure the core setup is up to date
if (!isSearchCoreValid()) {
// Recreate the home folder
@@ -311,8 +319,35 @@ private long getCoreVersion(File schemaFile)
return -1;
}
- private boolean isSearchCoreValid()
+ private boolean isSearchCoreValid() throws IOException
{
+
+ // Due to https://jira.xwiki.org/browse/XWIKI-22741 the cache on windows was
+ // being put in the wrong place. Let's check and clean it up if we find it
+ {
+
+ File corePropertiesFile = getCacheCorePropertiesFile(this.solrSearchCorePath);
+ if (corePropertiesFile.exists()) {
+ Properties properties = new Properties();
+ try (FileInputStream in = new FileInputStream(corePropertiesFile)) {
+ properties.load(in);
+ }
+
+ String dataDirPropertyValue = properties.getProperty(DATA_DIR_PROPERTY);
+ if (dataDirPropertyValue != null && !dataDirPropertyValue.contains(File.separator)) {
+ // it's bugged
+ this.logger.info("Found XWIKI-22741 bug!");
+ File badCacheLocation = new File(corePropertiesFile.getParent(), dataDirPropertyValue);
+ if (badCacheLocation.exists()) {
+ this.logger
+ .info("Removing old Solr Search Cache files from: " + badCacheLocation.getAbsolutePath());
+ FileUtils.deleteDirectory(badCacheLocation);
+ }
+ return false;
+ }
+ }
+ }
+
// Exists but is unusable.
if (!Files.isDirectory(this.solrSearchCorePath) || !Files.isWritable(this.solrSearchCorePath)
|| !Files.isReadable(this.solrSearchCorePath)) {
@@ -443,7 +478,7 @@ private void copyCoreConfiguration(InputStream stream, Path corePath, boolean sk
if (entry.isDirectory()) {
Files.createDirectories(targetPath);
} else if ((force != null && force.contains(entry.getName())) || (!Files.exists(targetPath)
- && (!skipCoreProperties || !entry.getName().equals("core.properties")))) {
+ && (!skipCoreProperties || !entry.getName().equals(CORE_PROPERTIES_FILENAME)))) {
FileUtils.copyInputStreamToFile(CloseShieldInputStream.wrap(zstream), targetPath.toFile());
}
}
@@ -460,12 +495,22 @@ private void createSearchCore() throws IOException
createCacheCore(this.solrSearchCorePath, toSolrCoreName(SolrClientInstance.CORE_NAME));
}
+ private File getCacheCorePropertiesFile(Path corePath)
+ {
+ File corePropertiesFile = corePath.resolve(CORE_PROPERTIES_FILENAME).toFile();
+ return corePropertiesFile;
+ }
+
private void createCacheCore(Path corePath, String solrCoreName) throws IOException
{
- // Indicate the path of the data
- Path dataPath = getCacheCoreDataDir(corePath, solrCoreName);
- FileUtils.write(corePath.resolve("core.properties").toFile(), "dataDir=" + dataPath, StandardCharsets.UTF_8,
- true);
+ Path dataDir = getCacheCoreDataDir(corePath, solrCoreName);
+ File corePropertiesFile = getCacheCorePropertiesFile(corePath);
+ Properties coreProperties = new Properties();
+ coreProperties.setProperty(DATA_DIR_PROPERTY, dataDir.toString());
+ // we recreate this file every time this gets called.
+ try (FileOutputStream out = new FileOutputStream(corePropertiesFile, false);) {
+ coreProperties.store(out, "");
+ }
}
private Path getCacheCoreDataDir(Path corePath, String solrCoreName)
diff --git a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/test/java/org/xwiki/search/solr/internal/EmbeddedSolrInitializationTest.java b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/test/java/org/xwiki/search/solr/internal/EmbeddedSolrInitializationTest.java
index d0f7f0c138b4..dc47d8a85dd4 100644
--- a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/test/java/org/xwiki/search/solr/internal/EmbeddedSolrInitializationTest.java
+++ b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/test/java/org/xwiki/search/solr/internal/EmbeddedSolrInitializationTest.java
@@ -19,10 +19,23 @@
*/
package org.xwiki.search.solr.internal;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.when;
+
+import java.io.BufferedReader;
import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileReader;
import java.io.InputStream;
+import java.io.PrintWriter;
import java.nio.charset.StandardCharsets;
import java.util.List;
+import java.util.Properties;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
@@ -47,14 +60,6 @@
import org.xwiki.test.junit5.mockito.InjectComponentManager;
import org.xwiki.test.mockito.MockitoComponentManager;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.when;
-
/**
* Test the initialization of the Solr instance.
*
@@ -126,17 +131,11 @@ private void getInstanceAndAssertHomeDirectory(String expected) throws Component
assertTrue(new File(configDirectory, core.getConfigResource()).exists());
}
- // Tests
-
- @Test
- void testInitializationWhenValid() throws Exception
+ // returns the search core directory
+ private File useStandardSolrHome(File solrHomeDirectory) throws Exception
{
- // Create the Solr home
- File solrHomeDirectory = new File(this.permanentDirectory, "store/solr");
solrHomeDirectory.mkdirs();
FileUtils.write(new File(solrHomeDirectory, "solr.xml"), "", StandardCharsets.UTF_8);
-
- // Unzip the standard Solr home
File solrSearchCoreDirectory = new File(solrHomeDirectory, SEARCH_SOLRCORE);
SolrConfiguration solrConfiguration = this.componentManager.getInstance(SolrConfiguration.class);
InputStream stream = solrConfiguration.getSearchCoreDefaultContent();
@@ -151,6 +150,16 @@ void testInitializationWhenValid() throws Exception
}
}
}
+ return solrSearchCoreDirectory;
+ }
+
+ // Tests
+
+ @Test
+ void testInitializationWhenValid() throws Exception
+ {
+ File solrHomeDirectory = new File(this.permanentDirectory, "store/solr");
+ File solrSearchCoreDirectory = useStandardSolrHome(solrHomeDirectory);
getInstanceAndAssertHomeDirectory(solrHomeDirectory.toString());
@@ -167,26 +176,8 @@ void testInitializationWhenValid() throws Exception
@Test
void testInitializationWhenInvalid() throws Exception
{
- // Create the Solr home
File solrHomeDirectory = new File(this.permanentDirectory, "store/solr");
- solrHomeDirectory.mkdirs();
- FileUtils.write(new File(solrHomeDirectory, "solr.xml"), "", StandardCharsets.UTF_8);
-
- // Unzip the standard Solr home
- File solrSearchCoreDirectory = new File(solrHomeDirectory, SEARCH_SOLRCORE);
- SolrConfiguration solrConfiguration = this.componentManager.getInstance(SolrConfiguration.class);
- InputStream stream = solrConfiguration.getSearchCoreDefaultContent();
- try (ZipInputStream zstream = new ZipInputStream(stream)) {
- for (ZipEntry entry = zstream.getNextEntry(); entry != null; entry = zstream.getNextEntry()) {
- if (entry.isDirectory()) {
- File destinationDirectory = new File(solrSearchCoreDirectory, entry.getName());
- destinationDirectory.mkdirs();
- } else {
- File destinationFile = new File(solrSearchCoreDirectory, entry.getName());
- FileUtils.copyInputStreamToFile(CloseShieldInputStream.wrap(zstream), destinationFile);
- }
- }
- }
+ File solrSearchCoreDirectory = useStandardSolrHome(solrHomeDirectory);
// Make the configuration too old
File solrconfigFile = new File(solrSearchCoreDirectory, "conf/solrconfig.xml");
@@ -209,4 +200,97 @@ void testInstantiationWhenDoesNotExist() throws Exception
getInstanceAndAssertHomeDirectory(newHome);
}
+
+ /**
+ * This looks for a regression where the file.seperator on windows was not properly escaped. in the
+ * ""xwiki\store\solr\search\core.properties" file.
+ */
+ @Test
+ void testCacheHomeProperlyEscapedOnWindows() throws Exception
+ {
+ String newHome = new File(this.permanentDirectory, "doesNotExist").getAbsolutePath();
+ when(this.mockXWikiProperties.getProperty(eq(SOLRHOME_PROPERTY), anyString())).thenReturn(newHome);
+
+ Solr instance = this.componentManager.getInstance(Solr.class, EmbeddedSolr.TYPE);
+ EmbeddedSolr implementation = ((EmbeddedSolr) instance);
+ CoreContainer container = implementation.getContainer();
+ SolrCore core = container.getCore(container.getLoadedCoreNames().get(0));
+ File coreBaseDirectory = new File(container.getSolrHome(), core.getName());
+
+ File file = new File(coreBaseDirectory, "core.properties");
+ assertTrue(file.exists(), "Couldn't find solr cache properties file: " + file.toString());
+ Properties properties = new Properties();
+ try (BufferedReader in = new BufferedReader(new FileReader(file, StandardCharsets.UTF_8));) {
+ properties.load(in);
+ }
+ String dataDir = properties.getProperty("dataDir");
+ assertNotNull(dataDir, "dataDir property from properties file was null: " + file.toString());
+ assertTrue(dataDir.contains(File.separator), "File seperators were not escaped properly in the "
+ + "cache path!: \"" + dataDir + "\" does not contain '" + File.separator + "'!");
+
+ }
+
+ /**
+ * Check that the cleanup for XWIKI-22741 deletes the cache directory in the wrong location during initialization.
+ * When this happens the search core should also get created fixing the core.properties value.
+ */
+ @Test
+ void testRemoveBadCacheLocation() throws Exception
+ {
+
+ File solrHomeDirectory = new File(this.permanentDirectory, "store/solr");
+ File solrSearchCoreDirectory = useStandardSolrHome(solrHomeDirectory);
+ File corePropertiesFile = new File(solrSearchCoreDirectory, "core.properties");
+ assertTrue(corePropertiesFile.exists(),
+ "Core properties file is missing: " + corePropertiesFile.getAbsolutePath());
+
+ // now simulate the bug even if we aren't on windows
+ String propertyValue = "..\\..\\..\\cache\\solr\\" + SEARCH_SOLRCORE;
+ String badCacheFolderPath = "......cachesolr" + SEARCH_SOLRCORE;
+ String goodCacheFolderPath = ".." + File.separator + ".." + File.separator + ".." + File.separator + "cache"
+ + File.separator + "solr" + File.separator + SEARCH_SOLRCORE;
+
+ // when you write the property like this without the Properties.store() method the '\\' becomes an
+ // escape on windows when read properly
+ try (PrintWriter out = new PrintWriter(corePropertiesFile)) {
+ out.println("dataDir=" + propertyValue);
+ }
+
+ File goodCacheLocation = new File(solrSearchCoreDirectory, propertyValue);
+ File badCacheLocation = new File(solrSearchCoreDirectory, badCacheFolderPath);
+
+ // now let's simulate some cache files in the wrong location.
+ File badCacheFolder = new File(badCacheLocation, "childFolder");
+ {
+ assertTrue(badCacheFolder.mkdirs());
+ File badCacheFile1 = new File(badCacheLocation, "testFile1.txt");
+ try (PrintWriter out = new PrintWriter(badCacheFile1)) {
+ out.println("whatever");
+ }
+ File badCacheFile2 = new File(badCacheFolder, "testFile2.txt");
+ try (PrintWriter out = new PrintWriter(badCacheFile2)) {
+ out.println("whatever");
+ }
+ assertTrue(badCacheFile1.exists());
+ assertTrue(badCacheFile2.exists());
+ }
+
+ getInstanceAndAssertHomeDirectory(solrHomeDirectory.toString());
+
+ // we created cache files in the wrong location but a new cache should have have gotten reinitialized at the
+ // right location.
+ assertTrue(goodCacheLocation.exists(),
+ "Expected to find cache at the right location but did not: " + goodCacheLocation.getAbsoluteFile());
+ // the old bad location should be cleaned up
+ assertTrue(!badCacheLocation.exists(),
+ "Expected not to find a cache at the bad location (yet) but did: " + badCacheLocation.getAbsolutePath());
+
+ Properties coreProperties = new Properties();
+ try (FileInputStream in = new FileInputStream(corePropertiesFile);) {
+ coreProperties.load(in);
+ }
+ // we set the dataDir property to the bad value, but it should be fixed.
+ assertEquals(coreProperties.get("dataDir"), goodCacheFolderPath, "Cache location should be: \""
+ + goodCacheFolderPath + "\" but was \"" + coreProperties.getProperty("dataDir") + "\"!");
+ }
}