diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java index 9fba374dc..e6a244e14 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java @@ -46,15 +46,10 @@ 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; @@ -123,29 +118,15 @@ public class FSJava7Test { @Test public void testExecutableAttributes() throws Exception { - FS fs = FS.DETECTED; + FS fs = FS.DETECTED.newInstance(); // If this assumption fails the test is halted and ignored. assumeTrue(fs instanceof FS_POSIX); + ((FS_POSIX) fs).setUmask(0022); 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)); @@ -158,27 +139,21 @@ public class FSJava7Test { 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)); - } - } + assertTrue("'others' execute permission not set", + permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); + + ((FS_POSIX) fs).setUmask(0033); + fs.setExecute(f, false); + assertFalse(fs.canExecute(f)); + fs.setExecute(f, true); - 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(); + permissions = readPermissions(f); + assertTrue("'owner' execute permission not set", + permissions.contains(PosixFilePermission.OWNER_EXECUTE)); + assertFalse("'group' execute permission set", + permissions.contains(PosixFilePermission.GROUP_EXECUTE)); + assertFalse("'others' execute permission set", + permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); } private Set readPermissions(File f) throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 47760f661..9c92d7c4c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -66,140 +66,70 @@ import org.eclipse.jgit.lib.Repository; * @since 3.0 */ public class FS_POSIX extends FS { + private static final int DEFAULT_UMASK = 0022; + private volatile int umask = -1; - 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; - } + /** Default constructor. */ + protected FS_POSIX() { } /** - * @since 4.0 - */ - protected static final Boolean EXECUTE_FOR_OTHERS; - - /** - * @since 4.0 + * Constructor + * + * @param src + * FS to copy some settings from */ - protected static final Boolean EXECUTE_FOR_GROUP; + protected FS_POSIX(FS src) { + super(src); + if (src instanceof FS_POSIX) { + umask = ((FS_POSIX) src).umask; + } + } @Override public FS newInstance() { - return new FS_POSIX(); + return new FS_POSIX(this); } /** - * 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). + * Set the umask, overriding any value observed from the shell. * - * @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 + * mask to apply when creating files. * @since 4.0 */ - protected 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$ - } + public void setUmask(int umask) { + this.umask = umask; } - /** - * @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 int umask() { + int u = umask; + if (u == -1) { + u = readUmask(); + umask = u; + } + return u; } - /** - * @return umask returned from running umask command in a shell - * @since 4.0 - */ - protected static String readUmask() { - Process p; + /** @return mask returned from running {@code umask} command in shell. */ + private static int readUmask() { try { - p = Runtime.getRuntime().exec( - new String[] { "sh", "-c", "umask" }, null, null); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + Process p = Runtime.getRuntime().exec( + new String[] { "sh", "-c", "umask" }, //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + null, null); try (BufferedReader lineRead = new BufferedReader( new InputStreamReader(p.getInputStream(), Charset .defaultCharset().name()))) { - p.waitFor(); - return lineRead.readLine(); + if (p.waitFor() == 0) { + String s = lineRead.readLine(); + if (s.matches("0?\\d{3}")) { //$NON-NLS-1$ + return Integer.parseInt(s, 8); + } + } + return DEFAULT_UMASK; } } catch (Exception e) { - return null; + return DEFAULT_UMASK; } } @@ -229,23 +159,6 @@ public class FS_POSIX extends FS { return null; } - /** - * Default constructor - */ - protected FS_POSIX() { - super(); - } - - /** - * Constructor - * - * @param src - * FS to copy some settings from - */ - protected FS_POSIX(FS src) { - super(src); - } - @Override public boolean isCaseSensitive() { return !SystemReader.getInstance().isMacOS(); @@ -265,35 +178,40 @@ public class FS_POSIX extends FS { public boolean setExecute(File f, boolean 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 (!canExecute) + return f.setExecutable(false); + + try { + Path path = f.toPath(); + Set pset = Files.getPosixFilePermissions(path); + + // owner (user) is always allowed to execute. + pset.add(PosixFilePermission.OWNER_EXECUTE); + + int mask = umask(); + apply(pset, mask, PosixFilePermission.GROUP_EXECUTE, 1 << 3); + apply(pset, mask, PosixFilePermission.OTHERS_EXECUTE, 1); + 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; + } + } + + private static void apply(Set set, + int umask, PosixFilePermission perm, int test) { + if ((umask & test) == 0) { + // If bit is clear in umask, permission is allowed. + set.add(perm); + } else { + // If bit is set in umask, permission is denied. + set.remove(perm); } - // 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); } @Override