search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.98k stars 17.67k forks source link

os: FileMode.String() generates a non-standard Mode representation #27452

Closed tep closed 1 year ago

tep commented 6 years ago

What version of Go are you using (go version)?

go version go1.11 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/tep/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/tep/go:/home/tep/working/go"
GOPROXY=""
GORACE=""
GOROOT="/misc/local/go"
GOTMPDIR=""
GOTOOLDIR="/misc/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build918873382=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/nDV8CK3VVrZ

What did you expect to see?

0 errors

What did you see instead?

FileMode(060000544).String() == "ugr-xr--r--"  ; Wanted "-r-sr-Sr--"
FileMode(024000544).String() == "gtr-xr--r--"  ; Wanted "-r-xr-Sr-T"
FileMode(040000444).String() == "ur--r--r--"   ; Wanted "-r-Sr--r--"
FileMode(060000444).String() == "ugr--r--r--"  ; Wanted "-r-Sr-Sr--"
FileMode(020000454).String() == "gr--r-xr--"   ; Wanted "-r--r-sr--"
FileMode(044000454).String() == "utr--r-xr--"  ; Wanted "-r-Sr-xr-T"
FileMode(044000445).String() == "utr--r--r-x"  ; Wanted "-r-Sr--r-t"
FileMode(020000544).String() == "gr-xr--r--"   ; Wanted "-r-xr-Sr--"
FileMode(004000454).String() == "tr--r-xr--"   ; Wanted "-r--r-xr-T"
FileMode(044000444).String() == "utr--r--r--"  ; Wanted "-r-Sr--r-T"
FileMode(064000544).String() == "ugtr-xr--r--" ; Wanted "-r-sr-Sr-T"
FileMode(064000445).String() == "ugtr--r--r-x" ; Wanted "-r-Sr-Sr-t"
FileMode(040000544).String() == "ur-xr--r--"   ; Wanted "-r-sr--r--"
FileMode(040000454).String() == "ur--r-xr--"   ; Wanted "-r-Sr-xr--"
FileMode(024000444).String() == "gtr--r--r--"  ; Wanted "-r--r-Sr-T"
FileMode(060000454).String() == "ugr--r-xr--"  ; Wanted "-r-Sr-sr--"
FileMode(020000444).String() == "gr--r--r--"   ; Wanted "-r--r-Sr--"
FileMode(020000445).String() == "gr--r--r-x"   ; Wanted "-r--r-Sr-x"
FileMode(004000444).String() == "tr--r--r--"   ; Wanted "-r--r--r-T"
FileMode(044000544).String() == "utr-xr--r--"  ; Wanted "-r-sr--r-T"
FileMode(024000454).String() == "gtr--r-xr--"  ; Wanted "-r--r-sr-T"
FileMode(060000445).String() == "ugr--r--r-x"  ; Wanted "-r-Sr-Sr-x"
FileMode(004000544).String() == "tr-xr--r--"   ; Wanted "-r-xr--r-T"
FileMode(004000445).String() == "tr--r--r-x"   ; Wanted "-r--r--r-t"
FileMode(064000444).String() == "ugtr--r--r--" ; Wanted "-r-Sr-Sr-T"
FileMode(064000454).String() == "ugtr--r-xr--" ; Wanted "-r-Sr-sr-T"
FileMode(040000445).String() == "ur--r--r-x"   ; Wanted "-r-Sr--r-x"
FileMode(024000445).String() == "gtr--r--r-x"  ; Wanted "-r--r-Sr-t"
28 errors

While never explicitly stated in the documentation, the format of the string generated by the os.FileMode String method implies that the author's intent is to closely mirror the common mode representation specified by the POSIX 1003.1 man page for ls(1).

With respect to os.ModeSetuid, os.ModeSetgid and os.ModeSticky - FileMode's String method deviates from the commonly expected behavior. The POSIX standard indicates that the values for these 3 bits are to be rendered as an overlay on the 3 groups of 3 characters that represent the file's permissions. The standard defines this representational behavior as:

The next three fields shall be three characters each:

  <owner permissions>

    Permissions  for  the  file  owner  class  (see  the  Base  Definitions
    volume  of IEEE Std 1003.1-2001, Section 4.4, File Access Permissions).

  <group permissions>

    Permissions for the file group class.

  <other permissions>

    Permissions for the file other class.

Each field shall have three character positions:

  1. If 'r', the file is readable; if '-', the file is not readable.

  2. If 'w', the file is writable; if '-', the file is not writable.

  3. The first of the following that applies:

    S   
      If in <owner permissions>, the file is not executable and 
      set-user-ID mode is set.

      If in <group permissions>, the file is not executable and 
      set-group-ID mode is set.

    s   
      If in <owner permissions>, the file is executable and 
      set-user-ID mode is set.

      If in <group permissions>, the file is executable and 
      set-group-ID mode is set.

    T   
      If in <other permissions> and the file is a directory,
      search permission is not granted to others, and the 
      restricted deletion flag is set.

    t   
      If in <other permissions> and the file is a directory,
      search permission is granted to others, and the restricted
      deletion flag is set.

    x   
      The file is executable or the directory is searchable.

    -   
      None of the attributes of 'S' , 's' , 'T' , 't' , or 'x' applies.

Implementations may add other characters to this list for the third character
position.  Such additions shall, however, be written in lowercase if the file
is executable or searchable, and in uppercase if it is not.

Instead of following this behavior, FileMode displays these values by prepending one or more characters to the 9 character permissions fields. This is the errant behavior.

Generating a string representation that, in the most common cases, appears to follow this generally accepted standard -- but then deviates with respect to the SUID, SGID and sticky bits -- can quickly lead to confusion on the part of the information consumer. This confusion can be eliminated by updating the String method to represent these bits in a more standard fashion.

odeke-em commented 6 years ago

Thank you for reporting this issue @tep and welcome to Go! I'll tag some folks who perhaps might be able to provide some insights into the design decisions or who might be helpful in fixing it /cc @rsc @bradfitz @ianlancetaylor @ericlagergren

ianlancetaylor commented 6 years ago

We're intentionally using a simpler approach to the mode bits, one that hopefully makes some sense on systems like Windows and Plan 9.

In any case I don't think we can change this now. It seems sure to break some currently working programs.

tepx commented 6 years ago

The cross-platform goal is perfectly understandable but, in this case, I don't really think it applies since none of ModeSetuid, ModeSetgid or ModeSticky are referenced by any of the Windows or Plan9 specific code. Changing the representation of these values would have no impact on those systems and, as far as I know, all remaining supported operating systems would benefit (since they are supposed to be POSIX compliant).

As to code-breakage -- I'd view this in a similar light to the adage that we should never depend on the string value of errors (but, I'm guessing the core Go team sees this differently).

Perhaps a minor update for v2?

ianlancetaylor commented 1 year ago

Even if we make a v2 of the os package, I can't see us changing this. All the information is available, so people who prefer a different representation can write that themselves. Closing this issue.