]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/internal/moddeps: use filepath.SkipDir only on directories
authorDmitri Shuralyov <dmitshur@golang.org>
Wed, 19 May 2021 03:29:14 +0000 (23:29 -0400)
committerDmitri Shuralyov <dmitshur@golang.org>
Wed, 19 May 2021 15:20:08 +0000 (15:20 +0000)
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>
src/cmd/internal/moddeps/moddeps_test.go
src/go.mod
src/go.sum
src/net/http/h2_bundle.go
src/net/http/socks_bundle.go
src/vendor/golang.org/x/sys/cpu/cpu.go
src/vendor/golang.org/x/sys/cpu/cpu_aix.go
src/vendor/modules.txt

index ba574f400495694a6d43bb68129c86d1ab8c4388..7723250468e692cc584753d4f82cec10911fadfe 100644 (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)
        }
index 93ff2d8d3c17a6b4faa0bf4071a82db524311f32..379dcf504e5316fe43a502bc12e92411650dd9d6 100644 (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
 )
index 1390ce6d458f0659c18f4c1e71eb32041e1c57df..b3de6c526c98c72c2029a8c354706653dba751b5 100644 (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=
index fd540ff255ef8efa3e1c129c6c8dca338f757f46..a948ff3eed4af65f0ac3393d89355cd2eabfee05 100644 (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.0–1.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
index e6db1c7640f976bea8ce94e266716966bbe5dbbc..e446669589905dcf71029c6774d75eab50279d76 100644 (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
index f77701fe86854828200db6d391769b50860fc195..abbec2d44bfbe30fa5dda7fd33da3cbf9bdc9523 100644 (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
 }
 
index 28b521643b1da473c0a9c12bfdc10894d157ffda..8aaeef545a76bee1f012a9fe4f02d65150b187ce 100644 (file)
@@ -20,6 +20,7 @@ func archInit() {
                PPC64.IsPOWER8 = true
        }
        if impl&_IMPL_POWER9 != 0 {
+               PPC64.IsPOWER8 = true
                PPC64.IsPOWER9 = true
        }
 
index 1b850342a5a82932d068ef701ace96e6ed9f3ab6..ff01db5cdc77fe99287db108845ae5403a729678 100644 (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