diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ea7825d5..faef9e40 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -56,6 +56,9 @@ importers: dotenv: specifier: ^17.4.2 version: 17.4.2 + fflate: + specifier: ^0.8.3 + version: 0.8.3 isbot: specifier: ^5.1.39 version: 5.1.39 @@ -2287,8 +2290,8 @@ packages: picomatch: optional: true - fflate@0.8.2: - resolution: {integrity: sha512-cPJU47OaAoCbg0pBvzsgpTPhmhqI5eJjh/JIu8tPj5q+T7iLvW/JAYUqmE7KOB4R1ZyEhzBaIQpQpardBF5z8A==} + fflate@0.8.3: + resolution: {integrity: sha512-tbZNuJrLwGUp3zshBtdy4W+ORxZuIh8a5ilyIEQDC5rY1f3U20JMry0Ll3WBzU58EZKsEuJFXhb5gwv8CsPvgA==} file-entry-cache@8.0.0: resolution: {integrity: sha512-XXTUwCvisa5oacNGRP9SfNtYBNAMi+RPwBFmblZEF7N7swHYQS6/Zfk7SRwx4D5j3CH211YNRco1DEMNVfZCnQ==} @@ -5729,7 +5732,7 @@ snapshots: '@vitest/ui@4.1.5(vitest@4.1.5)': dependencies: '@vitest/utils': 4.1.5 - fflate: 0.8.2 + fflate: 0.8.3 flatted: 3.4.2 pathe: 2.0.3 sirv: 3.0.2 @@ -6720,7 +6723,7 @@ snapshots: optionalDependencies: picomatch: 4.0.4 - fflate@0.8.2: {} + fflate@0.8.3: {} file-entry-cache@8.0.0: dependencies: diff --git a/src/main/frontend/app/routes/projectlanding/project-landing.tsx b/src/main/frontend/app/routes/projectlanding/project-landing.tsx index 74f0745c..bf4d7a77 100644 --- a/src/main/frontend/app/routes/projectlanding/project-landing.tsx +++ b/src/main/frontend/app/routes/projectlanding/project-landing.tsx @@ -21,12 +21,14 @@ import useEditorTabStore from '~/stores/editor-tab-store' import { cloneProject, createProject, + DEFAULT_MAX_IMPORT_BYTES, exportProject, importProjectFolder, + ImportTooLargeError, openProject, } from '~/services/project-service' import { useRecentProjects } from '~/hooks/use-projects' -import { showErrorToast } from '~/components/toast' +import { showErrorToast, showWarningToast } from '~/components/toast' export default function ProjectLanding() { const navigate = useNavigate() @@ -46,6 +48,7 @@ export default function ProjectLanding() { const [isDiscovering, setIsDiscovering] = useState(false) const [ffConfiguration, setFFConfiguration] = useState([]) const [ffInstanceName, setFFInstanceName] = useState('') + const [maxImportBytes, setMaxImportBytes] = useState(DEFAULT_MAX_IMPORT_BYTES) const importInputRef = useRef(null) useEffect(() => { @@ -59,6 +62,7 @@ export default function ProjectLanding() { .then((info) => { setIsLocalEnvironment(info.isLocal) setRootLocationName(info.isLocal ? 'Computer' : 'Cloud Workspace') + setMaxImportBytes(info.maxImportSize) }) .catch((_) => { showErrorToast('Failed to fetch environment info, defaulting to local mode.') @@ -174,11 +178,26 @@ export default function ProjectLanding() { setIsOpeningProject(true) try { - const project = await importProjectFolder(files) + const project = await importProjectFolder(files, maxImportBytes) openProjectAndNavigate(project) refetch() } catch (error) { - showErrorToast(error instanceof Error ? error.message : 'Failed to import project') + const limitMb = Math.round(maxImportBytes / (1024 * 1024)) + if (error instanceof ImportTooLargeError) { + const sizeMb = (error.bytes / (1024 * 1024)).toFixed(1) + const dimension = error.kind === 'compressed' ? 'when zipped' : 'uncompressed' + showWarningToast( + `This configuration is ${sizeMb} MB ${dimension}, which exceeds the ${limitMb} MB limit. Please import a smaller folder.`, + 'Configuration too large', + ) + } else if (error instanceof ApiError && error.httpCode === 413) { + showWarningToast( + `This configuration is too large to import (over ${limitMb} MB). Please import a smaller folder.`, + 'Configuration too large', + ) + } else { + showErrorToast(error instanceof Error ? error.message : 'Failed to import project') + } } finally { setIsOpeningProject(false) } diff --git a/src/main/frontend/app/services/app-info-service.ts b/src/main/frontend/app/services/app-info-service.ts index 55fca35d..faadd8eb 100644 --- a/src/main/frontend/app/services/app-info-service.ts +++ b/src/main/frontend/app/services/app-info-service.ts @@ -2,6 +2,7 @@ export interface AppInfo { isLocal: boolean + maxImportSize: number } export async function fetchAppInfo(signal?: AbortSignal): Promise { diff --git a/src/main/frontend/app/services/project-service.spec.ts b/src/main/frontend/app/services/project-service.spec.ts new file mode 100644 index 00000000..3607372f --- /dev/null +++ b/src/main/frontend/app/services/project-service.spec.ts @@ -0,0 +1,56 @@ +import { DEFAULT_MAX_IMPORT_BYTES, importProjectFolder, ImportTooLargeError } from './project-service' + +vi.mock('../utils/api', () => ({ + apiFetch: vi.fn(() => Promise.resolve({ name: 'proj', rootPath: '/tmp/proj', filepaths: [] })), + apiUrl: (path: string) => path, +})) + +vi.mock('fflate', () => ({ + zipSync: (_entries: Record, _options: unknown) => new Uint8Array(8), +})) + +function makeFile(relativePath: string, sizeOverride?: number): File { + const file = new File([new Uint8Array(4)], relativePath.split('/').pop() ?? 'file') + Object.defineProperty(file, 'webkitRelativePath', { value: relativePath }) + Object.defineProperty(file, 'arrayBuffer', { value: () => Promise.resolve(new ArrayBuffer(4)) }) + if (sizeOverride !== undefined) { + Object.defineProperty(file, 'size', { value: sizeOverride }) + } + return file +} + +function fileList(...files: File[]): FileList { + return files as unknown as FileList +} + +describe('importProjectFolder', () => { + afterEach(() => { + vi.clearAllMocks() + }) + + it('rejects with an uncompressed ImportTooLargeError when the folder exceeds the limit', async () => { + const half = DEFAULT_MAX_IMPORT_BYTES / 2 + const files = fileList(makeFile('proj/big1.bin', half + 1), makeFile('proj/big2.bin', half + 1)) + + const error = await importProjectFolder(files, DEFAULT_MAX_IMPORT_BYTES).catch((error_: unknown) => error_) + + expect(error).toBeInstanceOf(ImportTooLargeError) + expect((error as ImportTooLargeError).kind).toBe('uncompressed') + expect((error as ImportTooLargeError).bytes).toBe((half + 1) * 2) + }) + + it('ignores the top-level folder entry (empty relative path) when summing sizes', async () => { + const files = fileList(makeFile('proj', DEFAULT_MAX_IMPORT_BYTES), makeFile('proj/Configuration.xml', 10)) + + await expect(importProjectFolder(files, DEFAULT_MAX_IMPORT_BYTES)).resolves.toMatchObject({ name: 'proj' }) + }) + + it('uploads folders that stay within the limit', async () => { + const files = fileList( + makeFile('proj/Configuration.xml', 100), + makeFile('proj/src/main/resources/application.properties', 50), + ) + + await expect(importProjectFolder(files, DEFAULT_MAX_IMPORT_BYTES)).resolves.toMatchObject({ name: 'proj' }) + }) +}) diff --git a/src/main/frontend/app/services/project-service.ts b/src/main/frontend/app/services/project-service.ts index 3e95773b..a28db75c 100644 --- a/src/main/frontend/app/services/project-service.ts +++ b/src/main/frontend/app/services/project-service.ts @@ -1,6 +1,19 @@ +import { zipSync } from 'fflate' import { apiFetch, apiUrl } from '~/utils/api' import type { ConfigurationProject } from '~/types/project.types' +export const DEFAULT_MAX_IMPORT_BYTES = 80 * 1024 * 1024 + +export class ImportTooLargeError extends Error { + constructor( + public readonly bytes: number, + public readonly kind: 'compressed' | 'uncompressed' = 'compressed', + ) { + super('Configuration is too large to import') + this.name = 'ImportTooLargeError' + } +} + export async function fetchProject(name: string): Promise { return apiFetch(`/projects/${encodeURIComponent(name)}`) } @@ -52,21 +65,46 @@ export async function exportProject(projectName: string): Promise { URL.revokeObjectURL(a.href) } -export async function importProjectFolder(files: FileList): Promise { - const formData = new FormData() +export async function importProjectFolder(files: FileList, maxImportBytes: number): Promise { + const projectName = files[0].webkitRelativePath.split('/')[0] - const firstPath = files[0].webkitRelativePath - const projectName = firstPath.split('/')[0] - formData.append('projectName', projectName) + /* + * pre-check on the uncompressed size so an oversized folder fails fast, + */ + let uncompressedBytes = 0 + for (const file of files) { + if (file.webkitRelativePath.split('/').slice(1).join('/')) { + uncompressedBytes += file.size + } + } + + if (uncompressedBytes > maxImportBytes) { + throw new ImportTooLargeError(uncompressedBytes, 'uncompressed') + } + const entries: Record = {} for (const file of files) { - formData.append('files', file) const relativePath = file.webkitRelativePath.split('/').slice(1).join('/') - formData.append('paths', relativePath) + if (!relativePath) continue + entries[relativePath] = new Uint8Array(await file.arrayBuffer()) } + const archive = await zipAsync(entries) + + if (archive.byteLength > maxImportBytes) { + throw new ImportTooLargeError(archive.byteLength, 'compressed') + } + + const formData = new FormData() + formData.append('projectName', projectName) + formData.append('file', new Blob([archive], { type: 'application/zip' }), `${projectName}.zip`) + return apiFetch('/projects/import', { method: 'POST', body: formData, }) } + +async function zipAsync(entries: Record): Promise> { + return zipSync(entries, { level: 6 }) as Uint8Array +} diff --git a/src/main/frontend/package.json b/src/main/frontend/package.json index 3ee343e0..db3c9d9a 100644 --- a/src/main/frontend/package.json +++ b/src/main/frontend/package.json @@ -24,6 +24,7 @@ "clsx": "^2.1.1", "dagre": "^0.8.5", "dotenv": "^17.4.2", + "fflate": "^0.8.3", "isbot": "^5.1.39", "monaco-xsd-code-completion": "^1.1.1", "react": "^19.2.5", diff --git a/src/main/java/org/frankframework/flow/appinfo/AppInfoController.java b/src/main/java/org/frankframework/flow/appinfo/AppInfoController.java index 1166a7b7..57e5778f 100644 --- a/src/main/java/org/frankframework/flow/appinfo/AppInfoController.java +++ b/src/main/java/org/frankframework/flow/appinfo/AppInfoController.java @@ -2,6 +2,7 @@ import java.util.Map; import org.frankframework.flow.filesystem.FileSystemStorage; +import org.frankframework.flow.project.ImportProperties; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -10,13 +11,18 @@ @RequestMapping("/app-info") public class AppInfoController { private final FileSystemStorage fileSystemStorage; + private final ImportProperties importProperties; - public AppInfoController(FileSystemStorage fileSystemStorage) { + public AppInfoController(FileSystemStorage fileSystemStorage, ImportProperties importProperties) { this.fileSystemStorage = fileSystemStorage; + this.importProperties = importProperties; } @GetMapping public Map getInfo() { - return Map.of("isLocal", fileSystemStorage.isLocalEnvironment()); + return Map.of( + "isLocal", fileSystemStorage.isLocalEnvironment(), + "maxImportSize", importProperties.maxUploadSize().toBytes() + ); } } diff --git a/src/main/java/org/frankframework/flow/common/config/WebConfiguration.java b/src/main/java/org/frankframework/flow/common/config/WebConfiguration.java index 49535f73..a1b644fb 100644 --- a/src/main/java/org/frankframework/flow/common/config/WebConfiguration.java +++ b/src/main/java/org/frankframework/flow/common/config/WebConfiguration.java @@ -5,8 +5,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; import com.fasterxml.jackson.databind.json.JsonMapper; +import jakarta.servlet.MultipartConfigElement; +import org.frankframework.flow.project.ImportProperties; import org.frankframework.management.gateway.InputStreamHttpMessageConverter; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.converter.FormHttpMessageConverter; @@ -20,10 +23,13 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; @Configuration +@EnableConfigurationProperties(ImportProperties.class) public class WebConfiguration implements WebMvcConfigurer { private static final long MAX_AGE_SECONDS = 3600; + private static final long MULTIPART_REQUEST_HEADROOM_BYTES = 5L * 1024 * 1024; + @Value("${cors.allowed.origins:}") private String[] allowedOrigins; @@ -59,6 +65,12 @@ public void configureMessageConverters(HttpMessageConverters.ServerBuilder build builder.addCustomConverter(new FormHttpMessageConverter()); } + @Bean + public MultipartConfigElement multipartConfigElement(ImportProperties importProperties) { + long maxFileSize = importProperties.maxUploadSize().toBytes(); + long maxRequestSize = maxFileSize + MULTIPART_REQUEST_HEADROOM_BYTES; + return new MultipartConfigElement(null, maxFileSize, maxRequestSize, 0); + } @Bean public ObjectMapper objectMapper() { diff --git a/src/main/java/org/frankframework/flow/project/ConfigurationProjectController.java b/src/main/java/org/frankframework/flow/project/ConfigurationProjectController.java index 7a64bbea..969fc705 100644 --- a/src/main/java/org/frankframework/flow/project/ConfigurationProjectController.java +++ b/src/main/java/org/frankframework/flow/project/ConfigurationProjectController.java @@ -111,15 +111,15 @@ public void exportProject(@PathVariable String projectName, HttpServletResponse @PostMapping("/import") public ResponseEntity importProject( - @RequestParam("files") List files, - @RequestParam("paths") List paths, + @RequestParam("file") MultipartFile file, @RequestParam("projectName") String projectName ) throws IOException { - if (files.isEmpty() || files.size() != paths.size()) { + if (file.isEmpty()) { + log.warn("Rejected import for project \"{}\": uploaded file is empty", projectName); return ResponseEntity.badRequest().build(); } - ConfigurationProject configurationProject = configurationProjectService.importProjectFromFiles(projectName, files, paths); + ConfigurationProject configurationProject = configurationProjectService.importProjectFromZip(projectName, file); recentProjectsService.addRecentProject(configurationProject.getName(), configurationProject.getRootPath()); return ResponseEntity.ok(configurationProjectService.toDto(configurationProject)); } diff --git a/src/main/java/org/frankframework/flow/project/ConfigurationProjectService.java b/src/main/java/org/frankframework/flow/project/ConfigurationProjectService.java index 587056b9..3de97bf4 100644 --- a/src/main/java/org/frankframework/flow/project/ConfigurationProjectService.java +++ b/src/main/java/org/frankframework/flow/project/ConfigurationProjectService.java @@ -11,6 +11,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; import java.util.zip.ZipOutputStream; import lombok.extern.log4j.Log4j2; import org.eclipse.jgit.api.CloneCommand; @@ -26,6 +27,7 @@ import org.frankframework.flow.recentproject.RecentProject; import org.frankframework.flow.recentproject.RecentProjectsService; import org.frankframework.flow.utility.PathUtils; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; import org.springframework.core.io.ClassPathResource; import org.springframework.http.HttpStatus; @@ -35,15 +37,24 @@ @Log4j2 @Service public class ConfigurationProjectService { + private static final long BYTES_PER_MB = 1024L * 1024; + private static final int IMPORT_BUFFER_SIZE = 8192; + private final FileSystemStorage fileSystemStorage; private final RecentProjectsService recentProjectsService; - // Cache is now ONLY for lightweight Project state (Tokens, Filters), NOT files. + private final long maxUncompressedImportBytes; private final Map projectCache = new ConcurrentHashMap<>(); - public ConfigurationProjectService(FileSystemStorage fileSystemStorage, @Lazy RecentProjectsService recentProjectsService) { + @Autowired + public ConfigurationProjectService(FileSystemStorage fileSystemStorage, @Lazy RecentProjectsService recentProjectsService, ImportProperties importProperties) { + this(fileSystemStorage, recentProjectsService, importProperties.maxUploadSize().toBytes()); + } + + ConfigurationProjectService(FileSystemStorage fileSystemStorage, RecentProjectsService recentProjectsService, long maxUncompressedImportBytes) { this.fileSystemStorage = fileSystemStorage; this.recentProjectsService = recentProjectsService; + this.maxUncompressedImportBytes = maxUncompressedImportBytes; } private static void validatePathSafety(Path path) { @@ -209,28 +220,70 @@ public void exportProjectAsZip(String projectName, OutputStream outputStream) th } } - public ConfigurationProject importProjectFromFiles(String projectName, List files, List paths) throws IOException { + public ConfigurationProject importProjectFromZip(String projectName, MultipartFile zipFile) throws IOException { + log.info("Importing project \"{}\" from uploaded archive \"{}\" ({} bytes)", + projectName, zipFile.getOriginalFilename(), zipFile.getSize()); + Path projectDir = fileSystemStorage.createProjectDirectory(projectName); - for (int i = 0; i < files.size(); i++) { - String relativePath = PathUtils.toForwardSlash(paths.get(i)); + int fileCount = 0; + long totalUncompressedBytes = 0; + try (ZipInputStream zipInputStream = new ZipInputStream(zipFile.getInputStream())) { + ZipEntry entry; + while ((entry = zipInputStream.getNextEntry()) != null) { + String relativePath = PathUtils.toForwardSlash(entry.getName()); - if (relativePath.contains("..") || relativePath.startsWith("/")) { - throw new SecurityException("Invalid file path: " + relativePath); - } + if (relativePath.contains("..") || relativePath.startsWith("/")) { + log.warn("Rejected import of project \"{}\": invalid file path \"{}\"", projectName, relativePath); + throw new SecurityException("Invalid file path: " + relativePath); + } - Path targetPath = projectDir.resolve(relativePath).normalize(); - if (!targetPath.startsWith(projectDir)) { - throw new SecurityException("File path escapes project directory: " + relativePath); - } + Path targetPath = projectDir.resolve(relativePath).normalize(); + if (!targetPath.startsWith(projectDir)) { + log.warn("Rejected import of project \"{}\": file path escapes project directory \"{}\"", projectName, relativePath); + throw new SecurityException("File path escapes project directory: " + relativePath); + } + + if (entry.isDirectory()) { + Files.createDirectories(targetPath); + } else { + Files.createDirectories(targetPath.getParent()); + totalUncompressedBytes += copyZipEntry(zipInputStream, targetPath, totalUncompressedBytes); + fileCount++; + log.trace("Extracted import entry \"{}\" for project \"{}\"", relativePath, projectName); + } - Files.createDirectories(targetPath.getParent()); - files.get(i).transferTo(targetPath); + zipInputStream.closeEntry(); + } + } catch (IOException exception) { + log.error("Failed to import project \"{}\" from uploaded archive", projectName, exception); + throw exception; } + log.info("Imported project \"{}\": extracted {} files ({} uncompressed bytes) to {}", + projectName, fileCount, totalUncompressedBytes, projectDir); + return loadProjectAndCache(projectDir.toString()); } + private long copyZipEntry(ZipInputStream zipInputStream, Path targetPath, long bytesWrittenSoFar) throws IOException { + long entryBytes = 0; + byte[] buffer = new byte[IMPORT_BUFFER_SIZE]; + try (OutputStream out = Files.newOutputStream(targetPath)) { + int read; + while ((read = zipInputStream.read(buffer)) != -1) { + entryBytes += read; + if (bytesWrittenSoFar + entryBytes > maxUncompressedImportBytes) { + long limitMb = maxUncompressedImportBytes / BYTES_PER_MB; + log.warn("Rejected import: decompressed size exceeds the maximum allowed {} MB", limitMb); + throw new ApiException("Imported project exceeds the maximum allowed size of " + limitMb + " MB", HttpStatus.PAYLOAD_TOO_LARGE); + } + out.write(buffer, 0, read); + } + } + return entryBytes; + } + public ConfigurationProjectDTO toDto(ConfigurationProject configurationProject) { String cleanPath = fileSystemStorage.toRelativePath(configurationProject.getRootPath()); diff --git a/src/main/java/org/frankframework/flow/project/ImportProperties.java b/src/main/java/org/frankframework/flow/project/ImportProperties.java new file mode 100644 index 00000000..b35e9340 --- /dev/null +++ b/src/main/java/org/frankframework/flow/project/ImportProperties.java @@ -0,0 +1,15 @@ +package org.frankframework.flow.project; + +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.util.unit.DataSize; + +@ConfigurationProperties(prefix = "flow.import") +public record ImportProperties(DataSize maxUploadSize) { + private static final DataSize DEFAULT_MAX_UPLOAD_SIZE = DataSize.ofMegabytes(80); + + public ImportProperties { + if (maxUploadSize == null) { + maxUploadSize = DEFAULT_MAX_UPLOAD_SIZE; + } + } +} diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index f91867a3..d6e60a45 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -4,8 +4,10 @@ csrf.enabled=true spring.web.resources.static-locations=classpath:/frontend/ -spring.servlet.multipart.max-file-size=50MB -spring.servlet.multipart.max-request-size=50MB +# Maximum size of a project import (both the uploaded zip and its uncompressed contents). +# Aligned with the nginx client_max_body_size (80M) on production. A system administrator can raise this +# limit here without code changes, but the client_max_body_suze must be raised as well. +flow.import.max-upload-size=80MB # Spring Application log settings logging.level.root=INFO diff --git a/src/test/java/org/frankframework/flow/appinfo/AppInfoControllerTest.java b/src/test/java/org/frankframework/flow/appinfo/AppInfoControllerTest.java index df186a8c..c3998869 100644 --- a/src/test/java/org/frankframework/flow/appinfo/AppInfoControllerTest.java +++ b/src/test/java/org/frankframework/flow/appinfo/AppInfoControllerTest.java @@ -11,11 +11,13 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; @WebMvcTest(AppInfoController.class) @AutoConfigureMockMvc(addFilters = false) +@TestPropertySource(properties = "flow.import.max-upload-size=120MB") class AppInfoControllerTest { @Autowired @@ -45,4 +47,13 @@ void getInfoReturnsCloudEnvironmentFlag() throws Exception { verify(fileSystemStorage).isLocalEnvironment(); } + + @Test + void getInfoReturnsConfiguredMaxImportSize() throws Exception { + when(fileSystemStorage.isLocalEnvironment()).thenReturn(true); + + mockMvc.perform(get("/api/app-info")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.maxImportSize").value(120 * 1024 * 1024)); + } } diff --git a/src/test/java/org/frankframework/flow/project/ConfigurationProjectControllerTest.java b/src/test/java/org/frankframework/flow/project/ConfigurationProjectControllerTest.java index 9bcf7c1d..410937db 100644 --- a/src/test/java/org/frankframework/flow/project/ConfigurationProjectControllerTest.java +++ b/src/test/java/org/frankframework/flow/project/ConfigurationProjectControllerTest.java @@ -1,12 +1,12 @@ package org.frankframework.flow.project; -import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import java.io.IOException; import java.io.OutputStream; import java.nio.file.Paths; import java.util.ArrayList; @@ -313,38 +313,84 @@ void importProjectReturnsProjectDto() throws Exception { when(configurationProject.getRootPath()).thenReturn("/path/to/ImportedProject"); when(configurationProject.getConfigurationFiles()).thenReturn(new ArrayList<>()); - when(configurationProjectService.importProjectFromFiles(eq("ImportedProject"), anyList(), anyList())) + when(configurationProjectService.importProjectFromZip(eq("ImportedProject"), any())) .thenReturn(configurationProject); - MockMultipartFile file1 = new MockMultipartFile( - "files", "ConfigurationFile.xml", MediaType.APPLICATION_XML_VALUE, "test".getBytes()); - MockMultipartFile file2 = - new MockMultipartFile("files", "pom.xml", MediaType.APPLICATION_XML_VALUE, "".getBytes()); + MockMultipartFile zip = new MockMultipartFile( + "file", "ImportedProject.zip", "application/zip", "fake-zip-bytes".getBytes()); mockMvc.perform(multipart("/api/projects/import") - .file(file1) - .file(file2) - .param("paths", "src/main/configurations/ConfigurationFile.xml", "pom.xml") + .file(zip) .param("projectName", "ImportedProject")) .andExpect(status().isOk()) .andExpect(jsonPath("$.name").value("ImportedProject")) .andExpect(jsonPath("$.rootPath").value("/path/to/ImportedProject")); - verify(configurationProjectService).importProjectFromFiles(eq("ImportedProject"), anyList(), anyList()); + verify(configurationProjectService).importProjectFromZip(eq("ImportedProject"), any()); verify(recentProjectsService).addRecentProject("ImportedProject", "/path/to/ImportedProject"); } @Test - void importProjectWithMismatchedFilesAndPathsReturnsBadRequest() throws Exception { - MockMultipartFile file1 = new MockMultipartFile( - "files", "ConfigurationFile.xml", MediaType.APPLICATION_XML_VALUE, "test".getBytes()); + void importProjectWithEmptyFileReturnsBadRequest() throws Exception { + MockMultipartFile emptyZip = new MockMultipartFile("file", "empty.zip", "application/zip", new byte[0]); mockMvc.perform(multipart("/api/projects/import") - .file(file1) - .param("paths", "path1.xml", "path2.xml") + .file(emptyZip) .param("projectName", "TestProject")) .andExpect(status().isBadRequest()); - verify(configurationProjectService, never()).importProjectFromFiles(anyString(), anyList(), anyList()); + verify(configurationProjectService, never()).importProjectFromZip(anyString(), any()); + } + + @Test + void importProjectWithoutProjectNameParamReturnsBadRequest() throws Exception { + MockMultipartFile zip = new MockMultipartFile("file", "noname.zip", "application/zip", "zip-bytes".getBytes()); + + mockMvc.perform(multipart("/api/projects/import").file(zip)) + .andExpect(status().isBadRequest()); + + verify(configurationProjectService, never()).importProjectFromZip(anyString(), any()); + } + + @Test + void importProjectWithoutFilePartReturnsBadRequest() throws Exception { + mockMvc.perform(multipart("/api/projects/import").param("projectName", "NoFileProject")) + .andExpect(status().isBadRequest()); + + verify(configurationProjectService, never()).importProjectFromZip(anyString(), any()); + } + + @Test + void importProjectTooLargeReturnsPayloadTooLarge() throws Exception { + when(configurationProjectService.importProjectFromZip(eq("HugeProject"), any())) + .thenThrow(new ApiException("Imported project exceeds the maximum allowed size of 80 MB", HttpStatus.PAYLOAD_TOO_LARGE)); + + MockMultipartFile zip = new MockMultipartFile( + "file", "HugeProject.zip", "application/zip", "fake-zip-bytes".getBytes()); + + mockMvc.perform(multipart("/api/projects/import") + .file(zip) + .param("projectName", "HugeProject")) + .andExpect(status().isPayloadTooLarge()) + .andExpect(jsonPath("$.error").value("Imported project exceeds the maximum allowed size of 80 MB")); + + verify(recentProjectsService, never()).addRecentProject(anyString(), anyString()); + } + + @Test + void importProjectWhenServiceThrowsIOExceptionReturnsServerError() throws Exception { + when(configurationProjectService.importProjectFromZip(eq("FailingProject"), any())) + .thenThrow(new IOException("disk full")); + + MockMultipartFile zip = new MockMultipartFile( + "file", "FailingProject.zip", "application/zip", "fake-zip-bytes".getBytes()); + + mockMvc.perform(multipart("/api/projects/import") + .file(zip) + .param("projectName", "FailingProject")) + .andExpect(status().isInternalServerError()); + + verify(configurationProjectService).importProjectFromZip(eq("FailingProject"), any()); + verify(recentProjectsService, never()).addRecentProject(anyString(), anyString()); } } diff --git a/src/test/java/org/frankframework/flow/project/ConfigurationProjectServiceTest.java b/src/test/java/org/frankframework/flow/project/ConfigurationProjectServiceTest.java index e5272487..23a8ab64 100644 --- a/src/test/java/org/frankframework/flow/project/ConfigurationProjectServiceTest.java +++ b/src/test/java/org/frankframework/flow/project/ConfigurationProjectServiceTest.java @@ -1,5 +1,6 @@ package org.frankframework.flow.project; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -20,6 +21,7 @@ import java.util.stream.Stream; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; +import java.util.zip.ZipOutputStream; import org.frankframework.flow.exception.ApiException; import org.frankframework.flow.filesystem.FileSystemStorage; import org.frankframework.flow.filesystem.FilesystemEntry; @@ -32,8 +34,8 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockMultipartFile; -import org.springframework.web.multipart.MultipartFile; @ExtendWith(MockitoExtension.class) public class ConfigurationProjectServiceTest { @@ -51,10 +53,13 @@ public class ConfigurationProjectServiceTest { private final List recentProjects = new ArrayList<>(); + private static final long DEFAULT_MAX_UNCOMPRESSED_IMPORT_BYTES = 80L * 1024 * 1024; + @BeforeEach void init() { recentProjects.clear(); - configurationProjectService = new ConfigurationProjectService(fileSystemStorage, recentProjectsService); + configurationProjectService = + new ConfigurationProjectService(fileSystemStorage, recentProjectsService, DEFAULT_MAX_UNCOMPRESSED_IMPORT_BYTES); } private void stubFileSystemForProjectCreation() throws IOException { @@ -90,6 +95,23 @@ private void stubFileSystemForProjectCreation() throws IOException { .writeFile(anyString(), anyString()); } + private void stubCreateProjectDirectoryUnderTempDir() throws IOException { + when(fileSystemStorage.createProjectDirectory(anyString())).thenAnswer(invocation -> { + String dirName = invocation.getArgument(0); + Path projectDir = tempDir.resolve(Path.of(dirName).getFileName().toString()); + Files.createDirectories(projectDir); + return projectDir; + }); + } + + private void stubToAbsolutePathUnderTempDir() { + when(fileSystemStorage.toAbsolutePath(anyString())).thenAnswer(invocation -> { + String pathStr = invocation.getArgument(0); + Path path = Path.of(pathStr); + return path.isAbsolute() ? path : tempDir.resolve(pathStr); + }); + } + @Test public void testAddingProjectToProjectService() throws IOException, ApiException { when(fileSystemStorage.isLocalEnvironment()).thenReturn(true); @@ -261,7 +283,7 @@ public void testDisableFilterProjectNotFound() { } @Test - public void importProjectFromFilesSuccess() throws Exception { + public void importProjectFromZipSuccess() throws Exception { when(fileSystemStorage.createProjectDirectory(anyString())).thenAnswer(invocation -> { String dirName = invocation.getArgument(0); Path dirPath = Path.of(dirName); @@ -282,21 +304,12 @@ public void importProjectFromFilesSuccess() throws Exception { String projectName = "imported_project"; - MockMultipartFile configFile = new MockMultipartFile( - "files", - "Configuration.xml", - "application/xml", - "".getBytes(StandardCharsets.UTF_8) + MockMultipartFile archive = zipArchive( + "src/main/configurations/Configuration.xml", "", + "src/main/resources/application.properties", "key=value" ); - MockMultipartFile propsFile = new MockMultipartFile( - "files", "application.properties", "text/plain", "key=value".getBytes(StandardCharsets.UTF_8)); - - List files = List.of(configFile, propsFile); - List paths = - List.of("src/main/configurations/Configuration.xml", "src/main/resources/application.properties"); - - ConfigurationProject configurationProject = configurationProjectService.importProjectFromFiles(projectName, files, paths); + ConfigurationProject configurationProject = configurationProjectService.importProjectFromZip(projectName, archive); assertNotNull(configurationProject); assertEquals(projectName, configurationProject.getName()); @@ -313,7 +326,7 @@ public void importProjectFromFilesSuccess() throws Exception { } @Test - void importProjectFromFilesRejectsPathTraversalWithDoubleDots() throws IOException { + void importProjectFromZipRejectsPathTraversalWithDoubleDots() throws IOException { when(fileSystemStorage.createProjectDirectory(anyString())).thenAnswer(invocation -> { String dirName = invocation.getArgument(0); Path projectDir = tempDir.resolve(dirName); @@ -323,19 +336,15 @@ void importProjectFromFilesRejectsPathTraversalWithDoubleDots() throws IOExcepti String projectName = "traversal_project"; - MockMultipartFile maliciousFile = new MockMultipartFile( - "files", "evil.xml", "application/xml", "".getBytes(StandardCharsets.UTF_8)); + MockMultipartFile archive = zipArchive("../../../etc/evil.xml", ""); - List files = List.of(maliciousFile); - List paths = List.of("../../../etc/evil.xml"); - - SecurityException ex = assertThrows(SecurityException.class, () -> configurationProjectService.importProjectFromFiles(projectName, files, paths)); + SecurityException ex = assertThrows(SecurityException.class, () -> configurationProjectService.importProjectFromZip(projectName, archive)); assertTrue(ex.getMessage().contains("Invalid file path")); } @Test - void importProjectFromFilesRejectsAbsolutePath() throws IOException { + void importProjectFromZipRejectsAbsolutePath() throws IOException { when(fileSystemStorage.createProjectDirectory(anyString())).thenAnswer(invocation -> { String dirName = invocation.getArgument(0); Path projectDir = tempDir.resolve(dirName); @@ -345,17 +354,244 @@ void importProjectFromFilesRejectsAbsolutePath() throws IOException { String projectName = "abs_path_project"; - MockMultipartFile maliciousFile = new MockMultipartFile( - "files", "evil.xml", "application/xml", "".getBytes(StandardCharsets.UTF_8)); + MockMultipartFile archive = zipArchive("/etc/passwd", "root:x:0:0"); + + SecurityException ex = assertThrows(SecurityException.class, () -> configurationProjectService.importProjectFromZip(projectName, archive)); + + assertTrue(ex.getMessage().contains("Invalid file path")); + } + + @Test + void importProjectFromZipWithEmptyArchiveCreatesEmptyProject() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + ConfigurationProject configurationProject = + configurationProjectService.importProjectFromZip("empty_archive_project", emptyZipArchive()); + + assertNotNull(configurationProject); + assertEquals("empty_archive_project", configurationProject.getName()); + + Path projectDir = tempDir.resolve("empty_archive_project"); + assertTrue(Files.isDirectory(projectDir), "Project directory should be created even for an empty archive"); + try (Stream files = Files.walk(projectDir)) { + assertEquals(0, files.filter(Files::isRegularFile).count(), "Empty archive should not produce any files"); + } + } + + @Test + void importProjectFromZipWritesEmptyFileEntry() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + MockMultipartFile archive = zipArchive("src/main/configurations/Empty.xml", ""); + + configurationProjectService.importProjectFromZip("empty_file_project", archive); + + Path emptyFile = tempDir.resolve("empty_file_project/src/main/configurations/Empty.xml"); + assertTrue(Files.exists(emptyFile), "An empty file entry should still be written to disk"); + assertEquals(0, Files.size(emptyFile), "An empty file entry should be zero bytes"); + } + + @Test + void importProjectFromZipCreatesNestedDirectoryStructure() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + MockMultipartFile archive = zipArchive("a/b/c/Deep.xml", ""); + + configurationProjectService.importProjectFromZip("nested_project", archive); + + Path deepFile = tempDir.resolve("nested_project/a/b/c/Deep.xml"); + assertTrue(Files.exists(deepFile), "Missing parent directories should be created for deep entries"); + assertEquals("", Files.readString(deepFile, StandardCharsets.UTF_8)); + } + + @Test + void importProjectFromZipCreatesExplicitDirectoryEntries() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + MockMultipartFile archive = zipArchiveWithDirectory("emptyfolder/", "emptyfolder/keep.xml", ""); + + configurationProjectService.importProjectFromZip("dir_entry_project", archive); + + Path dir = tempDir.resolve("dir_entry_project/emptyfolder"); + assertTrue(Files.isDirectory(dir), "Explicit directory entries should be created"); + assertTrue(Files.exists(dir.resolve("keep.xml")), "Files inside an explicit directory entry should be written"); + } + + @Test + void importProjectFromZipPreservesBinaryContentByteForByte() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + byte[] binary = {0, 1, 2, (byte) 0xFF, (byte) 0xFE, 65, 66, 0, 67}; + MockMultipartFile archive = binaryZipArchive("assets/blob.bin", binary); + + configurationProjectService.importProjectFromZip("binary_project", archive); + + Path blob = tempDir.resolve("binary_project/assets/blob.bin"); + assertTrue(Files.exists(blob)); + assertArrayEquals(binary, Files.readAllBytes(blob), "Binary content should be written byte-for-byte"); + } + + @Test + void importProjectFromZipWritesAllFilesFromMultiEntryArchive() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + MockMultipartFile archive = zipArchive( + "Configuration.xml", "", + "src/main/resources/application.properties", "key=value", + "DeploymentSpecifics.properties", "env=test", + "README.md", "# Imported" + ); + + configurationProjectService.importProjectFromZip("multi_file_project", archive); + + Path projectDir = tempDir.resolve("multi_file_project"); + assertTrue(Files.exists(projectDir.resolve("Configuration.xml"))); + assertTrue(Files.exists(projectDir.resolve("src/main/resources/application.properties"))); + assertTrue(Files.exists(projectDir.resolve("DeploymentSpecifics.properties"))); + assertTrue(Files.exists(projectDir.resolve("README.md"))); + } + + @Test + void importedProjectIsRetrievableFromCacheAfterImport() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + MockMultipartFile archive = zipArchive("Configuration.xml", ""); + configurationProjectService.importProjectFromZip("retrievable_project", archive); + + ConfigurationProject retrieved = configurationProjectService.getProject("retrievable_project"); + assertNotNull(retrieved, "An imported project should be cached and retrievable by name"); + assertEquals("retrievable_project", retrieved.getName()); + } + + @Test + void importProjectFromZipRejectsArchiveExceedingMaxUncompressedSize() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + + ConfigurationProjectService limitedService = + new ConfigurationProjectService(fileSystemStorage, recentProjectsService, 16); + + MockMultipartFile archive = zipArchive("big.bin", "a".repeat(64)); + + ApiException ex = assertThrows( + ApiException.class, + () -> limitedService.importProjectFromZip("too_large_project", archive)); + + assertEquals(HttpStatus.PAYLOAD_TOO_LARGE, ex.getStatus()); + assertTrue(ex.getMessage().contains("exceeds the maximum allowed size")); + } + + @Test + void importProjectFromZipEnforcesLimitAcrossMultipleEntries() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + + ConfigurationProjectService limitedService = + new ConfigurationProjectService(fileSystemStorage, recentProjectsService, 100); + + MockMultipartFile archive = zipArchive( + "part1.bin", "a".repeat(60), + "part2.bin", "b".repeat(60) + ); + + ApiException ex = assertThrows( + ApiException.class, + () -> limitedService.importProjectFromZip("cumulative_too_large_project", archive)); + + assertEquals(HttpStatus.PAYLOAD_TOO_LARGE, ex.getStatus()); + assertTrue(ex.getMessage().contains("exceeds the maximum allowed size")); + } + + @Test + void importProjectFromZipAllowsArchiveExactlyAtMaxUncompressedSize() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + stubToAbsolutePathUnderTempDir(); + + long limit = 64; + ConfigurationProjectService limitedService = + new ConfigurationProjectService(fileSystemStorage, recentProjectsService, limit); + + MockMultipartFile archive = zipArchive("exact.bin", "a".repeat((int) limit)); - List files = List.of(maliciousFile); - List paths = List.of("/etc/passwd"); + ConfigurationProject configurationProject = limitedService.importProjectFromZip("boundary_project", archive); - SecurityException ex = assertThrows(SecurityException.class, () -> configurationProjectService.importProjectFromFiles(projectName, files, paths)); + assertNotNull(configurationProject); + Path written = tempDir.resolve("boundary_project/exact.bin"); + assertEquals(limit, Files.size(written), "Content exactly at the limit should be accepted"); + } + + @Test + void importProjectFromZipRejectsNestedPathTraversal() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + + MockMultipartFile archive = zipArchive("config/../../../evil.xml", ""); + + SecurityException ex = assertThrows( + SecurityException.class, + () -> configurationProjectService.importProjectFromZip("nested_traversal_project", archive)); assertTrue(ex.getMessage().contains("Invalid file path")); } + @Test + void importProjectFromZipRejectsBackslashPathTraversal() throws Exception { + stubCreateProjectDirectoryUnderTempDir(); + + MockMultipartFile archive = zipArchive("..\\..\\evil.xml", ""); + + SecurityException ex = assertThrows( + SecurityException.class, + () -> configurationProjectService.importProjectFromZip("backslash_traversal_project", archive)); + + assertTrue(ex.getMessage().contains("Invalid file path")); + } + + private static MockMultipartFile zipArchive(String... pathThenContent) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + for (int i = 0; i < pathThenContent.length; i += 2) { + zos.putNextEntry(new ZipEntry(pathThenContent[i])); + zos.write(pathThenContent[i + 1].getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + } + return new MockMultipartFile("file", "import.zip", "application/zip", baos.toByteArray()); + } + + private static MockMultipartFile emptyZipArchive() throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ZipOutputStream zos = new ZipOutputStream(baos); + zos.close(); + return new MockMultipartFile("file", "empty-import.zip", "application/zip", baos.toByteArray()); + } + + private static MockMultipartFile binaryZipArchive(String entryName, byte[] content) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + zos.putNextEntry(new ZipEntry(entryName)); + zos.write(content); + zos.closeEntry(); + } + return new MockMultipartFile("file", "binary-import.zip", "application/zip", baos.toByteArray()); + } + + private static MockMultipartFile zipArchiveWithDirectory(String directoryEntry, String fileEntry, String fileContent) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + zos.putNextEntry(new ZipEntry(directoryEntry)); + zos.closeEntry(); + zos.putNextEntry(new ZipEntry(fileEntry)); + zos.write(fileContent.getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + return new MockMultipartFile("file", "dir-import.zip", "application/zip", baos.toByteArray()); + } + @Test void testInvalidateCacheClearsAllProjects() throws Exception { stubFileSystemForProjectCreation(); @@ -425,7 +661,7 @@ void testOpenProjectFromDisk() throws Exception { } @Test - void testOpenProjectFromDiskThrowsWhenPathDoesNotExist() throws Exception { + void testOpenProjectFromDiskThrowsWhenPathDoesNotExist() { when(fileSystemStorage.toAbsolutePath(anyString())).thenAnswer(invocation -> { String pathStr = invocation.getArgument(0); Path path = Path.of(pathStr);