From d2762ca9b2e4915341d9785645f50d1c3ccfc93a Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 09:26:33 -0800 Subject: [PATCH 1/7] Update 'pom.xml' hashstore-java version to 1.1.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 105e212f..fb67f471 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ org.dataone hashstore - 1.0-SNAPSHOT + 1.1.0 hashstore https://github.com/DataONEorg/hashstore-java From c938a9ba127ee7b06408c188280d44e00d87016d Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 10:46:29 -0800 Subject: [PATCH 2/7] Refactor 'generateTmpFile' in 'FileHashStoreUtility' to set 'rw-r-----' file permissions on tmp file and add new junit test --- .../filehashstore/FileHashStoreUtility.java | 20 ++++++++++++---- .../FileHashStoreProtectedTest.java | 23 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java index b180b050..ebd36806 100644 --- a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java +++ b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.attribute.PosixFilePermission; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; @@ -16,9 +17,11 @@ import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Random; +import java.util.Set; import java.util.stream.Stream; import javax.xml.bind.DatatypeConverter; @@ -336,10 +339,19 @@ public static File generateTmpFile(String prefix, Path directory) int randomNumber = rand.nextInt(1000000); String newPrefix = prefix + "-" + System.currentTimeMillis() + randomNumber; - Path newPath = Files.createTempFile(directory, newPrefix, null); - File newFile = newPath.toFile(); - newFile.deleteOnExit(); - return newFile; + Path newTmpPath = Files.createTempFile(directory, newPrefix, null); + File newTmpFile = newTmpPath.toFile(); + + // Set default file permissions 'rw- r-- ---' / posix 640 + Set permissions = new HashSet<>(); + permissions.add(PosixFilePermission.OWNER_READ); + permissions.add(PosixFilePermission.OWNER_WRITE); + permissions.add(PosixFilePermission.GROUP_READ); + Files.setPosixFilePermissions(newTmpPath, permissions); + // Mark tmp file to be cleaned up if it runs into an issue + newTmpFile.deleteOnExit(); + + return newTmpFile; } /** diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java index f5a3c486..04535d6c 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java @@ -15,15 +15,18 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermission; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Properties; +import java.util.Set; import javax.xml.bind.DatatypeConverter; @@ -2421,4 +2424,24 @@ public void fileHashStoreUtility_renamePathForRestoration() throws Exception { } } } + + + /** + * Confirm that generateTemporaryFile creates tmpFile with expected permissions + */ + @Test + public void fileHashStoreUtility_generateTemporaryFile_permissions() throws Exception { + Path directory = tempFolder.resolve("hashstore"); + // newFile + File tmpFile = FileHashStoreUtility.generateTmpFile("testfile", directory); + + Collection expectedPermissions = new HashSet<>(); + expectedPermissions.add(PosixFilePermission.OWNER_READ); + expectedPermissions.add(PosixFilePermission.OWNER_WRITE); + expectedPermissions.add(PosixFilePermission.GROUP_READ); + + Set actualPermissions = Files.getPosixFilePermissions(tmpFile.toPath()); + + assertEquals(expectedPermissions, actualPermissions); + } } From 9432cc13c28ae0b6e4771c5fff6686ca79fd418c Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 10:50:45 -0800 Subject: [PATCH 3/7] Add new junit test for 'storeObject' to confirm that file permissions persist after moving a tmp file to its permanent location --- .../FileHashStoreInterfaceTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java index 303af320..514a93a8 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java @@ -17,6 +17,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermission; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; @@ -26,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -163,6 +165,32 @@ public void storeObject_hexDigests() throws Exception { } } + /** + * Check that data object stored contains the correct permission settings 'rw- r-- ---' + */ + @Test + public void storeObject_filePermissions() throws Exception { + for (String pid : testData.pidList) { + String pidFormatted = pid.replace("/", "_"); + Path testDataFile = testData.getTestFile(pidFormatted); + + try (InputStream dataStream = Files.newInputStream(testDataFile)) { + fileHashStore.storeObject(dataStream, pid, null, null, null, -1); + + Path objRealPath = fileHashStore.getHashStoreDataObjectPath(pid); + + Collection expectedPermissions = new HashSet<>(); + expectedPermissions.add(PosixFilePermission.OWNER_READ); + expectedPermissions.add(PosixFilePermission.OWNER_WRITE); + expectedPermissions.add(PosixFilePermission.GROUP_READ); + + Set actualPermissions = Files.getPosixFilePermissions(objRealPath); + + assertEquals(expectedPermissions, actualPermissions); + } + } + } + /** * Check that store object throws exception when object is null */ From 6b71e62a352d290c5c3c8bcccd4ad9482054ad0c Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 10:52:07 -0800 Subject: [PATCH 4/7] Rename junit test to match method being tested --- .../hashstore/filehashstore/FileHashStoreProtectedTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java index 04535d6c..8955e447 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java @@ -2430,7 +2430,7 @@ public void fileHashStoreUtility_renamePathForRestoration() throws Exception { * Confirm that generateTemporaryFile creates tmpFile with expected permissions */ @Test - public void fileHashStoreUtility_generateTemporaryFile_permissions() throws Exception { + public void fileHashStoreUtility_generateTmpFile_permissions() throws Exception { Path directory = tempFolder.resolve("hashstore"); // newFile File tmpFile = FileHashStoreUtility.generateTmpFile("testfile", directory); From a1b7f53ba7d0392010b60952777ff41089159bcd Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 11:23:14 -0800 Subject: [PATCH 5/7] Revise new junit tests for tmpFile permissions to be more thorough (check for absence of other permissions) --- .../hashstore/filehashstore/FileHashStoreInterfaceTest.java | 6 ++++++ .../hashstore/filehashstore/FileHashStoreProtectedTest.java | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java index 514a93a8..e9e63fd9 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java @@ -187,6 +187,12 @@ public void storeObject_filePermissions() throws Exception { Set actualPermissions = Files.getPosixFilePermissions(objRealPath); assertEquals(expectedPermissions, actualPermissions); + assertFalse(actualPermissions.contains(PosixFilePermission.OWNER_EXECUTE)); + assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_WRITE)); + assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_EXECUTE)); + assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_READ)); + assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_WRITE)); + assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_EXECUTE)); } } } diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java index 8955e447..172a6082 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java @@ -2443,5 +2443,11 @@ public void fileHashStoreUtility_generateTmpFile_permissions() throws Exception Set actualPermissions = Files.getPosixFilePermissions(tmpFile.toPath()); assertEquals(expectedPermissions, actualPermissions); + assertFalse(actualPermissions.contains(PosixFilePermission.OWNER_EXECUTE)); + assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_WRITE)); + assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_EXECUTE)); + assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_READ)); + assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_WRITE)); + assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_EXECUTE)); } } From 73e2d27decb92123235cf89c4f18361a2eeb43d9 Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 11:38:24 -0800 Subject: [PATCH 6/7] Add 'final' to permissions variable in 'generateTmpFile' for clarity --- .../dataone/hashstore/filehashstore/FileHashStoreUtility.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java index ebd36806..eac28a1b 100644 --- a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java +++ b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java @@ -343,7 +343,7 @@ public static File generateTmpFile(String prefix, Path directory) File newTmpFile = newTmpPath.toFile(); // Set default file permissions 'rw- r-- ---' / posix 640 - Set permissions = new HashSet<>(); + final Set permissions = new HashSet<>(); permissions.add(PosixFilePermission.OWNER_READ); permissions.add(PosixFilePermission.OWNER_WRITE); permissions.add(PosixFilePermission.GROUP_READ); From 82ed48182889430ed0157f340f81d994460a717e Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Thu, 7 Nov 2024 11:42:30 -0800 Subject: [PATCH 7/7] Revise javadoc for 'generateTmpFile' and fix minor formatting issue with new junit tests --- .../hashstore/filehashstore/FileHashStoreUtility.java | 7 ++++--- .../filehashstore/FileHashStoreInterfaceTest.java | 3 ++- .../filehashstore/FileHashStoreProtectedTest.java | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java index eac28a1b..5c15c769 100644 --- a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java +++ b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java @@ -324,8 +324,9 @@ public static String getHierarchicalPathString(int depth, int width, String dige } /** - * Creates an empty/temporary file in a given location. If this file is not moved, it will be - * deleted upon JVM gracefully exiting or shutting down. + * Creates an empty/temporary file in a given location. This temporary file has the default + * permissions of 'rw- r-- ---' (owner read/write, and group read). If this file is not + * moved, it will be deleted upon JVM gracefully exiting or shutting down. * * @param prefix string to prepend before tmp file * @param directory location to create tmp file @@ -342,7 +343,7 @@ public static File generateTmpFile(String prefix, Path directory) Path newTmpPath = Files.createTempFile(directory, newPrefix, null); File newTmpFile = newTmpPath.toFile(); - // Set default file permissions 'rw- r-- ---' / posix 640 + // Set default file permissions 'rw- r-- ---' final Set permissions = new HashSet<>(); permissions.add(PosixFilePermission.OWNER_READ); permissions.add(PosixFilePermission.OWNER_WRITE); diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java index e9e63fd9..7bc157fb 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreInterfaceTest.java @@ -184,7 +184,8 @@ public void storeObject_filePermissions() throws Exception { expectedPermissions.add(PosixFilePermission.OWNER_WRITE); expectedPermissions.add(PosixFilePermission.GROUP_READ); - Set actualPermissions = Files.getPosixFilePermissions(objRealPath); + Set actualPermissions = + Files.getPosixFilePermissions(objRealPath); assertEquals(expectedPermissions, actualPermissions); assertFalse(actualPermissions.contains(PosixFilePermission.OWNER_EXECUTE)); diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java index 172a6082..64522145 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java @@ -2440,7 +2440,8 @@ public void fileHashStoreUtility_generateTmpFile_permissions() throws Exception expectedPermissions.add(PosixFilePermission.OWNER_WRITE); expectedPermissions.add(PosixFilePermission.GROUP_READ); - Set actualPermissions = Files.getPosixFilePermissions(tmpFile.toPath()); + Set actualPermissions = + Files.getPosixFilePermissions(tmpFile.toPath()); assertEquals(expectedPermissions, actualPermissions); assertFalse(actualPermissions.contains(PosixFilePermission.OWNER_EXECUTE));