grpc: remove support for env var GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING (grpc#9112)

Fixes grpc#8986

This PR addresses a TODO to have this env var removed.

RELEASE NOTES:
- grpc: Remove support for environment variable
`GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING`. Strict path
checking will be enforced unconditionally, going forward.

Author: goingforstudying-ctrl

internal/envconfig/envconfig.go

@@ -104,26 +104,6 @@ var (
 	// to "false".
 	XDSRecoverPanicInResourceParsing = boolFromEnv("GRPC_GO_EXPERIMENTAL_XDS_RESOURCE_PANIC_RECOVERY", true)
 
-	// DisableStrictPathChecking indicates whether strict path checking is
-	// disabled. This feature can be disabled by setting the environment
-	// variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING to "true".
-	//
-	// When strict path checking is enabled, gRPC will reject requests with
-	// paths that do not conform to the gRPC over HTTP/2 specification found at
-	// https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md.
-	//
-	// When disabled, gRPC will allow paths that do not contain a leading slash.
-	// Enabling strict path checking is recommended for security reasons, as it
-	// prevents potential path traversal vulnerabilities.
-	//
-	// A future release will remove this environment variable, enabling strict
-	// path checking behavior unconditionally.
-	//
-	// See
-	// https://github.com/grpc/grpc-go/security/advisories/GHSA-p77j-4mvh-x3m3
-	// for more details.
-	DisableStrictPathChecking = boolFromEnv("GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING", false)
-
 	// EnablePriorityLBChildPolicyCache controls whether the priority balancer
 	// should cache child balancers that are removed from the LB policy config,
 	// for a period of 15 minutes. This is disabled by default, but can be

server.go

@@ -42,7 +42,6 @@ import (
 	"google.golang.org/grpc/internal"
 	"google.golang.org/grpc/internal/binarylog"
 	"google.golang.org/grpc/internal/channelz"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/grpcsync"
 	"google.golang.org/grpc/internal/grpcutil"
 	istats "google.golang.org/grpc/internal/stats"
@@ -150,8 +149,6 @@ type Server struct {
 
 	serverWorkerChannel      chan func()
 	serverWorkerChannelClose func()
-
-	strictPathCheckingLogEmitted atomic.Bool
 }
 
 type serverOptions struct {
@@ -1817,26 +1814,11 @@ func (s *Server) handleStream(t transport.ServerTransport, stream *transport.Ser
 		}
 	}
 
-	sm := stream.Method()
-	if sm == "" {
+	sm, found := strings.CutPrefix(stream.Method(), "/")
+	if !found {
 		s.handleMalformedMethodName(stream, ti)
 		return
 	}
-	if sm[0] != '/' {
-		if envconfig.DisableStrictPathChecking {
-			if old := s.strictPathCheckingLogEmitted.Swap(true); !old {
-				channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream received malformed method name %q. Allowing it because the environment variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING is set to true, but this option will be removed in a future release. See https://github.com/grpc/grpc-go/security/advisories/GHSA-p77j-4mvh-x3m3 for more information.", sm)
-			}
-		} else {
-			if old := s.strictPathCheckingLogEmitted.Swap(true); !old {
-				channelz.Warningf(logger, s.channelz, "grpc: Server.handleStream rejected malformed method name %q. To temporarily allow such requests, set the environment variable GRPC_GO_EXPERIMENTAL_DISABLE_STRICT_PATH_CHECKING to true. Note that this is not recommended as it may allow requests to bypass security policies. See https://github.com/grpc/grpc-go/security/advisories/GHSA-p77j-4mvh-x3m3 for more information.", sm)
-			}
-			s.handleMalformedMethodName(stream, ti)
-			return
-		}
-	} else {
-		sm = sm[1:]
-	}
 	pos := strings.LastIndex(sm, "/")
 	if pos == -1 {
 		s.handleMalformedMethodName(stream, ti)

test/malformed_method_test.go

@@ -26,9 +26,7 @@ import (
 
 	"golang.org/x/net/http2"
 	"golang.org/x/net/http2/hpack"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/stubserver"
-	"google.golang.org/grpc/internal/testutils"
 
 	testpb "google.golang.org/grpc/interop/grpc_testing"
 )
@@ -40,7 +38,6 @@ func (s) TestMalformedMethodPath(t *testing.T) {
 	tests := []struct {
 		name       string
 		path       string
-		envVar     bool
 		wantStatus string // string representation of codes.Code
 	}{
 		{
@@ -58,33 +55,13 @@ func (s) TestMalformedMethodPath(t *testing.T) {
 			path:       "/",
 			wantStatus: "12", // Unimplemented
 		},
-		{
-			name:       "missing_leading_slash_disableStrictPathChecking_true",
-			path:       "grpc.testing.TestService/UnaryCall",
-			envVar:     true,
-			wantStatus: "0", // OK
-		},
-		{
-			name:       "empty_path_disableStrictPathChecking_true",
-			path:       "",
-			envVar:     true,
-			wantStatus: "12", // Unimplemented
-		},
-		{
-			name:       "just_slash_disableStrictPathChecking_true",
-			path:       "/",
-			envVar:     true,
-			wantStatus: "12", // Unimplemented
-		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
 			defer cancel()
 
-			testutils.SetEnvConfig(t, &envconfig.DisableStrictPathChecking, tc.envVar)
-
 			ss := &stubserver.StubServer{
 				UnaryCallF: func(context.Context, *testpb.SimpleRequest) (*testpb.SimpleResponse, error) {
 					return &testpb.SimpleResponse{Payload: &testpb.Payload{Body: []byte("pwned")}}, nil