Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http/httputil: ReverseProxy doesn't support TCP half-close when HTTP is upgraded #35892

Open
sngchlko opened this issue Nov 28, 2019 · 2 comments · May be fixed by #35893
Open

net/http/httputil: ReverseProxy doesn't support TCP half-close when HTTP is upgraded #35892

sngchlko opened this issue Nov 28, 2019 · 2 comments · May be fixed by #35893
Labels
Milestone

Comments

@sngchlko
Copy link

@sngchlko sngchlko commented Nov 28, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/scko/.cache/go-build"
GOENV="/home/scko/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/scko/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build357268893=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I made the simple program to show the problem that I have.
This program has three servers: a frontend, a reverse proxy, and a backend.
After the frontend's HTTP upgrade request is accepted, the backend immediately closes the output stream. But the backend can't read the input stream because of the socket is closed by the reverse proxy.

I think httputil.ReverseProxy doesn't support TCP half-close.

package main

import (
	"bufio"
	"fmt"
	"io"
	"io/ioutil"
	"log"
	"net/http"
	"net/http/httptest"
	"net/http/httputil"
	"net/url"
	"strings"

	"golang.org/x/net/http/httpguts"
)

func upgradeType(h http.Header) string {
	if !httpguts.HeaderValuesContainsToken(h["Connection"], "Upgrade") {
		return ""
	}
	return strings.ToLower(h.Get("Upgrade"))
}

func main() {
	reqDone := make(chan struct{})
	backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		defer close(reqDone)
		if upgradeType(r.Header) != "websocket" {
			http.Error(w, "unexpected request", 400)
			return
		}
		c, _, err := w.(http.Hijacker).Hijack()
		if err != nil {
			http.Error(w, fmt.Sprintf("%v", err), 500)
			return
		}

		io.WriteString(c, "HTTP/1.1 101 Switching Protocols\r\nConnection: upgrade\r\nUpgrade: WebSocket\r\n\r\n")

		if tcpc, ok := c.(interface {
			CloseWrite() error
		}); ok {
			tcpc.CloseWrite()
		} else if closer, ok := c.(io.Closer); ok {
			closer.Close()
			return
		}

		bs := bufio.NewScanner(c)
		if !bs.Scan() {
			fmt.Printf("backend failed to read line from client: %v", bs.Err())
			return
		}

		got := bs.Text()
		want := "Hello"
		if got != want {
			panic(fmt.Sprintf("got %#q, want %#q", got, want))
		}
		fmt.Println("backend got", got)
	}))
	defer backendServer.Close()

	backURL, _ := url.Parse(backendServer.URL)
	rproxy := httputil.NewSingleHostReverseProxy(backURL)
	rproxy.ErrorLog = log.New(ioutil.Discard, "", 0) // quiet for tests

	frontendProxy := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
		rproxy.ServeHTTP(rw, req)
	}))

	defer frontendProxy.Close()

	req, _ := http.NewRequest("GET", frontendProxy.URL, nil)
	req.Header.Set("Connection", "Upgrade")
	req.Header.Set("Upgrade", "websocket")

	c := frontendProxy.Client()
	res, err := c.Do(req)
	if err != nil {
		panic(err)
	}
	if res.StatusCode != 101 {
		panic(fmt.Sprintf("status = %v; want 101", res.Status))
	}

	if upgradeType(res.Header) != "websocket" {
		panic(fmt.Sprintf("not websocket upgrade; got %#v", res.Header))
	}
	rwc, ok := res.Body.(io.ReadWriteCloser)
	if !ok {
		panic(fmt.Sprintf("response body is of type %T; does not implement ReadWriteCloser", res.Body))
	}
	defer rwc.Close()

	ioutil.ReadAll(rwc)
	io.WriteString(rwc, "Hello\n")
	<-reqDone
}

What did you expect to see?

The backend can read the input stream even though the output stream is closed.

backend got Hello

What did you see instead?

But the input stream was closed as well.

backend failed to read line from client: <nil>
sngchlko added a commit to sngchlko/go that referenced this issue Nov 28, 2019
In order to support TCP half-close in ReverseProxy,
ReverseProxy.handleUpgradeResponse should not close all sockets
immediately when the one side gets an EOF. But we should close the
write stream on the socket to inform the transfer is complete.

Fixes golang#35892
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 28, 2019

Change https://golang.org/cl/209357 mentions this issue: net: support TCP half-close when HTTP is upgraded in ReverseProxy

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Nov 29, 2019

Thank you for this bug report as well as CL and welcome to the Go project @sngchlko!

I've added some feedback to your CL and we are currently in a code freeze until perhaps February 2020, but I look forward to encountering you more here and on the code review list and thank you again!

@odeke-em odeke-em added the NeedsFix label Nov 29, 2019
@odeke-em odeke-em added this to the Backlog milestone Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.