From ba1909efac0d6774d42b552cafa4b97ee56aa69a Mon Sep 17 00:00:00 2001 From: Isaries Date: Wed, 30 Sep 2026 07:42:53 +0800 Subject: [PATCH] fix: use atomic write for project.json to prevent corruption from concurrent saves The authoring UI fires a save request on every form change without debouncing. When a teacher edits two fields in rapid succession, two POST /api/author/project/save/{id} requests can reach the backend concurrently, both calling saveProjectContentToDisk() on the same file. Each call opens a new FileOutputStream (which truncates the file) and writes through a BufferedWriter. When two threads race: 1. Thread A opens the file (truncates), begins writing via BufferedWriter which flushes in 8 KiB chunks 2. Thread B opens the same file (truncates again), writes its content, and closes 3. Thread A's close() flushes its remaining buffer at Thread A's file descriptor position, which is now past the end of Thread B's shorter content 4. The file ends up with Thread B's content plus trailing bytes from Thread A's final buffer flush The trailing bytes are always the end of Thread A's JSON (typically "]}"), producing a file that starts as valid JSON but has extra data appended. The Angular client's HttpClient then fails to parse the response, the VLE never initializes, and the author sees a blank preview page with no error message. This was observed in production on a 778 KB project.json: two bytes were appended, causing JSON.parse to throw "Extra data" at position 730,760. The corruption reproduced across multiple save sessions on the same unit, confirming it is a systemic race rather than a one-time event. The fix replaces direct FileOutputStream writes with a write-to-temp-then-atomic-rename pattern in both saveProjectContentToDisk() and replaceMetadataInProjectJSONFile(): 1. Create a temp file in the same directory (Files.createTempFile) 2. Write the full content to the temp file using try-with-resources 3. Set the temp file readable (0644) so static-file servers can serve it 4. Atomically rename the temp file to the target path (Files.move with ATOMIC_MOVE, falling back to REPLACE_EXISTING on filesystems that do not support atomic moves) 5. Clean up the temp file in a finally block Because rename() is atomic on POSIX filesystems, readers always see either the old complete file or the new complete file, never a partially written or interleaved version. The try-with-resources conversion also fixes an existing fd leak: if writer.write() threw an exception in the old code, writer.close() was never called because there was no finally block. --- .../project/impl/ProjectServiceImpl.java | 46 +++++++++++++++---- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/wise/portal/service/project/impl/ProjectServiceImpl.java b/src/main/java/org/wise/portal/service/project/impl/ProjectServiceImpl.java index b995edaec..a1ae876da 100644 --- a/src/main/java/org/wise/portal/service/project/impl/ProjectServiceImpl.java +++ b/src/main/java/org/wise/portal/service/project/impl/ProjectServiceImpl.java @@ -32,6 +32,9 @@ import java.io.OutputStreamWriter; import java.io.Serializable; import java.io.Writer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -857,20 +860,43 @@ public void replaceMetadataInProjectJSONFile(String projectFilePath, ProjectMeta String projectStr = FileUtils.readFileToString(new File(projectFilePath)); JSONObject projectJSONObj = new JSONObject(projectStr); projectJSONObj.put("metadata", metadata.toJSONObject()); - File newProjectJSONFile = new File(projectFilePath); - Writer writer = new BufferedWriter( - new OutputStreamWriter(new FileOutputStream(newProjectJSONFile), "UTF-8")); - writer.write(projectJSONObj.toString()); - writer.close(); + Path targetPath = new File(projectFilePath).toPath(); + Path tempFile = Files.createTempFile(targetPath.getParent(), "project-", ".tmp"); + try { + try (Writer writer = new BufferedWriter( + new OutputStreamWriter(new FileOutputStream(tempFile.toFile()), "UTF-8"))) { + writer.write(projectJSONObj.toString()); + } + tempFile.toFile().setReadable(true, false); + atomicMoveWithFallback(tempFile, targetPath); + } finally { + Files.deleteIfExists(tempFile); + } } public void saveProjectContentToDisk(String projectJSONString, Project project) throws FileNotFoundException, IOException { - String projectJSONPath = curriculumBaseDir + project.getModulePath(); - Writer writer = new BufferedWriter( - new OutputStreamWriter(new FileOutputStream(new File(projectJSONPath)), "UTF-8")); - writer.write(projectJSONString); - writer.close(); + Path targetPath = new File(curriculumBaseDir + project.getModulePath()).toPath(); + Path tempFile = Files.createTempFile(targetPath.getParent(), "project-", ".tmp"); + try { + try (Writer writer = new BufferedWriter( + new OutputStreamWriter(new FileOutputStream(tempFile.toFile()), "UTF-8"))) { + writer.write(projectJSONString); + } + tempFile.toFile().setReadable(true, false); + atomicMoveWithFallback(tempFile, targetPath); + } finally { + Files.deleteIfExists(tempFile); + } + } + + private static void atomicMoveWithFallback(Path source, Path target) throws IOException { + try { + Files.move(source, target, StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE); + } catch (java.nio.file.AtomicMoveNotSupportedException e) { + Files.move(source, target, StandardCopyOption.REPLACE_EXISTING); + } } public Map getDirectoryInfo(File directory) {