reverse_proxy: fix bi-h2stream breaking gzip encode handle(caddyserve…

…r#3606).

caddytest/integration/stream_test.go

@@ -1,7 +1,9 @@
 package integration
 
 import (
+	"compress/gzip"
 	"context"
+	"crypto/rand"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -199,3 +201,235 @@ func testH2ToH2CStreamServeH2C(t *testing.T) *http.Server {
 	}
 	return server
 }
+
+// (see https://github.com/caddyserver/caddy/issues/3606 for use case)
+func TestH2ToH1ChunkedResponse(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(` 
+  {
+    "logging": {
+        "logs": {
+            "default": {
+                "level": "DEBUG"
+            }
+        }
+    },
+    "apps": {
+      "http": {
+        "http_port": 9080,
+        "https_port": 9443,
+        "servers": {
+          "srv0": {
+            "listen": [
+              ":9443"
+            ],
+            "routes": [
+              {
+				"handle": [
+					{
+						"handler": "subroute",
+						"routes": [
+							{
+								"handle": [
+									{
+										"encodings": {
+											"gzip": {}
+										},
+										"handler": "encode"
+									}
+								]
+							},
+							{
+								"handle": [
+								{
+									"handler": "reverse_proxy",
+									"upstreams": [
+									{
+										"dial": "localhost:54321"
+									}
+									]
+								}
+								],
+								"match": [
+								{
+									"path": [
+									"/tov2ray"
+									]
+								}
+								]
+							}
+						]
+					}
+				],
+				"terminal": true
+              }
+            ],
+            "tls_connection_policies": [
+              {
+                "certificate_selection": {
+                  "any_tag": ["cert0"]
+                },
+                "default_sni": "a.caddy.localhost"
+              }
+            ]
+          }
+        }
+      },
+      "tls": {
+        "certificates": {
+          "load_files": [
+            {
+              "certificate": "/a.caddy.localhost.crt",
+              "key": "/a.caddy.localhost.key",
+              "tags": [
+                "cert0"
+              ]
+            }
+          ]
+        }
+      },
+      "pki": {
+        "certificate_authorities" : {
+          "local" : {
+            "install_trust": false
+          }
+        }
+      }
+    }
+  }
+  `, "json")
+
+	// need a large body here to trigger caddy's compression, larger than gzip.miniLength
+	expectedBody, err := GenerateRandomString(1024)
+	if err != nil {
+		t.Fatalf("generate expected body failed, err: %s", err)
+	}
+
+	// start the server
+	server := testH2ToH1ChunkedResponseServeH1(t)
+	go server.ListenAndServe()
+	defer func() {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+		defer cancel()
+		server.Shutdown(ctx)
+	}()
+
+	r, w := io.Pipe()
+	req := &http.Request{
+		Method: "PUT",
+		Body:   ioutil.NopCloser(r),
+		URL: &url.URL{
+			Scheme: "https",
+			Host:   "127.0.0.1:9443",
+			Path:   "/tov2ray",
+		},
+		Proto:      "HTTP/2",
+		ProtoMajor: 2,
+		ProtoMinor: 0,
+		Header:     make(http.Header),
+	}
+	// underlying transport will automaticlly add gzip
+	// req.Header.Set("Accept-Encoding", "gzip")
+	go func() {
+		fmt.Fprint(w, expectedBody)
+		w.Close()
+	}()
+	resp := tester.AssertResponseCode(req, 200)
+	if 200 != resp.StatusCode {
+		return
+	}
+
+	defer resp.Body.Close()
+	bytes, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatalf("unable to read the response body %s", err)
+	}
+
+	body := string(bytes)
+
+	if body != expectedBody {
+		t.Errorf("requesting \"%s\" expected response body \"%s\" but got \"%s\"", req.RequestURI, expectedBody, body)
+	}
+	return
+}
+
+func testH2ToH1ChunkedResponseServeH1(t *testing.T) *http.Server {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+
+		if r.Host != "127.0.0.1:9443" {
+			t.Errorf("r.Host doesn't match, %v!", r.Host)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+
+		if !strings.HasPrefix(r.URL.Path, "/tov2ray") {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+
+		defer r.Body.Close()
+		bytes, err := ioutil.ReadAll(r.Body)
+		if err != nil {
+			t.Fatalf("unable to read the response body %s", err)
+		}
+
+		n := len(bytes)
+
+		var writer io.Writer
+		if strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
+			gw, err := gzip.NewWriterLevel(w, 5)
+			if err != nil {
+				t.Error("can't return gzip data")
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			defer gw.Close()
+			writer = gw
+			w.Header().Set("Content-Encoding", "gzip")
+			w.Header().Del("Content-Length")
+			w.WriteHeader(200)
+		} else {
+			writer = w
+		}
+		if n > 0 {
+			writer.Write(bytes[:])
+		}
+	})
+
+	server := &http.Server{
+		Addr:    "127.0.0.1:54321",
+		Handler: handler,
+	}
+	return server
+}
+
+// GenerateRandomBytes returns securely generated random bytes.
+// It will return an error if the system's secure random
+// number generator fails to function correctly, in which
+// case the caller should not continue.
+func GenerateRandomBytes(n int) ([]byte, error) {
+	b := make([]byte, n)
+	_, err := rand.Read(b)
+	// Note that err == nil only if we read len(b) bytes.
+	if err != nil {
+		return nil, err
+	}
+
+	return b, nil
+}
+
+// GenerateRandomString returns a securely generated random string.
+// It will return an error if the system's secure random
+// number generator fails to function correctly, in which
+// case the caller should not continue.
+func GenerateRandomString(n int) (string, error) {
+	const letters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-"
+	bytes, err := GenerateRandomBytes(n)
+	if err != nil {
+		return "", err
+	}
+	for i, b := range bytes {
+		bytes[i] = letters[b%byte(len(letters))]
+	}
+	return string(bytes), nil
+}

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -614,7 +614,12 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, di Dia
 	// some apps need the response headers before starting to stream content with http2,
 	// so it's important to explicitly flush the headers to the client before streaming the data.
 	// (see https://github.com/caddyserver/caddy/issues/3556 for use case)
-	if req.ProtoMajor == 2 && res.ContentLength == -1 {
+	// we have to check encoding here, only flush headers with identity encoding.
+	// Non-identity encoding might combine with "encode gzip" directive,
+	// in that case, if body size larger than enc.MinLength,
+	// upper level encode handle might have Content-Encoding header to write.
+	// (see https://github.com/caddyserver/caddy/issues/3606 for use case)
+	if req.ProtoMajor == 2 && res.ContentLength == -1 && req.Header.Get("Accept-Encoding") == "identity" {
 		if wf, ok := rw.(http.Flusher); ok {
 			wf.Flush()
 		}

modules/caddyhttp/reverseproxy/streaming.go

@@ -96,8 +96,8 @@ func (h Handler) flushInterval(req *http.Request, res *http.Response) time.Durat
 		return -1 // negative means immediately
 	}
 
-	// for h2 and h2c upstream streaming data to client (issue #3556)
-	if req.ProtoMajor == 2 && res.ContentLength == -1 {
+	// for h2 and h2c upstream streaming data to client (issue #3556,#3606)
+	if req.ProtoMajor == 2 && res.ContentLength == -1 && req.Header.Get("Accept-Encoding") == "identity" {
 		return -1
 	}