Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
tep opened this issue Sep 2, 2018 · 4 comments
Closed

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

tep opened this issue Sep 2, 2018 · 4 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. v2 A language change or incompatible library change
Milestone
Unplanned

Comments

@tep
Copy link

tep commented Sep 2, 2018

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 odeke-em changed the title FileMode(m).String() Generates a non-standard Mode Representation os: FileMode.String() generates a non-standard Mode representation Sep 2, 2018
@odeke-em odeke-em added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 2, 2018
@odeke-em
Copy link
Member

odeke-em commented Sep 2, 2018

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
Copy link
Contributor

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
Copy link

tepx commented Sep 4, 2018

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?

@ALTree ALTree added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. v2 A language change or incompatible library change and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 22, 2020
@kevlar700 kevlar700 mentioned this issue Feb 24, 2021
Open
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
@rsc rsc mentioned this issue Mar 15, 2023
Closed
@ianlancetaylor
Copy link
Contributor

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.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. v2 A language change or incompatible library change
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@ianlancetaylor @ALTree @odeke-em @tep @seankhliao @tepx