Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable-16.10.x] XWIKI-22741: Cache path for solr embedded is not properly escaped on Windows #3824

Merged
merged 1 commit into from
Jan 22, 2025
Merged

[stable-16.10.x] XWIKI-22741: Cache path for solr embedded is not properly escaped on Windows #3824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 51 additions & 6 deletions ...i-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/EmbeddedSolr.java
View file
Open in desktop
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
Open in desktop
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") + "\"!");
}
}
Loading