cmd/internal/moddeps: use filepath.SkipDir only on directories

If a filepath.WalkFunc returns filepath.SkipDir when invoked on a
non-directory file, it skips the remaining files in the containing
directory.¹

CL 276272 accidentally added a code path that triggers this behavior
whenever filepath.Walk reaches a non-directory file that begins with
a dot, such as .gitattributes or .DS_Store, causing findGorootModules
to return early without finding any modules in GOROOT. Tests that use
it ceased to provide test coverage that the tree is tidy.

Add an explicit check for info.IsDir in the 5 places that intend to
use filepath.SkipDir to skip traversing that directory. Even paths
like GOROOT/bin and GOROOT/pkg which are unlikely to be anything but
a directory are worth checking, since the goal of moddeps is to take
a possibly problematic GOROOT tree as input and detect problems.

While the goal of findGorootModules is to find all modules in GOROOT
programmatically (in case new modules are added or modified), there
are 4 modules now that are quite likely to exist, so check for their
presence to avoid similar regressions. (It's not hard to update this
test if a well-known GOROOT module is removed or otherwise modified;
but if it becomes hard we can simplify it to check for a reasonable
number of modules instead.)

Also fix the minor skew that has crept in since the test got disabled.

¹ This wasn't necessarily an intentional design decision, but it was
  found only when Go 1.4 was already out. See CL 11690 for details.

Fixes #46254.

Change-Id: Id55ed926f8c0094b1af923070de72bacca05996f
Reviewed-on: https://go-review.googlesource.com/c/go/+/320991
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This commit is contained in:
Dmitri Shuralyov 2021-05-18 23:29:14 -04:00
parent 658b5e66ec
commit 6c1c055d1e
8 changed files with 137 additions and 48 deletions

View file

@ -227,7 +227,7 @@ func makeGOROOTCopy(t *testing.T) string {
if err != nil {
return err
}
if src == filepath.Join(runtime.GOROOT(), ".git") {
if info.IsDir() && src == filepath.Join(runtime.GOROOT(), ".git") {
return filepath.SkipDir
}
@ -237,9 +237,8 @@ func makeGOROOTCopy(t *testing.T) string {
}
dst := filepath.Join(gorootCopyDir, rel)
switch src {
case filepath.Join(runtime.GOROOT(), "bin"),
filepath.Join(runtime.GOROOT(), "pkg"):
if info.IsDir() && (src == filepath.Join(runtime.GOROOT(), "bin") ||
src == filepath.Join(runtime.GOROOT(), "pkg")) {
// If the OS supports symlinks, use them instead
// of copying the bin and pkg directories.
if err := os.Symlink(src, dst); err == nil {
@ -414,7 +413,7 @@ func findGorootModules(t *testing.T) []gorootModule {
if info.IsDir() && (info.Name() == "vendor" || info.Name() == "testdata") {
return filepath.SkipDir
}
if path == filepath.Join(runtime.GOROOT(), "pkg") {
if info.IsDir() && path == filepath.Join(runtime.GOROOT(), "pkg") {
// GOROOT/pkg contains generated artifacts, not source code.
//
// In https://golang.org/issue/37929 it was observed to somehow contain
@ -422,7 +421,7 @@ func findGorootModules(t *testing.T) []gorootModule {
// running time of this test anyway.)
return filepath.SkipDir
}
if strings.HasPrefix(info.Name(), "_") || strings.HasPrefix(info.Name(), ".") {
if info.IsDir() && (strings.HasPrefix(info.Name(), "_") || strings.HasPrefix(info.Name(), ".")) {
// _ and . prefixed directories can be used for internal modules
// without a vendor directory that don't contribute to the build
// but might be used for example as code generators.
@ -457,8 +456,31 @@ func findGorootModules(t *testing.T) []gorootModule {
goroot.modules = append(goroot.modules, m)
return nil
})
})
if goroot.err != nil {
return
}
// knownGOROOTModules is a hard-coded list of modules that are known to exist in GOROOT.
// If findGorootModules doesn't find a module, it won't be covered by tests at all,
// so make sure at least these modules are found. See issue 46254. If this list
// becomes a nuisance to update, can be replaced with len(goroot.modules) check.
knownGOROOTModules := [...]string{
"std",
"cmd",
"misc",
"test/bench/go1",
}
var seen = make(map[string]bool) // Key is module path.
for _, m := range goroot.modules {
seen[m.Path] = true
}
for _, m := range knownGOROOTModules {
if !seen[m] {
goroot.err = fmt.Errorf("findGorootModules didn't find the well-known module %q", m)
break
}
}
})
if goroot.err != nil {
t.Fatal(goroot.err)
}

View file

@ -5,6 +5,6 @@ go 1.17
require (
golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e
golang.org/x/net v0.0.0-20210510120150-4163338589ed
golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6 // indirect
golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 // indirect
golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f // indirect
)

View file

@ -2,7 +2,7 @@ golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e h1:8foAy0aoO5GkqCvAEJ4VC4
golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8=
golang.org/x/net v0.0.0-20210510120150-4163338589ed h1:p9UgmWI9wKpfYmgaV/IZKGdXc5qEK45tDwwwDyjS26I=
golang.org/x/net v0.0.0-20210510120150-4163338589ed/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6 h1:cdsMqa2nXzqlgs183pHxtvoVwU7CyzaCTAUOg94af4c=
golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 h1:yhBbb4IRs2HS9PPlAg6DMC6mUOKexJBNsLf4Z+6En1Q=
golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f h1:yQJrRE0hDxDFmZLlRaw+3vusO4fwNHgHIjUOMO7bHYI=
golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=

View file

@ -53,6 +53,48 @@ import (
"golang.org/x/net/idna"
)
// asciiEqualFold is strings.EqualFold, ASCII only. It reports whether s and t
// are equal, ASCII-case-insensitively.
func http2asciiEqualFold(s, t string) bool {
if len(s) != len(t) {
return false
}
for i := 0; i < len(s); i++ {
if http2lower(s[i]) != http2lower(t[i]) {
return false
}
}
return true
}
// lower returns the ASCII lowercase version of b.
func http2lower(b byte) byte {
if 'A' <= b && b <= 'Z' {
return b + ('a' - 'A')
}
return b
}
// isASCIIPrint returns whether s is ASCII and printable according to
// https://tools.ietf.org/html/rfc20#section-4.2.
func http2isASCIIPrint(s string) bool {
for i := 0; i < len(s); i++ {
if s[i] < ' ' || s[i] > '~' {
return false
}
}
return true
}
// asciiToLower returns the lowercase version of s if s is ASCII and printable,
// and whether or not it was.
func http2asciiToLower(s string) (lower string, ok bool) {
if !http2isASCIIPrint(s) {
return "", false
}
return strings.ToLower(s), true
}
// A list of the possible cipher suite ids. Taken from
// https://www.iana.org/assignments/tls-parameters/tls-parameters.txt
@ -2907,6 +2949,20 @@ func http2traceGot1xxResponseFunc(trace *httptrace.ClientTrace) func(int, textpr
return nil
}
// dialTLSWithContext uses tls.Dialer, added in Go 1.15, to open a TLS
// connection.
func (t *http2Transport) dialTLSWithContext(ctx context.Context, network, addr string, cfg *tls.Config) (*tls.Conn, error) {
dialer := &tls.Dialer{
Config: cfg,
}
cn, err := dialer.DialContext(ctx, network, addr)
if err != nil {
return nil, err
}
tlsCn := cn.(*tls.Conn) // DialContext comment promises this will always succeed
return tlsCn, nil
}
var http2DebugGoroutines = os.Getenv("DEBUG_HTTP2_GOROUTINES") == "1"
type http2goroutineLock uint64
@ -3128,12 +3184,12 @@ func http2buildCommonHeaderMaps() {
}
}
func http2lowerHeader(v string) string {
func http2lowerHeader(v string) (lower string, ascii bool) {
http2buildCommonHeaderMapsOnce()
if s, ok := http2commonLowerHeader[v]; ok {
return s
return s, true
}
return strings.ToLower(v)
return http2asciiToLower(v)
}
var (
@ -3831,13 +3887,12 @@ func http2ConfigureServer(s *Server, conf *http2Server) error {
if s.TLSConfig == nil {
s.TLSConfig = new(tls.Config)
} else if s.TLSConfig.CipherSuites != nil {
// If they already provided a CipherSuite list, return
// an error if it has a bad order or is missing
// ECDHE_RSA_WITH_AES_128_GCM_SHA256 or ECDHE_ECDSA_WITH_AES_128_GCM_SHA256.
} else if s.TLSConfig.CipherSuites != nil && s.TLSConfig.MinVersion < tls.VersionTLS13 {
// If they already provided a TLS 1.01.2 CipherSuite list, return an
// error if it is missing ECDHE_RSA_WITH_AES_128_GCM_SHA256 or
// ECDHE_ECDSA_WITH_AES_128_GCM_SHA256.
haveRequired := false
sawBad := false
for i, cs := range s.TLSConfig.CipherSuites {
for _, cs := range s.TLSConfig.CipherSuites {
switch cs {
case tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
// Alternative MTI cipher to not discourage ECDSA-only servers.
@ -3845,14 +3900,9 @@ func http2ConfigureServer(s *Server, conf *http2Server) error {
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:
haveRequired = true
}
if http2isBadCipher(cs) {
sawBad = true
} else if sawBad {
return fmt.Errorf("http2: TLSConfig.CipherSuites index %d contains an HTTP/2-approved cipher suite (%#04x), but it comes after unapproved cipher suites. With this configuration, clients that don't support previous, approved cipher suites may be given an unapproved one and reject the connection.", i, cs)
}
}
if !haveRequired {
return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256).")
return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)")
}
}
@ -6394,8 +6444,12 @@ func (w *http2responseWriter) Push(target string, opts *PushOptions) error {
// but PUSH_PROMISE requests cannot have a body.
// http://tools.ietf.org/html/rfc7540#section-8.2
// Also disallow Host, since the promised URL must be absolute.
switch strings.ToLower(k) {
case "content-length", "content-encoding", "trailer", "te", "expect", "host":
if http2asciiEqualFold(k, "content-length") ||
http2asciiEqualFold(k, "content-encoding") ||
http2asciiEqualFold(k, "trailer") ||
http2asciiEqualFold(k, "te") ||
http2asciiEqualFold(k, "expect") ||
http2asciiEqualFold(k, "host") {
return fmt.Errorf("promised request headers cannot include %q", k)
}
}
@ -7148,14 +7202,10 @@ func (t *http2Transport) dialTLS(ctx context.Context) func(string, string, *tls.
return t.DialTLS
}
return func(network, addr string, cfg *tls.Config) (net.Conn, error) {
dialer := &tls.Dialer{
Config: cfg,
}
cn, err := dialer.DialContext(ctx, network, addr)
tlsCn, err := t.dialTLSWithContext(ctx, network, addr, cfg)
if err != nil {
return nil, err
}
tlsCn := cn.(*tls.Conn) // DialContext comment promises this will always succeed
state := tlsCn.ConnectionState()
if p := state.NegotiatedProtocol; p != http2NextProtoTLS {
return nil, fmt.Errorf("http2: unexpected ALPN protocol %q; want %q", p, http2NextProtoTLS)
@ -7163,7 +7213,7 @@ func (t *http2Transport) dialTLS(ctx context.Context) func(string, string, *tls.
if !state.NegotiatedProtocolIsMutual {
return nil, errors.New("http2: could not negotiate protocol mutually")
}
return cn, nil
return tlsCn, nil
}
}
@ -7552,7 +7602,7 @@ func http2checkConnHeaders(req *Request) error {
if vv := req.Header["Transfer-Encoding"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && vv[0] != "chunked") {
return fmt.Errorf("http2: invalid Transfer-Encoding request header: %q", vv)
}
if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && !strings.EqualFold(vv[0], "close") && !strings.EqualFold(vv[0], "keep-alive")) {
if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && !http2asciiEqualFold(vv[0], "close") && !http2asciiEqualFold(vv[0], "keep-alive")) {
return fmt.Errorf("http2: invalid Connection request header: %q", vv)
}
return nil
@ -8078,19 +8128,21 @@ func (cc *http2ClientConn) encodeHeaders(req *Request, addGzipHeader bool, trail
var didUA bool
for k, vv := range req.Header {
if strings.EqualFold(k, "host") || strings.EqualFold(k, "content-length") {
if http2asciiEqualFold(k, "host") || http2asciiEqualFold(k, "content-length") {
// Host is :authority, already sent.
// Content-Length is automatic, set below.
continue
} else if strings.EqualFold(k, "connection") || strings.EqualFold(k, "proxy-connection") ||
strings.EqualFold(k, "transfer-encoding") || strings.EqualFold(k, "upgrade") ||
strings.EqualFold(k, "keep-alive") {
} else if http2asciiEqualFold(k, "connection") ||
http2asciiEqualFold(k, "proxy-connection") ||
http2asciiEqualFold(k, "transfer-encoding") ||
http2asciiEqualFold(k, "upgrade") ||
http2asciiEqualFold(k, "keep-alive") {
// Per 8.1.2.2 Connection-Specific Header
// Fields, don't send connection-specific
// fields. We have already checked if any
// are error-worthy so just ignore the rest.
continue
} else if strings.EqualFold(k, "user-agent") {
} else if http2asciiEqualFold(k, "user-agent") {
// Match Go's http1 behavior: at most one
// User-Agent. If set to nil or empty string,
// then omit it. Otherwise if not mentioned,
@ -8103,7 +8155,7 @@ func (cc *http2ClientConn) encodeHeaders(req *Request, addGzipHeader bool, trail
if vv[0] == "" {
continue
}
} else if strings.EqualFold(k, "cookie") {
} else if http2asciiEqualFold(k, "cookie") {
// Per 8.1.2.5 To allow for better compression efficiency, the
// Cookie header field MAY be split into separate header fields,
// each with one or more cookie-pairs.
@ -8162,7 +8214,12 @@ func (cc *http2ClientConn) encodeHeaders(req *Request, addGzipHeader bool, trail
// Header list size is ok. Write the headers.
enumerateHeaders(func(name, value string) {
name = strings.ToLower(name)
name, ascii := http2asciiToLower(name)
if !ascii {
// Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header
// field names have to be ASCII characters (just as in HTTP/1.x).
return
}
cc.writeHeader(name, value)
if traceHeaders {
http2traceWroteHeaderField(trace, name, value)
@ -8210,9 +8267,14 @@ func (cc *http2ClientConn) encodeTrailers(req *Request) ([]byte, error) {
}
for k, vv := range req.Trailer {
lowKey, ascii := http2asciiToLower(k)
if !ascii {
// Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header
// field names have to be ASCII characters (just as in HTTP/1.x).
continue
}
// Transfer-Encoding, etc.. have already been filtered at the
// start of RoundTrip
lowKey := strings.ToLower(k)
for _, v := range vv {
cc.writeHeader(lowKey, v)
}
@ -9635,7 +9697,12 @@ func http2encodeHeaders(enc *hpack.Encoder, h Header, keys []string) {
}
for _, k := range keys {
vv := h[k]
k = http2lowerHeader(k)
k, ascii := http2lowerHeader(k)
if !ascii {
// Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header
// field names have to be ASCII characters (just as in HTTP/1.x).
continue
}
if !http2validWireHeaderFieldName(k) {
// Skip it as backup paranoia. Per
// golang.org/issue/14048, these should

View file

@ -453,7 +453,7 @@ func (up *socksUsernamePassword) Authenticate(ctx context.Context, rw io.ReadWri
b = append(b, up.Username...)
b = append(b, byte(len(up.Password)))
b = append(b, up.Password...)
// TODO(mikio): handle IO deadlines and cancellation if
// TODO(mikio): handle IO deadlines and cancelation if
// necessary
if _, err := rw.Write(b); err != nil {
return err

View file

@ -154,14 +154,13 @@ var MIPS64X struct {
// For ppc64/ppc64le, it is safe to check only for ISA level starting on ISA v3.00,
// since there are no optional categories. There are some exceptions that also
// require kernel support to work (DARN, SCV), so there are feature bits for
// those as well. The minimum processor requirement is POWER8 (ISA 2.07).
// The struct is padded to avoid false sharing.
// those as well. The struct is padded to avoid false sharing.
var PPC64 struct {
_ CacheLinePad
HasDARN bool // Hardware random number generator (requires kernel enablement)
HasSCV bool // Syscall vectored (requires kernel enablement)
IsPOWER8 bool // ISA v2.07 (POWER8)
IsPOWER9 bool // ISA v3.00 (POWER9)
IsPOWER9 bool // ISA v3.00 (POWER9), implies IsPOWER8
_ CacheLinePad
}

View file

@ -20,6 +20,7 @@ func archInit() {
PPC64.IsPOWER8 = true
}
if impl&_IMPL_POWER9 != 0 {
PPC64.IsPOWER8 = true
PPC64.IsPOWER9 = true
}

View file

@ -18,7 +18,7 @@ golang.org/x/net/idna
golang.org/x/net/lif
golang.org/x/net/nettest
golang.org/x/net/route
# golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6
# golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744
## explicit; go 1.17
golang.org/x/sys/cpu
# golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f