From d81529029a887cbc2356d6fc8d0168bd2f031ca2 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Fri, 15 Aug 2014 17:27:15 +0200 Subject: [PATCH] Set permission bits for "executable" attribute according to the umask Bug: 424395 Change-Id: I3f5c55dd4c084529af2319029305ba2e174e0636 Signed-off-by: Andrey Loskutov Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/util/FSJava7Test.java | 78 +++++++- .../org/eclipse/jgit/util/FS_POSIX_Java7.java | 174 +++++++++++++++++- .../src/org/eclipse/jgit/util/FileUtil.java | 8 + 3 files changed, 257 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.java7.test/src/org/eclipse/jgit/util/FSJava7Test.java b/org.eclipse.jgit.java7.test/src/org/eclipse/jgit/util/FSJava7Test.java index 70eaef231..91555f371 100644 --- a/org.eclipse.jgit.java7.test/src/org/eclipse/jgit/util/FSJava7Test.java +++ b/org.eclipse.jgit.java7.test/src/org/eclipse/jgit/util/FSJava7Test.java @@ -46,13 +46,21 @@ package org.eclipse.jgit.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeNotNull; +import static org.junit.Assume.assumeTrue; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import java.util.Set; import org.eclipse.jgit.junit.RepositoryTestCase; -import org.eclipse.jgit.util.FS; -import org.eclipse.jgit.util.FileUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -112,4 +120,70 @@ public class FSJava7Test { assertTrue(fs.canExecute(target)); } + @Test + public void testExecutableAttributes() throws Exception { + FS fs = FS.DETECTED; + // If this assumption fails the test is halted and ignored. + assumeTrue(fs instanceof FS_POSIX_Java7); + + File f = new File(trash, "bla"); + assertTrue(f.createNewFile()); + assertFalse(fs.canExecute(f)); + + String umask = readUmask(); + assumeNotNull(umask); + + char others = umask.charAt(umask.length() - 1); + + boolean badUmask; + if (others != '0' && others != '2' && others != '4' && others != '6') { + // umask is set in the way that "others" can not "execute" => git + // CLI will not set "execute" attribute for "others", so we also + // don't care + badUmask = true; + } else { + badUmask = false; + } + + Set permissions = readPermissions(f); + assertTrue(!permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); + assertTrue(!permissions.contains(PosixFilePermission.GROUP_EXECUTE)); + assertTrue(!permissions.contains(PosixFilePermission.OWNER_EXECUTE)); + + fs.setExecute(f, true); + + permissions = readPermissions(f); + assertTrue("'owner' execute permission not set", + permissions.contains(PosixFilePermission.OWNER_EXECUTE)); + assertTrue("'group' execute permission not set", + permissions.contains(PosixFilePermission.GROUP_EXECUTE)); + if (badUmask) { + assertFalse("'others' execute permission set", + permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); + System.err.println("WARNING: your system's umask: \"" + umask + + "\" doesn't allow FSJava7Test to test if setting posix" + + " permissions for \"others\" works properly"); + assumeFalse(badUmask); + } else { + assertTrue("'others' execute permission not set", + permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); + } + } + + private String readUmask() throws Exception { + Process p = Runtime.getRuntime().exec( + new String[] { "sh", "-c", "umask" }, null, null); + final BufferedReader lineRead = new BufferedReader( + new InputStreamReader(p.getInputStream(), Charset + .defaultCharset().name())); + p.waitFor(); + return lineRead.readLine(); + } + + private Set readPermissions(File f) throws IOException { + return Files + .getFileAttributeView(f.toPath(), PosixFileAttributeView.class) + .readAttributes().permissions(); + } + } diff --git a/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FS_POSIX_Java7.java b/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FS_POSIX_Java7.java index 0879e8021..4a73a9bcf 100644 --- a/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FS_POSIX_Java7.java +++ b/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FS_POSIX_Java7.java @@ -43,14 +43,55 @@ package org.eclipse.jgit.util; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.util.Set; /** * FS implementation for Java7 on unix like systems */ public class FS_POSIX_Java7 extends FS_POSIX { + /* + * True if the current user "umask" allows to set execute bit for "others". + * Can be null if "umask" is not supported (or returns unexpected values) by + * current user shell. + * + * Bug 424395: with the umask of 0002 (user: rwx group: rwx others: rx) egit + * checked out files as rwx,rwx,r (execution not allowed for "others"). To + * fix this and properly set "executable" permission bit for "others", we + * must consider the user umask on checkout + */ + private static final Boolean EXECUTE_FOR_OTHERS; + + /* + * True if the current user "umask" allows to set execute bit for "group". + * Can be null if "umask" is not supported (or returns unexpected values) by + * current user shell. + */ + private static final Boolean EXECUTE_FOR_GROUP; + + static { + String umask = readUmask(); + + // umask return value consists of 3 or 4 digits, like "002" or "0002" + if (umask != null && umask.length() > 0 && umask.matches("\\d{3,4}")) { //$NON-NLS-1$ + EXECUTE_FOR_OTHERS = isGranted(PosixFilePermission.OTHERS_EXECUTE, + umask); + EXECUTE_FOR_GROUP = isGranted(PosixFilePermission.GROUP_EXECUTE, + umask); + } else { + EXECUTE_FOR_OTHERS = null; + EXECUTE_FOR_GROUP = null; + } + } + FS_POSIX_Java7(FS_POSIX_Java7 src) { super(src); } @@ -76,7 +117,138 @@ public class FS_POSIX_Java7 extends FS_POSIX { @Override public boolean setExecute(File f, boolean canExecute) { - return FileUtil.setExecute(f, canExecute); + if (!isFile(f)) + return false; + // only if the execute has to be set, and we know the umask + if (canExecute && EXECUTE_FOR_OTHERS != null) { + try { + Path path = f.toPath(); + Set pset = Files + .getPosixFilePermissions(path); + // user is always allowed to set execute + pset.add(PosixFilePermission.OWNER_EXECUTE); + + if (EXECUTE_FOR_GROUP.booleanValue()) + pset.add(PosixFilePermission.GROUP_EXECUTE); + + if (EXECUTE_FOR_OTHERS.booleanValue()) + pset.add(PosixFilePermission.OTHERS_EXECUTE); + + Files.setPosixFilePermissions(path, pset); + return true; + } catch (IOException e) { + // The interface doesn't allow to throw IOException + final boolean debug = Boolean.parseBoolean(SystemReader + .getInstance().getProperty("jgit.fs.debug")); //$NON-NLS-1$ + if (debug) + System.err.println(e); + return false; + } + } + // if umask is not working for some reason: fall back to default (buggy) + // implementation which does not consider umask: see bug 424395 + return f.setExecutable(canExecute); + } + + /** + * Derives requested permission from given octal umask value as defined e.g. + * in http://linux.die.net/man/2/ + * umask. + *

+ * The umask expected here must consist of 3 or 4 digits. Last three digits + * are significant here because they represent file permissions granted to + * the "owner", "group" and "others" (in this order). + *

+ * Each single digit from the umask represents 3 bits of the mask standing + * for "read, write, execute" permissions (in this + * order). + *

+ * The possible umask values table: + * + * 

+	 * Value : Bits:Abbr.: Permission
+	 *     0 : 000 :rwx  : read, write and execute
+	 *     1 : 001 :rw   : read and write
+	 *     2 : 010 :rx   : read and execute
+	 *     3 : 011 :r    : read only
+	 *     4 : 100 :wx   : write and execute
+	 *     5 : 101 :w    : write only
+	 *     6 : 110 :x    : execute only
+	 *     7 : 111 :     : no permissions
+	 *
+ *

+ * Note, that umask value is used to "mask" the requested permissions on + * file creation by combining the requested permission bit with the + * negated value of the umask bit. + *

+ * Simply speaking, if a bit is not set in the umask, then the + * appropriate right will be granted if requested. If a bit is + * set in the umask value, then the appropriate permission will be not + * granted. + *

+ * Example: + *

  • umask 023 ("000 010 011" or rwx rx r) combined with the request to + * create an executable file with full set of permissions for everyone (777) + * results in the file with permissions 754 (rwx rx r). + *
  • umask 002 ("000 000 010" or rwx rwx rx) combined with the request to + * create an executable file with full set of permissions for everyone (777) + * results in the file with permissions 775 (rwx rwx rx). + *
  • umask 002 ("000 000 010" or rwx rwx rx) combined with the request to + * create a file without executable rights for everyone (666) results in the + * file with permissions 664 (rw rw r). + * + * @param p + * non null permission + * @param umask + * octal umask value represented by at least three digits. The + * digits (read from the end to beginning of the umask) represent + * permissions for "others", "group" and "owner". + * + * @return true if the requested permission is set according to given umask + */ + private static Boolean isGranted(PosixFilePermission p, String umask) { + char val; + switch (p) { + case OTHERS_EXECUTE: + // Read last digit, because umask is ordered as: User/Group/Others. + val = umask.charAt(umask.length() - 1); + return isExecuteGranted(val); + case GROUP_EXECUTE: + val = umask.charAt(umask.length() - 2); + return isExecuteGranted(val); + default: + throw new UnsupportedOperationException( + "isGranted() for " + p + " is not implemented!"); //$NON-NLS-1$ //$NON-NLS-2$ + } + } + + /** + * @param c + * character representing octal permission value from the table + * in {@link #isGranted(PosixFilePermission, String)} + * @return true if the "execute" permission is granted according to given + * character + */ + private static Boolean isExecuteGranted(char c) { + if (c == '0' || c == '2' || c == '4' || c == '6') + return Boolean.TRUE; + return Boolean.FALSE; + } + + private static String readUmask() { + Process p; + try { + p = Runtime.getRuntime().exec( + new String[] { "sh", "-c", "umask" }, null, null); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + try (BufferedReader lineRead = new BufferedReader( + new InputStreamReader(p.getInputStream(), Charset + .defaultCharset().name()))) { + p.waitFor(); + return lineRead.readLine(); + } + } catch (Exception e) { + return null; + } } @Override diff --git a/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FileUtil.java b/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FileUtil.java index c958494d0..67fcc9263 100644 --- a/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FileUtil.java +++ b/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FileUtil.java @@ -152,6 +152,14 @@ class FileUtil { return path.canExecute(); } + /** + * @param path + * @param executable + * @return true if succeeded, false if not supported or failed + * @deprecated the implementation is highly platform dependent, consider + * using {@link FS#setExecute(File, boolean)} instead + */ + @Deprecated public static boolean setExecute(File path, boolean executable) { if (!isFile(path)) return false;