Fix theme zip file upload bug and provide finding root folder function (#1080)

* Fix theme zip file upload bug and provide finding root folder function

* Update ThemeServiceImpl.java

Co-authored-by: Ryan Wang <i@ryanc.cc>
pull/1081/head
John Niang 2020-09-23 20:20:52 +08:00 committed by GitHub
parent fc5742eae8
commit d0568686c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 130 additions and 67 deletions

View File

@ -386,10 +386,15 @@ public class AdminServiceImpl implements AdminService {
// Unzip
FileUtils.unzip(downloadResponseEntity.getBody(), assetTempPath);
// find root folder
Path adminRootPath = FileUtils.findRootPath(assetTempPath,
path -> StringUtils.equalsIgnoreCase("index.html", path.getFileName().toString()))
.orElseThrow(() -> new BadRequestException("无法准确定位到压缩包的根路径,请确认包含 index.html 文件。"));
// Copy it to template/admin folder
FileUtils.copyFolder(FileUtils.tryToSkipZipParentFolder(assetTempPath), adminPath);
FileUtils.copyFolder(adminRootPath, adminPath);
} catch (Throwable t) {
throw new ServiceException("更新 Halo admin 失败", t);
throw new ServiceException("更新 Halo admin 失败" + t.getMessage(), t);
}
}

View File

@ -45,7 +45,6 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.zip.ZipInputStream;
@ -78,8 +77,6 @@ public class ThemeServiceImpl implements ThemeService {
private final ApplicationEventPublisher eventPublisher;
private final AtomicReference<String> activeThemeId = new AtomicReference<>();
/**
* Activated theme id.
*/
@ -422,7 +419,7 @@ public class ThemeServiceImpl implements ThemeService {
// Unzip to temp path
FileUtils.unzip(zis, themeTempPath);
Path themePath = FileUtils.tryToSkipZipParentFolder(themeTempPath);
Path themePath = getThemeRootPath(themeTempPath);
// Go to the base folder and add the theme into system
return add(themePath);
@ -523,7 +520,7 @@ public class ThemeServiceImpl implements ThemeService {
return add(themeTmpPath);
} catch (IOException | GitAPIException e) {
throw new ServiceException("主题拉取失败 " + uri, e);
throw new ServiceException("主题拉取失败 " + uri + "。" + e.getMessage(), e);
} finally {
FileUtils.deleteFolderQuietly(tmpPath);
}
@ -643,6 +640,9 @@ public class ThemeServiceImpl implements ThemeService {
if (e instanceof ThemeNotSupportException) {
throw (ThemeNotSupportException) e;
}
if (e instanceof GitAPIException) {
throw new ThemeUpdateException("主题更新失败!" + e.getMessage(), e);
}
throw new ThemeUpdateException("主题更新失败!您与主题作者可能同时更改了同一个文件,您也可以尝试删除主题并重新拉取最新的主题", e).setErrorData(themeId);
}
@ -681,7 +681,7 @@ public class ThemeServiceImpl implements ThemeService {
// Unzip to temp path
FileUtils.unzip(zis, themeTempPath);
Path preparePath = FileUtils.tryToSkipZipParentFolder(themeTempPath);
Path preparePath = getThemeRootPath(themeTempPath);
ThemeProperty prepareThemeProperty = getProperty(preparePath);
@ -857,4 +857,17 @@ public class ThemeServiceImpl implements ThemeService {
.orElseThrow(() -> new ThemePropertyMissingException(themePath + " 没有说明文件").setErrorData(themePath));
}
/**
* Get theme root path.
*
* @param themePath theme folder path
* @return real theme root path
* @throws IOException IO exception
*/
@NonNull
private Path getThemeRootPath(@NonNull Path themePath) throws IOException {
return FileUtils.findRootPath(themePath,
path -> StringUtils.equalsAny(path.getFileName().toString(), "theme.yaml", "theme.yml"))
.orElseThrow(() -> new BadRequestException("无法准确定位到主题根目录,请确认主题目录中包含 theme.ymltheme.yaml。"));
}
}

View File

@ -63,7 +63,7 @@ public enum ThemePropertyScanner {
}
try (Stream<Path> pathStream = Files.list(themePath)) {
// List and filter sub folders
List<Path> themePaths = pathStream.filter(path -> Files.isDirectory(path))
List<Path> themePaths = pathStream.filter(Files::isDirectory)
.collect(Collectors.toList());
if (CollectionUtils.isEmpty(themePaths)) {

View File

@ -10,8 +10,10 @@ import run.halo.app.exception.ForbiddenException;
import java.io.*;
import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.zip.ZipEntry;
@ -104,14 +106,16 @@ public class FileUtils {
* @throws IOException throws when failed to access file to be unzipped
*/
public static void unzip(@NonNull ZipInputStream zis, @NonNull Path targetPath) throws IOException {
// 1. unzip file to folder
// 2. return the folder path
Assert.notNull(zis, "Zip input stream must not be null");
Assert.notNull(targetPath, "Target path must not be null");
// Create path if absent
createIfAbsent(targetPath);
// Must be empty
mustBeEmpty(targetPath);
// Folder must be empty
ensureEmpty(targetPath);
ZipEntry zipEntry = zis.getNextEntry();
@ -120,7 +124,7 @@ public class FileUtils {
Path entryPath = targetPath.resolve(zipEntry.getName());
// Check directory
FileUtils.checkDirectoryTraversal(targetPath, entryPath);
checkDirectoryTraversal(targetPath, entryPath);
if (zipEntry.isDirectory()) {
// Create directories
@ -132,19 +136,6 @@ public class FileUtils {
zipEntry = zis.getNextEntry();
}
File targetDir = targetPath.toFile();
List<File> files = Arrays.asList(targetDir.listFiles());
// if zip file has root file
if (files.size() == 1 && files.get(0).isDirectory()) {
String rootPath = files.get(0).toPath().toString();
String rootFile = rootPath.substring(rootPath.lastIndexOf("/") + 1);
File[] propertyFiles = files.get(0).listFiles();
for (File propertyFile : propertyFiles) {
String filePath = propertyFile.toPath().toString();
String destPath = filePath.replace(rootFile, "");
Files.copy(propertyFile.toPath(), Paths.get(destPath));
}
}
}
/**
@ -230,27 +221,46 @@ public class FileUtils {
}
/**
* Try to skip zip parent folder. (Go into base folder)
* Find root path.
*
* @param unzippedPath unzipped path must not be null
* @return path containing base files
* @throws IOException
* @param path super root path starter
* @param pathPredicate path predicate
* @return empty if path is not a directory or the given path predicate is null
* @throws IOException IO exception
*/
public static Path tryToSkipZipParentFolder(@NonNull Path unzippedPath) throws IOException {
Assert.notNull(unzippedPath, "Unzipped folder must not be null");
// TODO May cause a latent problem.
try (Stream<Path> pathStream = Files.list(unzippedPath)) {
List<Path> childrenPath = pathStream.collect(Collectors.toList());
Path realPath = childrenPath.get(0);
if (childrenPath.size() == 1 && Files.isDirectory(realPath)) {
// Check directory traversal
checkDirectoryTraversal(unzippedPath, realPath);
return realPath;
}
return unzippedPath;
@NonNull
public static Optional<Path> findRootPath(@NonNull final Path path, @Nullable final Predicate<Path> pathPredicate) throws IOException {
if (!Files.isDirectory(path) || pathPredicate == null) {
// if the path is not a directory or the given path predicate is null, then return an empty optional
return Optional.empty();
}
log.debug("Trying to find root path from [{}]", path);
// the queue holds folders which may be root
final LinkedList<Path> queue = new LinkedList<>();
queue.push(path);
while (!queue.isEmpty()) {
// pop the first path as candidate root path
final Path rootPath = queue.pop();
try (final Stream<Path> childrenPaths = Files.list(rootPath)) {
List<Path> subFolders = new LinkedList<>();
Optional<Path> matchedPath = childrenPaths.peek(child -> {
if (Files.isDirectory(child)) {
// collect directory
subFolders.add(child);
}
}).filter(pathPredicate).findAny();
if (matchedPath.isPresent()) {
log.debug("Found root path: [{}]", rootPath);
return Optional.of(rootPath);
}
// add all folder into queue
subFolders.forEach(queue::push);
}
}
// if tests are failed completely
return Optional.empty();
}
/**
@ -295,7 +305,7 @@ public class FileUtils {
* @param path path must not be null
* @throws IOException
*/
public static void mustBeEmpty(@NonNull Path path) throws IOException {
public static void ensureEmpty(@NonNull Path path) throws IOException {
if (!isEmpty(path)) {
throw new DirectoryNotEmptyException("Target directory: " + path + " was not empty");
}
@ -397,4 +407,5 @@ public class FileUtils {
public static Path createTempDirectory() throws IOException {
return Files.createTempDirectory("halo");
}
}

View File

@ -7,6 +7,7 @@ import run.halo.app.service.ThemeService;
import java.io.FileNotFoundException;
import java.util.*;
import java.util.concurrent.TimeUnit;
/**
* GithubUtils send request to api.github.com
@ -298,7 +299,7 @@ public class GithubUtils {
}
try {
Thread.sleep(2000);
TimeUnit.SECONDS.sleep(2);
} catch (InterruptedException e) {
break;
}

View File

@ -1,6 +1,7 @@
package run.halo.app.utils;
import lombok.extern.slf4j.Slf4j;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import run.halo.app.model.support.HaloConst;
@ -12,6 +13,7 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.zip.ZipOutputStream;
@ -25,10 +27,20 @@ import static org.junit.jupiter.api.Assertions.*;
@Slf4j
class FileUtilsTest {
Path tempDirectory = null;
@AfterEach
void cleanUp() throws IOException {
if (tempDirectory != null) {
FileUtils.deleteFolder(tempDirectory);
assertTrue(Files.notExists(tempDirectory));
}
}
@Test
void deleteFolder() throws IOException {
// Create a temp folder
Path tempDirectory = Files.createTempDirectory("halo-test");
tempDirectory = Files.createTempDirectory("halo-test");
Path testPath = tempDirectory.resolve("test/test/test");
@ -58,21 +70,16 @@ class FileUtilsTest {
walkList.forEach(path -> log.debug(path.toString()));
assertEquals(0, walkList.size());
}
// Delete it
FileUtils.deleteFolder(tempDirectory);
assertTrue(Files.notExists(tempDirectory));
}
@Test
void zipFolderTest() throws IOException {
// Create some temporary files
Path rootFolder = Files.createTempDirectory("zip-root-");
log.debug("Folder name: [{}]", rootFolder.getFileName());
Files.createTempFile(rootFolder, "zip-file1-", ".txt");
Files.createTempFile(rootFolder, "zip-file2-", ".txt");
Path subRootFolder = Files.createTempDirectory(rootFolder, "zip-subroot-");
tempDirectory = Files.createTempDirectory("zip-root-");
log.debug("Folder name: [{}]", tempDirectory.getFileName());
Files.createTempFile(tempDirectory, "zip-file1-", ".txt");
Files.createTempFile(tempDirectory, "zip-file2-", ".txt");
Path subRootFolder = Files.createTempDirectory(tempDirectory, "zip-subroot-");
Files.createTempFile(subRootFolder, "zip-subfile1-", ".txt");
Files.createTempFile(subRootFolder, "zip-subfile2-", ".txt");
@ -81,11 +88,10 @@ class FileUtilsTest {
// Create zip output stream
try (ZipOutputStream zipOut = new ZipOutputStream(Files.newOutputStream(zipToStore))) {
// Zip file
FileUtils.zip(rootFolder, zipOut);
FileUtils.zip(tempDirectory, zipOut);
}
// Clear the test folder created before
FileUtils.deleteFolder(rootFolder);
Files.delete(zipToStore);
}
@ -112,7 +118,7 @@ class FileUtilsTest {
@Test
void testRenameFile() throws IOException {
// Create a temp folder
Path tempDirectory = Files.createTempDirectory("halo-test");
tempDirectory = Files.createTempDirectory("halo-test");
Path testPath = tempDirectory.resolve("test/test");
Path filePath = tempDirectory.resolve("test/test/test.file");
@ -132,14 +138,12 @@ class FileUtilsTest {
assertFalse(Files.exists(filePath));
assertTrue(Files.isRegularFile(newPath));
assertEquals(content, new String(Files.readAllBytes(newPath)));
FileUtils.deleteFolder(tempDirectory);
}
@Test
void testRenameFolder() throws IOException {
// Create a temp folder
Path tempDirectory = Files.createTempDirectory("halo-test");
tempDirectory = Files.createTempDirectory("halo-test");
Path testPath = tempDirectory.resolve("test/test");
Path filePath = tempDirectory.resolve("test/test.file");
@ -154,14 +158,12 @@ class FileUtilsTest {
assertTrue(Files.isDirectory(newPath));
assertTrue(Files.isRegularFile(newPath.resolve("test.file")));
FileUtils.deleteFolder(tempDirectory);
}
@Test
void testRenameRepeat() throws IOException {
// Create a temp folder
Path tempDirectory = Files.createTempDirectory("halo-test");
tempDirectory = Files.createTempDirectory("halo-test");
Path testPathOne = tempDirectory.resolve("test/testOne");
Path testPathTwo = tempDirectory.resolve("test/testTwo");
@ -191,7 +193,38 @@ class FileUtilsTest {
} catch (Exception e) {
assertTrue(e instanceof FileAlreadyExistsException);
}
}
FileUtils.deleteFolder(tempDirectory);
@Test
void findRootPathTest() throws IOException {
// build folder structure
// folder1
// file1
// folder2
// file2
// folder3
// file3
// expected: folder2
tempDirectory = Files.createTempDirectory("halo-test");
log.info("Preparing test folder structure");
Path folder1 = tempDirectory.resolve("folder1");
Files.createDirectory(folder1);
Path file1 = tempDirectory.resolve("file1");
Files.createFile(file1);
Path folder2 = tempDirectory.resolve("folder2");
Files.createDirectory(folder2);
Path file2 = folder2.resolve("file2");
Files.createFile(file2);
Path folder3 = folder2.resolve("folder3");
Files.createDirectory(folder3);
Path file3 = folder3.resolve("file3");
Files.createFile(file3);
log.info("Prepared test folder structure");
// find the root folder where file3 locates, and we expect folder3
Optional<Path> rootPath = FileUtils.findRootPath(tempDirectory, path -> path.getFileName().toString().equals("file3"));
assertTrue(rootPath.isPresent());
assertEquals(folder3.toString(), rootPath.get().toString());
}
}