You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by ro...@apache.org on 2021/05/09 14:53:52 UTC
[felix-dev] branch master updated: [FELIX-6413] Make sure file
checksums account for write permissions
This is an automated email from the ASF dual-hosted git repository.
rotty3000 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git
The following commit(s) were added to refs/heads/master by this push:
new 7c160b6 [FELIX-6413] Make sure file checksums account for write permissions
7c160b6 is described below
commit 7c160b63546acf231ae8121b0bb4cc2bb67e369d
Author: Raymond Augé <ro...@apache.org>
AuthorDate: Sun May 9 10:50:34 2021 -0400
[FELIX-6413] Make sure file checksums account for write permissions
Signed-off-by: Raymond Augé <ro...@apache.org>
---
.../fileinstall/internal/ConfigInstaller.java | 95 ++++++++++++----------
.../apache/felix/fileinstall/internal/Scanner.java | 7 +-
.../apache/felix/fileinstall/internal/Util.java | 28 +++++++
3 files changed, 83 insertions(+), 47 deletions(-)
diff --git a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
index 3e11c29..78bd5cb 100644
--- a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
+++ b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
@@ -75,7 +75,6 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
private final Method removeAttributesMethod;
private final Method updateIfDifferentMethod;
private final Object READ_ONLY_ATTRIBUTE_ARRAY;
- private final boolean osWin;
private ServiceRegistration<?> registration;
ConfigInstaller(BundleContext context, ConfigurationAdmin configAdmin, FileInstall fileInstall)
@@ -135,7 +134,6 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
}
this.READ_ONLY_ATTRIBUTE_ARRAY = roAttributesArray;
- this.osWin = System.getProperty("os.name").toLowerCase().contains("win");
}
public void init()
@@ -378,7 +376,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
String pid[] = parsePid(f.getName());
Configuration config = getConfiguration(toConfigKey(f), pid[0], pid[1]);
- setReadOnly(pid, config, f);
+ clearReadOnlyIfWritable(pid, config, f);
Dictionary<String, Object> props = config.getProperties();
Hashtable<String, Object> old = props != null ? new Hashtable<String, Object>(new DictionaryAsMap<>(props)) : null;
@@ -388,24 +386,29 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
old.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
}
- if( !ht.equals( old ) )
- {
- ht.put(DirectoryWatcher.FILENAME, toConfigKey(f));
- if (old == null) {
- Util.log(context, Logger.LOG_INFO, "Creating configuration {" + pid[0]
- + (pid[1] == null ? "" : "-" + pid[1])
- + "} from " + f.getName(), null);
- } else {
- Util.log(context, Logger.LOG_INFO, "Updating configuration {" + pid[0]
- + (pid[1] == null ? "" : "-" + pid[1])
- + "} from " + f.getName(), null);
+ try {
+ if( !ht.equals( old ) )
+ {
+ ht.put(DirectoryWatcher.FILENAME, toConfigKey(f));
+ if (old == null) {
+ Util.log(context, Logger.LOG_INFO, "Creating configuration {" + pid[0]
+ + (pid[1] == null ? "" : "-" + pid[1])
+ + "} from " + f.getAbsolutePath(), null);
+ } else {
+ Util.log(context, Logger.LOG_INFO, "Updating configuration {" + pid[0]
+ + (pid[1] == null ? "" : "-" + pid[1])
+ + "} from " + f.getAbsolutePath(), null);
+ }
+ update0(config, ht);
+ return true;
+ }
+ else
+ {
+ return false;
}
- update0(config, ht);
- return true;
}
- else
- {
- return false;
+ finally {
+ setReadOnlyInNotWritable(pid, config, f);
}
}
@@ -422,7 +425,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
String pid[] = parsePid(f.getName());
Util.log(context, Logger.LOG_INFO, "Deleting configuration {" + pid[0]
+ (pid[1] == null ? "" : "-" + pid[1])
- + "} from " + f.getName(), null);
+ + "} from " + f.getAbsolutePath(), null);
Configuration config = getConfiguration(toConfigKey(f), pid[0], pid[1]);
config.delete();
return true;
@@ -534,28 +537,46 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
}
}
- boolean setReadOnly(String[] pid, Configuration configuration, File f) throws IOException {
+ void clearReadOnlyIfWritable(String[] pid, Configuration configuration, File f) throws IOException {
+ if (removeAttributesMethod == null) {
+ return;
+ }
+
+ try {
+ if (Util.canWrite(f) && isReadOnly(configuration)) {
+ Util.log(context, Logger.LOG_INFO, "Removing READ_ONLY attribute from configuration {" + pid[0]
+ + (pid[1] == null ? "" : "-" + pid[1])
+ + "} from " + f.getAbsolutePath(), null);
+ removeAttributesMethod.invoke(configuration, READ_ONLY_ATTRIBUTE_ARRAY);
+ }
+ }
+ catch (InvocationTargetException ite) {
+ Throwable targetException = ite.getTargetException();
+ if (targetException instanceof IOException) {
+ throw (IOException)targetException;
+ }
+
+ throw new RuntimeException(targetException);
+ }
+ catch (Throwable t) {
+ throw new RuntimeException(t);
+ }
+ }
+
+ boolean setReadOnlyInNotWritable(String[] pid, Configuration configuration, File f) throws IOException {
if (addAttributesMethod == null) {
return false;
}
try {
- if (!canWrite(f)) {
+ if (!Util.canWrite(f)) {
Util.log(context, Logger.LOG_INFO, "Adding READ_ONLY attribute to configuration {" + pid[0]
+ (pid[1] == null ? "" : "-" + pid[1])
- + "} from " + f.getName(), null);
+ + "} from " + f.getAbsolutePath(), null);
addAttributesMethod.invoke(configuration, READ_ONLY_ATTRIBUTE_ARRAY);
return true;
}
- else if (isReadOnly(configuration)) {
- Util.log(context, Logger.LOG_INFO, "Removing READ_ONLY attribute from configuration {" + pid[0]
- + (pid[1] == null ? "" : "-" + pid[1])
- + "} from " + f.getName(), null);
- removeAttributesMethod.invoke(configuration, READ_ONLY_ATTRIBUTE_ARRAY);
-
- return true;
- }
}
catch (InvocationTargetException ite) {
Throwable targetException = ite.getTargetException();
@@ -594,18 +615,6 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
}
}
- private boolean canWrite(File f) throws IOException {
- if (osWin) {
- return f.canWrite();
- }
-
- Set<PosixFilePermission> posixFilePermissions = Files.getPosixFilePermissions(f.toPath());
-
- return posixFilePermissions.contains(PosixFilePermission.OWNER_WRITE) ||
- posixFilePermissions.contains(PosixFilePermission.GROUP_WRITE) ||
- posixFilePermissions.contains(PosixFilePermission.OTHERS_WRITE);
- }
-
private String escapeFilterValue(String s) {
return s.replaceAll("[(]", "\\\\(").
replaceAll("[)]", "\\\\)").
diff --git a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Scanner.java b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Scanner.java
index a680b48..074c1ee 100644
--- a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Scanner.java
+++ b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Scanner.java
@@ -22,10 +22,8 @@ import java.io.Closeable;
import java.io.File;
import java.io.FilenameFilter;
import java.io.IOException;
-import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
@@ -119,7 +117,7 @@ public class Scanner implements Closeable {
* Modifications are checked against a computed checksum on some file
* attributes to detect any modification.
* Upon restart, such checksums are not known so that all files will
- * be reported as modified.
+ * be reported as modified.
*
* @param reportImmediately report all files immediately without waiting for the checksum to be stable
* @return a list of changes on the files included in the directory
@@ -147,7 +145,7 @@ public class Scanner implements Closeable {
if (skipSubdir)
{
continue;
- }
+ }
else if (recurseSubdir)
{
files.addAll(processFiles(reportImmediately, file.listFiles()));
@@ -243,6 +241,7 @@ public class Scanner implements Closeable {
{
checksum(file.lastModified(), crc);
checksum(file.length(), crc);
+ checksum(Util.collectWriteableChecksum(file), crc);
}
else if (file.isDirectory())
{
diff --git a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Util.java b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Util.java
index db924e5..344175a 100644
--- a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Util.java
+++ b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/Util.java
@@ -26,6 +26,8 @@ import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.attribute.PosixFilePermission;
import java.util.Collections;
import java.util.Set;
import java.util.jar.JarFile;
@@ -44,6 +46,8 @@ public class Util
{
private static final String CHECKSUM_SUFFIX = ".checksum";
+ private static final boolean OS_WIN = System.getProperty("os.name").toLowerCase().contains("win");
+
/**
* Returns the log level as defined in the BundleContext or System properties.
* @param context {@link BundleContext} of the FileInstall bundle.
@@ -377,6 +381,30 @@ public class Util
}
}
+ public static long collectWriteableChecksum(File file) {
+ return canWrite(file) ? 1000l : -1000l;
+ }
+
+ public static boolean canWrite(File f) {
+ if (!OS_WIN) {
+ try {
+ Set<PosixFilePermission> posixFilePermissions;
+ posixFilePermissions = Files.getPosixFilePermissions(f.toPath());
+
+ return posixFilePermissions.contains(PosixFilePermission.OWNER_WRITE) ||
+ posixFilePermissions.contains(PosixFilePermission.GROUP_WRITE) ||
+ posixFilePermissions.contains(PosixFilePermission.OTHERS_WRITE);
+ }
+ catch (IOException e) {
+ logger.log(
+ Logger.LOG_ERROR, Logger.LOG_ERROR,
+ "Could not get POSIX file permissions for " + f.getAbsolutePath(), e);
+ }
+ }
+
+ return f.canWrite();
+ }
+
private static String getBundleKey(Bundle b)
{
return Long.toString(b.getBundleId());