Add "force downgrade" (client) and "prefer downgrade" (server) options (#4)

Author: misberner

_integration-tests/echo_service_test.go

@@ -73,7 +73,15 @@ func newCtx(t *testing.T, checkHeaders bool, checkTrailers bool) (context.Contex
 }
 
 func TestWithEchoService(t *testing.T) {
-	testCfg := newTestConfig(t)
+	for _, serverPreferGRPCWeb := range []bool{false, true} {
+		t.Run(fmt.Sprintf("prefer-grpc-web=%t", serverPreferGRPCWeb), func(t *testing.T) {
+			testWithEchoService(t, serverPreferGRPCWeb)
+		})
+	}
+}
+
+func testWithEchoService(t *testing.T, serverPreferGRPCWeb bool) {
+	testCfg := newTestConfig(t, serverPreferGRPCWeb)
 	defer testCfg.TearDown()
 
 	cases := []testCase{
@@ -145,6 +153,44 @@ func TestWithEchoService(t *testing.T) {
 			expectClientStreamOK:    false,
 			expectBidiStreamOK:      false,
 		},
+		{
+			targetID:             "raw-grpc",
+			useProxy:             true,
+			forceDowngrade:       true,
+			expectUnaryOK:        true,
+			expectServerStreamOK: true,
+			expectClientStreamOK: true,
+			expectBidiStreamOK:   true,
+		},
+		{
+			targetID:                "raw-grpc",
+			behindHTTP1ReverseProxy: true,
+			useProxy:                true,
+			forceDowngrade:          true,
+			expectUnaryOK:           false,
+			expectServerStreamOK:    false,
+			expectClientStreamOK:    false,
+			expectBidiStreamOK:      false,
+		},
+		{
+			targetID:             "downgrading-grpc",
+			useProxy:             true,
+			forceDowngrade:       true,
+			expectUnaryOK:        true,
+			expectServerStreamOK: true,
+			expectClientStreamOK: false,
+			expectBidiStreamOK:   false,
+		},
+		{
+			targetID:                "downgrading-grpc",
+			behindHTTP1ReverseProxy: true,
+			useProxy:                true,
+			forceDowngrade:          true,
+			expectUnaryOK:           true,
+			expectServerStreamOK:    true,
+			expectClientStreamOK:    false,
+			expectBidiStreamOK:      false,
+		},
 	}
 
 	for _, c := range cases {
@@ -155,7 +201,7 @@ func TestWithEchoService(t *testing.T) {
 }
 
 func TestWSWithEchoService(t *testing.T) {
-	testCfg := newTestConfig(t)
+	testCfg := newTestConfig(t, false)
 	defer testCfg.TearDown()
 
 	cases := []testCase{
@@ -269,6 +315,7 @@ type testCase struct {
 	behindHTTP1ReverseProxy bool
 	useProxy                bool
 	useWebSocket            bool
+	forceDowngrade          bool
 
 	expectUnaryOK        bool
 	expectClientStreamOK bool
@@ -282,6 +329,8 @@ func (c *testCase) Name() string {
 
 	if c.useWebSocket {
 		sb.WriteString("-ws")
+	} else if c.forceDowngrade {
+		sb.WriteString("-forced-downgrade")
 	}
 
 	if c.behindHTTP1ReverseProxy {
@@ -329,7 +378,7 @@ func (c *testCase) Run(t *testing.T, cfg *testConfig) {
 		if !c.behindHTTP1ReverseProxy {
 			opts = append(opts, client.ForceHTTP2())
 		}
-		opts = append(opts, client.UseWebSocket(c.useWebSocket))
+		opts = append(opts, client.UseWebSocket(c.useWebSocket), client.ForceDowngrade(c.forceDowngrade))
 
 		cc, err = client.ConnectViaProxy(ctx, targetAddr, nil, opts...)
 	} else {
@@ -704,7 +753,7 @@ type testConfig struct {
 	targetAddrs map[string]string
 }
 
-func newTestConfig(t *testing.T) *testConfig {
+func newTestConfig(t *testing.T, preferGRPCWeb bool) *testConfig {
 	targetAddrs := make(map[string]string)
 	grpcSrv := grpc.NewServer()
 	echo.RegisterEchoServer(grpcSrv, echoService{})
@@ -713,11 +762,13 @@ func newTestConfig(t *testing.T) *testConfig {
 	go grpcSrv.Serve(lis)
 	targetAddrs["raw-grpc"] = lis.Addr().String()
 
+	opts := []server.Option{server.PreferGRPCWeb(preferGRPCWeb)}
+
 	downgradingSrv := &http.Server{}
 	var h2Srv http2.Server
 	require.NoError(t, http2.ConfigureServer(downgradingSrv, &h2Srv))
 	downgradingSrv.Handler = h2c.NewHandler(
-		server.CreateDowngradingHandler(grpcSrv, http.NotFoundHandler()),
+		server.CreateDowngradingHandler(grpcSrv, http.NotFoundHandler(), opts...),
 		&h2Srv)
 
 	lis = listenLocal(t)

client/options.go

@@ -17,10 +17,11 @@ package client
 import "google.golang.org/grpc"
 
 type connectOptions struct {
-	dialOpts     []grpc.DialOption
-	extraH2ALPNs []string
-	forceHTTP2   bool
-	useWebSocket bool
+	dialOpts       []grpc.DialOption
+	extraH2ALPNs   []string
+	forceHTTP2     bool
+	forceDowngrade bool
+	useWebSocket   bool
 }
 
 // ConnectOption is an option that can be passed to the `ConnectViaProxy` method.
@@ -57,6 +58,14 @@ func UseWebSocket(use bool) ConnectOption {
 	return useWebSocketOption(use)
 }
 
+// ForceDowngrade returns a connection option that instructs the
+// client to always force gRPC-Web downgrade for gRPC requests.
+// Client- or Bidi-streaming requests will not work.
+// This option has no effect if websockets are being used.
+func ForceDowngrade(force bool) ConnectOption {
+	return forceDowngradeOption(force)
+}
+
 type dialOptsOption []grpc.DialOption
 
 func (o dialOptsOption) apply(opts *connectOptions) {
@@ -80,3 +89,9 @@ type useWebSocketOption bool
 func (o useWebSocketOption) apply(opts *connectOptions) {
 	opts.useWebSocket = bool(o)
 }
+
+type forceDowngradeOption bool
+
+func (o forceDowngradeOption) apply(opts *connectOptions) {
+	opts.forceDowngrade = bool(o)
+}

client/proxy.go

@@ -69,14 +69,21 @@ func writeError(w http.ResponseWriter, err error) {
 	w.Header().Set("Grpc-Message", errors.Wrap(err, "transport").Error())
 }
 
-func createReverseProxy(endpoint string, transport http.RoundTripper, insecure bool) *httputil.ReverseProxy {
+func createReverseProxy(endpoint string, transport http.RoundTripper, insecure, forceDowngrade bool) *httputil.ReverseProxy {
 	scheme := "https"
 	if insecure {
 		scheme = "http"
 	}
 	return &httputil.ReverseProxy{
 		Director: func(req *http.Request) {
-			req.Header.Add("Accept", "application/grpc")
+			if forceDowngrade {
+				req.ProtoMajor, req.ProtoMinor, req.Proto = 1, 1, "HTTP/1.1"
+				req.Header.Del("TE")
+				req.Header.Del("Accept")
+				req.Header.Add(grpcweb.GRPCWebOnlyHeader, "true")
+			} else {
+				req.Header.Add("Accept", "application/grpc")
+			}
 			req.Header.Add("Accept", "application/grpc-web")
 			req.URL.Scheme = scheme
 			req.URL.Host = endpoint
@@ -124,12 +131,12 @@ func createTransport(tlsClientConf *tls.Config, forceHTTP2 bool, extraH2ALPNs []
 	return transport, nil
 }
 
-func createClientProxy(endpoint string, tlsClientConf *tls.Config, forceHTTP2 bool, extraH2ALPNs []string) (*http.Server, pipeconn.DialContextFunc, error) {
+func createClientProxy(endpoint string, tlsClientConf *tls.Config, forceHTTP2, forceDowngrade bool, extraH2ALPNs []string) (*http.Server, pipeconn.DialContextFunc, error) {
 	transport, err := createTransport(tlsClientConf, forceHTTP2, extraH2ALPNs)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "creating transport")
 	}
-	proxy := createReverseProxy(endpoint, transport, tlsClientConf == nil)
+	proxy := createReverseProxy(endpoint, transport, tlsClientConf == nil, forceDowngrade)
 	return makeProxyServer(proxy)
 }
 
@@ -153,7 +160,7 @@ func ConnectViaProxy(ctx context.Context, endpoint string, tlsClientConf *tls.Co
 	if connectOpts.useWebSocket {
 		proxy, dialCtx, err = createClientWSProxy(endpoint, tlsClientConf)
 	} else {
-		proxy, dialCtx, err = createClientProxy(endpoint, tlsClientConf, connectOpts.forceHTTP2, connectOpts.extraH2ALPNs)
+		proxy, dialCtx, err = createClientProxy(endpoint, tlsClientConf, connectOpts.forceHTTP2, connectOpts.forceDowngrade, connectOpts.extraH2ALPNs)
 	}
 
 	if err != nil {

internal/grpcweb/defs.go

@@ -19,4 +19,10 @@ const (
 	trailerMessageFlag byte = 1 << 7
 
 	completeHeaderLen = 5
+
+	// GRPCWebOnlyHeader is a header to indicate that the server should always return gRPC-Web
+	// responses, regardless of detected client capabilities. The presence of the header alone
+	// is sufficient, however it is recommended that a client chooses "true" as the only value
+	// whenver the header is used.
+	GRPCWebOnlyHeader = `Grpc-Web-Only`
 )

internal/grpcweb/defs_test.go

@@ -0,0 +1,12 @@
+package grpcweb
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGRPCWebOnlyHeaderNameIsCanonical(t *testing.T) {
+	assert.Equal(t, GRPCWebOnlyHeader, http.CanonicalHeaderKey(GRPCWebOnlyHeader))
+}

server/options.go

@@ -0,0 +1,24 @@
+package server
+
+type options struct {
+	preferGRPCWeb bool
+}
+
+// Option is an object that controls the behavior of the downgrading gRPC server.
+type Option interface {
+	apply(o *options)
+}
+
+type optionFunc func(o *options)
+
+func (f optionFunc) apply(o *options) {
+	f(o)
+}
+
+// PreferGRPCWeb instructs the server to always send a downgraded gRPC-Web response
+// if the client indicates it accepts gRPC-Web responses.
+func PreferGRPCWeb(prefer bool) Option {
+	return optionFunc(func(o *options) {
+		o.preferGRPCWeb = prefer
+	})
+}

server/server.go

@@ -93,31 +93,50 @@ func handleGRPCWS(w http.ResponseWriter, req *http.Request, grpcSrv *grpc.Server
 	_ = conn.Close(websocket.StatusNormalClosure, "")
 }
 
-func handleGRPCWeb(w http.ResponseWriter, req *http.Request, validPaths map[string]struct{}, grpcSrv *grpc.Server) {
+func handleGRPCWeb(w http.ResponseWriter, req *http.Request, validPaths map[string]struct{}, grpcSrv *grpc.Server, srvOpts *options) {
+	_, isDowngradableMethod := validPaths[req.URL.Path]
+
 	// Check for HTTP/2.
 	if req.ProtoMajor != 2 {
-		if _, ok := validPaths[req.URL.Path]; !ok {
+		if !isDowngradableMethod {
 			// Client-streaming only works with HTTP/2.
 			http.Error(w, "Method cannot be downgraded", http.StatusInternalServerError)
 			return
 		}
 		req.ProtoMajor, req.ProtoMinor, req.Proto = 2, 0, "HTTP/2.0"
 	}
 
-	if req.Header.Get("TE") == "trailers" {
-		// Yay, client accepts trailers! Let the normal gRPC handler handle the request.
+	acceptedContentTypes := strings.FieldsFunc(strings.Join(req.Header["Accept"], ","), spaceOrComma)
+	acceptGRPCWeb := sliceutils.StringFind(acceptedContentTypes, "application/grpc-web") != -1
+	// The standard gRPC client doesn't actually send an `Accept: application/grpc` header, so always assume
+	// the client accepts gRPC _unless_ it explicitly specifies an `application/grpc-web` accept header
+	// WITHOUT an `application/grpc` accept header.
+	acceptGRPC := !acceptGRPCWeb || sliceutils.StringFind(acceptedContentTypes, "application/grpc") != -1
+
+	// Only consider sending a gRPC response if we are not told to prefer gRPC-Web or the client doesn't support
+	// gRPC-Web.
+	if srvOpts.preferGRPCWeb && isDowngradableMethod && acceptGRPCWeb {
+		acceptGRPC = false
+	}
+
+	// If the client accepts trailers, AND gRPC responses, AND did not set the "Grpc-Web-Only" header,
+	// return the response as a normal gRPC response.
+	if req.Header.Get("TE") == "trailers" && acceptGRPC && len(req.Header[grpcweb.GRPCWebOnlyHeader]) == 0 {
 		grpcSrv.ServeHTTP(w, req)
 		return
 	}
 
-	acceptedContentTypes := strings.FieldsFunc(strings.Join(req.Header["Accept"], ","), spaceOrComma)
-	acceptGRPCWeb := sliceutils.StringFind(acceptedContentTypes, "application/grpc-web") != -1
 	if !acceptGRPCWeb {
 		// Client doesn't support trailers and doesn't accept a response downgraded to gRPC web.
 		http.Error(w, "Client neither supports trailers nor gRPC web responses", http.StatusInternalServerError)
 		return
 	}
 
+	if !isDowngradableMethod {
+		http.Error(w, "Client requires a gRPC-Web response to a method that cannot be downgraded", http.StatusBadRequest)
+		return
+	}
+
 	// Tell the server we would accept trailers (the gRPC server currently (v1.29.1) doesn't check for this but it
 	// really should, as the purpose of the TE header according to the gRPC spec is to detect incompatible proxies).
 	req.Header.Set("TE", "trailers")
@@ -132,7 +151,7 @@ func handleGRPCWeb(w http.ResponseWriter, req *http.Request, validPaths map[stri
 
 // CreateDowngradingHandler takes a gRPC server and a plain HTTP handler, and returns an HTTP handler that has the
 // capability of handling HTTP requests and gRPC requests that may require downgrading the response to gRPC-Web or gRPC-WebSocket.
-func CreateDowngradingHandler(grpcSrv *grpc.Server, httpHandler http.Handler) http.Handler {
+func CreateDowngradingHandler(grpcSrv *grpc.Server, httpHandler http.Handler, opts ...Option) http.Handler {
 	// Only allow paths corresponding to gRPC methods that do not use client streaming for gRPC-Web.
 	validGRPCWebPaths := make(map[string]struct{})
 
@@ -148,6 +167,11 @@ func CreateDowngradingHandler(grpcSrv *grpc.Server, httpHandler http.Handler) ht
 		}
 	}
 
+	var serverOpts options
+	for _, opt := range opts {
+		opt.apply(&serverOpts)
+	}
+
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		if isWebSocketUpgrade(req.Header) {
 			handleGRPCWS(w, req, grpcSrv)
@@ -160,7 +184,7 @@ func CreateDowngradingHandler(grpcSrv *grpc.Server, httpHandler http.Handler) ht
 			return
 		}
 
-		handleGRPCWeb(w, req, validGRPCWebPaths, grpcSrv)
+		handleGRPCWeb(w, req, validGRPCWebPaths, grpcSrv, &serverOpts)
 	})
 }