Skip to content

Commit

Permalink
XWIKI-22741: Cache path for solr embedded is not properly escaped on …
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@jameswartell
jameswartell authored Jan 22, 2025
1 parent ef97831 commit 2e4f1f7
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 41 deletions.
57 changes: 51 additions & 6 deletions ...i-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/EmbeddedSolr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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());
}
}
Expand All @@ -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)
Expand Down
154 changes: 119 additions & 35 deletions ...solr-api/src/test/java/org/xwiki/search/solr/internal/EmbeddedSolrInitializationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
*
Expand Down Expand Up @@ -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"), "<solr/>", 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();
Expand All @@ -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());

Expand All @@ -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"), "<solr/>", 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");
Expand All @@ -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") + "\"!");
}
}

0 comments on commit 2e4f1f7

Please sign in to comment.