SOLR-18178: Fix configset archive path handling

changelog/unreleased/SOLR-18178.yml

@@ -0,0 +1,7 @@
+title: Write portable ZIP entry names when exporting configsets, normalize legacy Windows entries on upload, and skip unsafe entries on upload.
+type: fixed
+authors:
+  - name: Nick Shanin
+links:
+  - name: SOLR-18178
+    url: https://issues.apache.org/jira/browse/SOLR-18178

solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java

@@ -309,7 +309,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException ioException) {
             if (!relativePath.isEmpty()) {
               // We always want to have a trailing forward slash on a directory to
               // match the normalization to forward slashes everywhere.
-              filePaths.add(relativePath + '/');
+              filePaths.add(normalizePathToForwardSlash(relativePath) + '/');
             }
             return FileVisitResult.CONTINUE;
           }

solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java

@@ -100,11 +100,8 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
                 if (Files.isHidden(dir)) {
                   return FileVisitResult.SKIP_SUBTREE;
                 }
-                String dirName = tmpDirectory.relativize(dir).toString();
-                if (!dirName.isEmpty()) {
-                  if (!dirName.endsWith("/")) {
-                    dirName += "/";
-                  }
+                String dirName = toZipEntryName(tmpDirectory, dir, true);
+                if (dirName != null) {
                   zipOut.putNextEntry(new ZipEntry(dirName));
                   zipOut.closeEntry();
                 }
@@ -115,10 +112,12 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
               public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
                   throws IOException {
                 if (!Files.isHidden(file)) {
-                  try (InputStream fis = Files.newInputStream(file)) {
-                    ZipEntry zipEntry = new ZipEntry(tmpDirectory.relativize(file).toString());
-                    zipOut.putNextEntry(zipEntry);
-                    fis.transferTo(zipOut);
+                  String entryName = toZipEntryName(tmpDirectory, file, false);
+                  if (entryName != null) {
+                    try (InputStream fis = Files.newInputStream(file)) {
+                      zipOut.putNextEntry(new ZipEntry(entryName));
+                      fis.transferTo(zipOut);
+                    }
                   }
                 }
                 return FileVisitResult.CONTINUE;
@@ -130,4 +129,21 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
     }
     return baos.toByteArray();
   }
+
+  /**
+   * Convert a path under {@code root} into a ZIP-legal relative entry name.
+   *
+   * <p>ZIP requires {@code /} separators and forbids a leading slash. Returns {@code null} for the
+   * root itself so callers skip the nameless {@code /} directory entry.
+   */
+  static String toZipEntryName(Path root, Path path, boolean directory) {
+    String name = root.relativize(path).toString().replace('\\', '/');
+    if (name.isEmpty()) {
+      return null;
+    }
+    if (directory && !name.endsWith("/")) {
+      name += "/";
+    }
+    return name;
+  }
 }

solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java

@@ -102,9 +102,21 @@ public SolrJerseyResponse uploadConfigSet(
         while (entries.hasMoreElements()) {
           ZipEntry zipEntry = entries.nextElement();
           hasEntry = true;
-          String filePath = zipEntry.getName();
+          String filePath = normalizeZipEntryName(zipEntry.getName());
           filesToDelete.remove(filePath);
-          if (!zipEntry.isDirectory()) {
+          // Backslashes are invalid as ZIP separators, but older Windows-produced archives may
+          // contain them. Normalize before handing the path to either config-set implementation.
+          if (!isSafeZipEntryPath(filePath)) {
+            // The filesystem backend contains such entries on its own, but the ZooKeeper
+            // backend builds a znode path from the entry name and the upload aborts. Reject
+            // unsafe entry paths here, before either backend sees them.
+            log.warn(
+                "Not uploading file [{}] from the uploaded zip, as its path could resolve"
+                    + " outside of the configset root directory",
+                filePath);
+            continue;
+          }
+          if (!zipEntry.isDirectory() && !filePath.endsWith("/")) {
             try (InputStream entryStream = zipFile.getInputStream(zipEntry)) {
               configSetService.uploadFileToConfig(
                   configSetName, filePath, entryStream.readAllBytes(), true);
@@ -130,6 +142,37 @@ public SolrJerseyResponse uploadConfigSet(
     return response;
   }
 
+  static String normalizeZipEntryName(String entryName) {
+    return entryName.replace('\\', '/');
+  }
+
+  /**
+   * Whether a normalized zip entry path stays inside the configset it is uploaded to. Absolute
+   * paths, drive-qualified Windows paths, empty names, and paths with "." or ".." segments are
+   * unsafe: the filesystem backend would resolve them outside of the configset directory, and the
+   * ZooKeeper backend cannot use them as znode path segments at all.
+   */
+  static boolean isSafeZipEntryPath(String normalizedPath) {
+    if (normalizedPath.isEmpty() || normalizedPath.startsWith("/")) {
+      return false;
+    }
+    // A drive-qualified name such as "C:/..." does not start with "/" but is still
+    // absolute on Windows. The check must not depend on the host platform, and a colon
+    // cannot appear in a Windows file name at all, so reject any leading drive
+    // designator here.
+    if (normalizedPath.length() >= 2
+        && Character.isLetter(normalizedPath.charAt(0))
+        && normalizedPath.charAt(1) == ':') {
+      return false;
+    }
+    for (String segment : normalizedPath.split("/")) {
+      if (segment.equals(".") || segment.equals("..")) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   @Override
   @PermissionName(CONFIG_EDIT_PERM)
   public SolrJerseyResponse uploadConfigSetFile(

solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java

@@ -132,6 +132,26 @@ public void testUploadAndDeleteConfig() throws IOException {
     assertFalse(fileSystemConfigSetService.checkConfigExists("copytestconfig"));
   }
 
+  @Test
+  public void testGetAllConfigFilesUsesForwardSlashesForDirectories() throws IOException {
+    String configName = "nestedconfig";
+    fileSystemConfigSetService.uploadFileToConfig(
+        configName, "lang/stopwords_en.txt", "a\nthe".getBytes(StandardCharsets.UTF_8), true);
+
+    assertEquals(
+        List.of("lang/", "lang/stopwords_en.txt"),
+        fileSystemConfigSetService.getAllConfigFiles(configName));
+
+    // A single directory name contains no separator, so the case above cannot show the
+    // conversion; a nested directory is what produces a separator inside the returned name.
+    fileSystemConfigSetService.uploadFileToConfig(
+        configName, "lang/nested/deep.txt", "deep".getBytes(StandardCharsets.UTF_8), true);
+
+    assertEquals(
+        List.of("lang/", "lang/nested/", "lang/nested/deep.txt", "lang/stopwords_en.txt"),
+        fileSystemConfigSetService.getAllConfigFiles(configName));
+  }
+
   private static List<String> getFileList(Path confDir) throws IOException {
     try (Stream<Path> configs = Files.list(confDir)) {
       return configs

solr/core/src/test/org/apache/solr/handler/configsets/DownloadConfigSetAPITest.java

@@ -22,9 +22,12 @@
 import static org.mockito.Mockito.when;
 
 import jakarta.ws.rs.core.Response;
+import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
 import org.apache.solr.SolrTestCase;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.core.CoreContainer;
@@ -92,4 +95,54 @@ public void testSuccessfulDownloadReturnsZipResponse() throws Exception {
       assertNull(response.getHeaderString("Content-Disposition"));
     }
   }
+
+  @Test
+  public void testZipConfigSetUsesForwardSlashEntryNames() throws Exception {
+    Path configDir = configSetBase.resolve("nestedconfig");
+    Files.createDirectories(configDir.resolve("lang"));
+    Files.writeString(configDir.resolve("solrconfig.xml"), "<config/>", StandardCharsets.UTF_8);
+    Files.writeString(
+        configDir.resolve("lang").resolve("stopwords_en.txt"), "a\n", StandardCharsets.UTF_8);
+
+    byte[] zipBytes = DownloadConfigSet.zipConfigSet(configSetService, "nestedconfig");
+    assertTrue(zipBytes.length > 0);
+
+    boolean foundSolrConfig = false;
+    boolean foundStopWords = false;
+    boolean foundLangDir = false;
+    try (ZipInputStream stream = new ZipInputStream(new ByteArrayInputStream(zipBytes))) {
+      ZipEntry entry;
+      while ((entry = stream.getNextEntry()) != null) {
+        String entryName = entry.getName();
+        assertFalse(
+            "ZIP entry names must use / and must not be empty: " + entryName, entryName.isEmpty());
+        assertFalse("ZIP must not include a nameless root directory", "/".equals(entryName));
+        assertFalse("ZIP entry names must use / not \\: " + entryName, entryName.contains("\\"));
+        if ("solrconfig.xml".equals(entryName)) {
+          foundSolrConfig = true;
+        } else if ("lang/stopwords_en.txt".equals(entryName)) {
+          foundStopWords = true;
+        } else if ("lang/".equals(entryName)) {
+          foundLangDir = true;
+        }
+      }
+    }
+    assertTrue("Did not find solrconfig.xml in downloaded configset", foundSolrConfig);
+    assertTrue("Did not find lang/stopwords_en.txt in downloaded configset", foundStopWords);
+    assertTrue("Did not find lang/ directory entry in downloaded configset", foundLangDir);
+  }
+
+  @Test
+  public void testToZipEntryNameUsesForwardSlashAndSkipsRoot() {
+    Path root = Path.of("configset-root");
+    assertNull(DownloadConfigSet.toZipEntryName(root, root, true));
+    assertEquals("lang/", DownloadConfigSet.toZipEntryName(root, root.resolve("lang"), true));
+    assertEquals(
+        "lang/stopwords_en.txt",
+        DownloadConfigSet.toZipEntryName(
+            root, root.resolve("lang").resolve("stopwords_en.txt"), false));
+    assertEquals(
+        "lang/stopwords_en.txt",
+        DownloadConfigSet.toZipEntryName(root, root.resolve("lang\\stopwords_en.txt"), false));
+  }
 }