From 62244f01c24cec05ceaf6d3bcaf40808091da3f5 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sat, 25 Mar 2017 22:25:33 -0700 Subject: [PATCH] fix `umask` handling of symbolic modes This fixes the handling of symbolic umask values. It also removes two invocations of `perl` and all but two `math` commands. Fixes #738 --- CHANGELOG.md | 1 + share/functions/umask.fish | 226 +++++++++++++++++++------------------ tests/umask.err | 3 + tests/umask.in | 84 ++++++++++++++ tests/umask.out | 39 +++++++ 5 files changed, 242 insertions(+), 111 deletions(-) create mode 100644 tests/umask.err create mode 100644 tests/umask.in create mode 100644 tests/umask.out diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b9a3006..30d45fcb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Empty CDPATH elements are now equivalent to "." (#2106). - The `read` command now accepts simple strings for the prompt rather than fish script via the new `-P` and `--prompt-str` flags (#802). - `type` now no longer requires `which`, which means fish no longer uses it anywhere. Packagers should remove the dependency (#3912). +- Using symbolic permissions with the `umask` command now works (#738). --- diff --git a/share/functions/umask.fish b/share/functions/umask.fish index 4ad547818..9919c6da1 100644 --- a/share/functions/umask.fish +++ b/share/functions/umask.fish @@ -1,117 +1,129 @@ +# Support the usual (i.e., bash compatible) `umask` UI. This reports or modifies the magic global +# `umask` variable which is monitored by the fish process. -function __fish_umask_parse -d "Internal umask function" - # Test if already a valid octal mask, and pad it with zeros - if echo $argv | __fish_sgrep -E '^0?[0-7]{1,3}$' >/dev/null - set -l char_count (echo $argv| wc -c) - for i in (seq (math 5 - $char_count)) - set argv 0$argv - end - echo $argv - else - # Test if argument really is a valid symbolic mask - if not echo $argv | __fish_sgrep -E '^(((u|g|o|a|)(=|\+|-)|)(r|w|x)*)(,(((u|g|o|a|)(=|\+|-)|)(r|w|x)*))*$' >/dev/null +# This table is indexed by the base umask value to be modified. Each digit represents the new umask +# value when the permissions to add are applied to the base umask value. +set __fish_umask_add_table 0101010 2002200 2103210 4440000 4541010 6442200 6543210 + +function __fish_umask_add + set -l mask_digit $argv[1] + set -l to_add $argv[2] + + set -l mask_table 0000000 + if test $mask_digit -gt 0 + set mask_table $__fish_umask_add_table[$mask_digit] + end + set -l new_vals (string split '' $mask_table) + echo $new_vals[$to_add] +end + +# This table is indexed by the base umask value to be modified. Each digit represents the new umask +# value when the permissions to remove are applied to the base umask value. +set __fish_umask_remove_table 1335577 3236767 3337777 5674567 5775577 7676767 7777777 + +function __fish_umask_remove + set -l mask_digit $argv[1] + set -l to_remove $argv[2] + + set -l mask_table 1234567 + if test $mask_digit -gt 0 + set mask_table $__fish_umask_remove_table[$mask_digit] + end + set -l new_vals (string split '' $mask_table) + echo $new_vals[$to_remove] +end + +# This returns the mask corresponding to allowing the permissions to allow. In other words it +# returns the inverse of the mask passed in. +set __fish_umask_set_table 6 5 4 3 2 1 0 +function __fish_umask_set + set -l to_set $argv[1] + if test $to_set -eq 0 + return 7 + end + echo $__fish_umask_set_table[$to_set] +end + +# Given a umask string, possibly in symbolic mode, return an octal value with leading zeros. +# This function expects to be called with a single value. +function __fish_umask_parse + # Test if already a valid octal mask. If so pad it with zeros and return it. + # Note that umask values are always base 8 so they don't require a leading zero. + if string match -qr '^0?[0-7]{1,3}$' -- $argv + string sub -s -4 0000$argv + return 0 + end + + # Test if argument is a valid symbolic mask. Note that the basic pattern allows one illegal + # pattern: who and perms without a mode such as "urw". We test for that below after using the + # pattern to split the rights then testing for that invalid combination. + set -l basic_pattern '([ugoa]*)([=+-]?)([rwx]*)' + if not string match -qr "^$basic_pattern(,$basic_pattern)*\$" -- $argv + printf (_ "%s: Invalid mask '%s'\n") umask $argv >&2 + return 1 + end + + # Split umask into individual digits. We erase the first one because it should always be zero. + set -l res (string split '' $umask) + set -e res[1] + + for rights in (string split , $argv) + set -l match (string match -r "^$basic_pattern\$" $rights) + set -l scope $match[2] + set -l mode $match[3] + set -l perms $match[4] + if test -n "$scope" -a -z "$mode" printf (_ "%s: Invalid mask '%s'\n") umask $argv >&2 return 1 end - - set -l implicit_all - - # Insert inverted umask into res variable - - set -l mode - set -l val - set -l tmp $umask - set -l res - - for i in 1 2 3 - set tmp (echo $tmp|cut -c 2-) - set -l char_count (echo $tmp|cut -c 1) - set res[$i] (math 7 - $char_count) + if test -z "$scope" + set scope a + end + if test -z "$mode" + set mode = end - set -l el (echo $argv|tr , \n) - for i in $el - switch $i - case 'u*' - set idx 1 - set i (echo $i| cut -c 2-) + set -l scopes_to_modify + string match -q '*u*' $scope + and set scopes_to_modify 1 + string match -q '*g*' $scope + and set scopes_to_modify $scopes_to_modify 2 + string match -q '*o*' $scope + and set scopes_to_modify $scopes_to_modify 3 + string match -q '*a*' $scope + and set scopes_to_modify 1 2 3 - case 'g*' - set idx 2 - set i (echo $i| cut -c 2-) - - case 'o*' - set idx 3 - set i (echo $i| cut -c 2-) - - case 'a*' - set idx 1 2 3 - set i (echo $i| cut -c 2-) - - case '*' - set implicit_all 1 - set idx 1 2 3 - end - - switch $i - case '=*' - set mode set - set i (echo $i| cut -c 2-) - - case '+*' - set mode add - set i (echo $i| cut -c 2-) - - case '-*' - set mode remove - set i (echo $i| cut -c 2-) - - case '*' - if not count $implicit_all >/dev/null - printf (_ "%s: Invalid mask '%s'\n") umask $argv >&2 - return - end - set mode set - end - - if not echo $perm | __fish_sgrep -E '^(r|w|x)*$' >/dev/null - printf (_ "%s: Invalid mask '%s'\n") umask $argv >&2 - return - end - - set val 0 - if echo $i | __fish_sgrep 'r' >/dev/null - set val 4 - end - if echo $i | __fish_sgrep 'w' >/dev/null - set val (math $val + 2) - end - if echo $i | __fish_sgrep 'x' >/dev/null - set val (math $val + 1) - end - - for j in $idx - switch $mode - case set - set res[$j] $val - - case add - set res[$j] (perl -e 'print( ( '$res[$j]'|'$val[$j]' )."\n" )') - - case remove - set res[$j] (perl -e 'print( ( (7-'$res[$j]')&'$val[$j]' )."\n" )') - end - end + set -l val 0 + if string match -q '*r*' $perms + set val 4 + end + if string match -q '*w*' $perms + set val (math $val + 2) + end + if string match -q '*x*' $perms + set val (math $val + 1) end - for i in 1 2 3 - set res[$i] (math 7 - $res[$i]) + for j in $scopes_to_modify + switch $mode + case '=' + set res[$j] (__fish_umask_set $val) + + case '+' + set res[$j] (__fish_umask_add $res[$j] $val) + + case '-' + set res[$j] (__fish_umask_remove $res[$j] $val) + end end - echo 0$res[1]$res[2]$res[3] end + + echo 0$res[1]$res[2]$res[3] + return 0 end function __fish_umask_print_symbolic + set -l val set -l res "" set -l letter a u g o @@ -133,14 +145,12 @@ function __fish_umask_print_symbolic end - echo $res | cut -c 2- + echo (string split -m 1 '' -- $res)[2] end function umask --description "Set default file permission mask" - set -l as_command 0 set -l symbolic 0 - set -l options set -l shortopt pSh if not getopt -T >/dev/null @@ -164,7 +174,6 @@ function umask --description "Set default file permission mask" eval set opt $tmp while count $opt >/dev/null - switch $opt[1] case -h --help __fish_print_help umask @@ -179,15 +188,12 @@ function umask --description "Set default file permission mask" case -- set -e opt[1] break - end set -e opt[1] - end switch (count $opt) - case 0 if not set -q umask set -g umask 113 @@ -203,8 +209,7 @@ function umask --description "Set default file permission mask" end case 1 - set -l parsed (__fish_umask_parse $opt) - if test (count $parsed) -eq 1 + if set -l parsed (__fish_umask_parse $opt) set -g umask $parsed return 0 end @@ -212,7 +217,6 @@ function umask --description "Set default file permission mask" case '*' printf (_ '%s: Too many arguments\n') umask >&2 - + return 1 end - end diff --git a/tests/umask.err b/tests/umask.err new file mode 100644 index 000000000..b32c3fb97 --- /dev/null +++ b/tests/umask.err @@ -0,0 +1,3 @@ +umask: Invalid mask '1234' +umask: Invalid mask '228' +umask: Invalid mask '0282' diff --git a/tests/umask.in b/tests/umask.in new file mode 100644 index 000000000..efeed6366 --- /dev/null +++ b/tests/umask.in @@ -0,0 +1,84 @@ +# Test the umask command. In particular the symbolic modes since they've been +# broken for four years (see issue #738) at the time I added these tests. + +# Establish a base line umask. +umask 027 +umask +echo umask var = $umask +umask -S + +# Verify that an invalid umask is rejected +umask 1234 +umask 228 +umask 0282 + +# Verify that symbolic modifications and output is correct. +# +# When I wrote these tests I based all of the results on the behavior of bash +# when executing identical commands. So if bash has a bug with the umask +# command it's possible fish will as well. However, I did verify the result of +# each interaction and did not find any bugs in how bash or fish handled these +# scenarios. +# +echo +echo running umask a-r +umask 0777 +umask a-r +umask +umask -S + +echo +echo running umask u+x +umask 0777 +umask u+x +umask +umask -S + +echo +echo running umask g+rwx,o+x +umask 777 +umask g+rwx,o+x +umask +umask -S + +echo +echo running umask u-w,o-x +umask 0 +umask u-w,o-x +umask +umask -S + +echo +echo running umask a-r +umask 0 +umask a-r +umask +umask -S + +echo +echo running umask ug-rx +umask 0 +umask ug-rx +umask +umask -S + +echo +echo running umask u+r,g+w,o=rw +umask 777 +umask u+r,g+w,o=rw +umask +umask -S + +echo +echo running umask =r,g+w,o+x,o-r +umask 777 +umask =r,g+w,o+x,o-r +umask +umask -S + +echo +echo running umask rx +umask 0 +umask rx +umask +umask -S diff --git a/tests/umask.out b/tests/umask.out new file mode 100644 index 000000000..69de0dfb4 --- /dev/null +++ b/tests/umask.out @@ -0,0 +1,39 @@ +0027 +umask var = 0027 +u=rwx,g=rx,o= + +running umask a-r +0777 +u=,g=,o= + +running umask u+x +0677 +u=x,g=,o= + +running umask g+rwx,o+x +0706 +u=,g=rwx,o=x + +running umask u-w,o-x +0201 +u=rx,g=rwx,o=rw + +running umask a-r +0444 +u=wx,g=wx,o=wx + +running umask ug-rx +0550 +u=w,g=w,o=rwx + +running umask u+r,g+w,o=rw +0351 +u=r,g=w,o=rw + +running umask =r,g+w,o+x,o-r +0316 +u=r,g=rw,o=x + +running umask rx +0222 +u=rx,g=rx,o=rx