commit da81c19ce576ceb0d290c6e935abe2f162e8ce28 Author: David Fifield david@bamsoftware.com Date: Mon Jan 14 23:02:34 2019 -0700

Deduplicate request creation across HTTP/helper. --- meek-client/helper.go | 27 --------------------------- meek-client/meek-client.go | 26 +++++++++++++++----------- 2 files changed, 15 insertions(+), 38 deletions(-)

diff --git a/meek-client/helper.go b/meek-client/helper.go index f412aaa..eaaf302 100644 --- a/meek-client/helper.go +++ b/meek-client/helper.go @@ -186,30 +186,3 @@ func (rt *HelperRoundTripper) RoundTrip(req *http.Request) (*http.Response, erro } return &resp, nil } - -// Do an HTTP roundtrip through the configured browser extension, using the -// payload data in buf and the request metadata in info. -func roundTripWithHelper(buf []byte, info *RequestInfo) (*http.Response, error) { - var body io.Reader - if len(buf) > 0 { - // Leave body == nil when buf is empty. A nil body is an - // explicit signal that the body is empty. An empty - // *bytes.Reader or the magic value http.NoBody are supposed to - // be equivalent ways to signal an empty body, but in Go 1.8 the - // HTTP/2 code only understands nil. Not leaving body == nil - // causes the Content-Length header to be omitted from HTTP/2 - // requests, which in some cases can cause the server to return - // a 411 "Length Required" error. See - // https://bugs.torproject.org/22865. - body = bytes.NewReader(buf) - } - req, err := http.NewRequest("POST", info.URL.String(), body) - if err != nil { - return nil, err - } - if info.Host != "" { - req.Host = info.Host - } - req.Header.Set("X-Session-Id", info.SessionID) - return helperRoundTripper.RoundTrip(req) -} diff --git a/meek-client/meek-client.go b/meek-client/meek-client.go index 3923b9a..fd16099 100644 --- a/meek-client/meek-client.go +++ b/meek-client/meek-client.go @@ -119,9 +119,9 @@ type RequestInfo struct { Host string }

-// Do an HTTP roundtrip using the payload data in buf and the request metadata -// in info. -func roundTripWithHTTP(buf []byte, info *RequestInfo) (*http.Response, error) { +// Make an http.Request from the payload data in buf and the request metadata in +// info. +func makeRequest(buf []byte, info *RequestInfo) (*http.Request, error) { var body io.Reader if len(buf) > 0 { // Leave body == nil when buf is empty. A nil body is an @@ -143,7 +143,7 @@ func roundTripWithHTTP(buf []byte, info *RequestInfo) (*http.Response, error) { req.Host = info.Host } req.Header.Set("X-Session-Id", info.SessionID) - return httpRoundTripper.RoundTrip(req) + return req, nil }

// Do a roundtrip, trying at most limit times if there is an HTTP status other @@ -154,16 +154,12 @@ func roundTripWithHTTP(buf []byte, info *RequestInfo) (*http.Response, error) { // which will cause the connection to die. The alternative, though, is to just // kill the connection immediately. A better solution would be a system of // acknowledgements so we know what to resend after an error. -func roundTripRetries(buf []byte, info *RequestInfo, limit int) (*http.Response, error) { - roundTrip := roundTripWithHTTP - if options.HelperAddr != nil { - roundTrip = roundTripWithHelper - } +func roundTripRetries(rt http.RoundTripper, req *http.Request, limit int) (*http.Response, error) { var resp *http.Response var err error again: limit-- - resp, err = roundTrip(buf, info) + resp, err = rt.RoundTrip(req) // Retry only if the HTTP roundtrip completed without error, but // returned a status other than 200. Other kinds of errors and success // with 200 always return immediately. @@ -181,7 +177,15 @@ again: // Send the data in buf to the remote URL, wait for a reply, and feed the reply // body back into conn. func sendRecv(buf []byte, conn net.Conn, info *RequestInfo) (int64, error) { - resp, err := roundTripRetries(buf, info, maxTries) + var rt http.RoundTripper = httpRoundTripper + if options.HelperAddr != nil { + rt = helperRoundTripper + } + req, err := makeRequest(buf, info) + if err != nil { + return 0, err + } + resp, err := roundTripRetries(rt, req, maxTries) if err != nil { return 0, err }