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: support "gzip" as a Transfer Encoding #29162

Open
rv-dtomaz opened this issue Dec 9, 2018 · 22 comments
Open

net/http: support "gzip" as a Transfer Encoding #29162

rv-dtomaz opened this issue Dec 9, 2018 · 22 comments

Comments

@rv-dtomaz
Copy link

rv-dtomaz commented Dec 9, 2018

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

go version go1.10.3 windows/amd64

Does this issue reproduce with the latest release?

Yes.

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

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\xxxx\AppData\Local\go-build
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\xxxx\go
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\xxxx\AppData\Local\Temp\go-build557436246=/tmp/go-build -gno-record-gcc-switches

What did you do?

I am requesting a URL with code below:

package main

import (
	"fmt"
	"net/http"
	"time"
)

func main() {

	
	urlReq2 := "https://www.azulseguros.com.br/azul-cms/wp-content/themes/azul2016/integracao-azul-leve.php?timestamp=NGJYHX7MXNS"
	req, err := http.NewRequest("GET", urlReq2, nil)
	netClient := &http.Client{
		Timeout: time.Second * 10,
	}
	_, err = netClient.Do(req)
	if err != nil {
		fmt.Println(err)
	}

	

}

I got this error:
net/http: HTTP/1.x transport connection broken: unsupported transfer encoding "gzip"

What did you expect to see?

I expect to be foward to other page and get HTML.
In other languages like nodejs and c#, the same request is OK.
Running cURL also bring me the result.

What did you see instead?

Error on netClient.Do():
net/http: HTTP/1.x transport connection broken: unsupported transfer encoding "gzip"

@odeke-em odeke-em changed the title Error on Request: Transport connection broken: unsupported transfer encoding "gzip" net/http: support "gzip" as a Transfer Encoding Dec 9, 2018
@odeke-em
Copy link
Member

odeke-em commented Dec 9, 2018

Thank you for filing this issue @rv-dtomaz and welcome to the Go project!

So I believe we only support "identity" and "chunked" transfer encodings. However, I've retitled this
as a feature request and unfortunately we are in the Go1.12 so this will have to wait until Go1.13.

@bradfitz if I may indulge you, perhaps we should document that we only support "identity" and "chunked" transfer encodings, for this cycle? In the next cycle we can do a net/http hackathon where we examine all supported transfer encodings and perhaps add them.

@odeke-em odeke-em added this to the Go1.13 milestone Dec 9, 2018
@odeke-em
Copy link
Member

odeke-em commented Dec 9, 2018

And here is a standalone repro that doesn't need to make the network call, obtained by the results of calling that server https://play.golang.org/p/ND1AgZHXoDZ or inlined below

package main

import (
	"bufio"
	"io"
	"io/ioutil"
	"log"
	"net/http"
	"strings"
)

func main() {
	res, err := http.ReadResponse(bufio.NewReader(strings.NewReader(
		`HTTP/1.1 302 Found
Date: Sun, 09 Dec 2018 21:00:11 GMT
Server: Apache
X-Powered-By: PHP/5.6.25
Set-Cookie: PHPSESSID=0pn7kf6i398bml60vnjr28m1o7; path=/
Expires: Sat, 01 Jan 2017 01:00:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Location: https://as.azulseguros.com.br/AzulLeve/index.jsf?token=b75c366dbccb65be524d215aa9a1a55ff4f813230b3f18795a2c1edda4509339
Transfer-Encoding: gzip
Vary: Accept-Encoding
Content-Length: 0
Connection: close
Content-Type: text/html; charset=UTF-8
Set-Cookie: AzulSeguros-WWW_HTTPS=EBABABAK; Expires=Sun, 09-Dec-2018 23:00:11 GMT; Path=/

`)), nil)
	if err != nil {
		log.Fatalf("Failed to parse response: %v", err)
	}
	io.Copy(ioutil.Discard, res.Body)
	_ = res.Body.Close()
}
Failed to parse response: unsupported transfer encoding "gzip"

@rv-dtomaz
Copy link
Author

Hi @odeke-em ,thanks a lot for your quick suport.
I suspected that this was the problem.
Thanks a lot.
If I can help in solution, I am here.

best

@rv-dtomaz
Copy link
Author

@odeke-em Is there any workaround about this ? Something that we could do temporary.
I would really apreciate.

best

@bradfitz
Copy link
Contributor

Good overview of Content-Encoding gzip vs Transfer-Encoding gzip: https://stackoverflow.com/questions/11641923/transfer-encoding-gzip-vs-content-encoding-gzip

@odeke-em
Copy link
Member

@odeke-em Is there any workaround about this ? Something that we could do temporary.
I would really apreciate.

Great question @rv-dtomaz! My apologies for the late reply, I just saw this on waking up early.

So currently the work around that I can think of is a bit of a hackasauraus but it'll produce the identical response to the last response from curl -i -L $URL
It basically involves trying the request, if it spits out the error you've encountered, then:
a) Make a connection the server
b) Speak HTTP/1.1 to it
c) If it returns "Transfer-Encoding":"gzip", turn that into "Content-Encoding":"gzip"
d) Rewire the entire request's body and invoke http.ReadResponse

and here it is https://gist.github.com/odeke-em/39c8ecb7522edf80f1d91df71e415340 or inlined below

package main

import (
	"bufio"
	"bytes"
	"crypto/tls"
	"fmt"
	"io"
	"log"
	"net"
	"net/http"
	"net/http/httputil"
	"net/textproto"
	"strings"
	"time"
)

func main() {
	uri := "https://www.azulseguros.com.br/azul-cms/wp-content/themes/azul2016/integracao-azul-leve.php?timestamp=NGJYHX7MXNS"
	req, err := http.NewRequest("GET", uri, nil)
	netClient := &http.Client{
		Timeout:   time.Second * 10,
		Transport: &transferEncodingRedactor{base: http.DefaultTransport.(*http.Transport)},
	}
	res, err := netClient.Do(req)
	if err != nil {
		log.Fatalf("Failed to make request: %v", err)
	}
	blob, _ := httputil.DumpResponse(res, true)
	fmt.Printf("%s\n", blob)
	_ = res.Body.Close()
}

type transferEncodingRedactor struct {
	base *http.Transport
}

func (rt *transferEncodingRedactor) RoundTrip(req *http.Request) (*http.Response, error) {
	// Optimistic route i.e common case.
	res, err := rt.base.RoundTrip(req)
	if err == nil || res != nil {
		return res, err
	}

	// An error occured here but now time ot examine the contents
	if !strings.Contains(err.Error(), "unsupported transfer encoding") {
		return res, err
	}

	u := req.URL
	var conn net.Conn
	switch req.URL.Scheme {
	case "https":
		tr := rt.base
		if tr == nil {
			tr = http.DefaultTransport.(*http.Transport)
		}
		conn, err = tls.Dial("tcp", "www.azulseguros.com.br:443", tr.TLSClientConfig)
	default:
		conn, err = net.Dial("tcp", "www.azulseguros.com.br:80")
	}

	if err != nil {
		return nil, fmt.Errorf("Dial error: %v", err)
	}

	// Ensure we close the connection after.
	// Perhaps you might want to reuse it per host?
	defer conn.Close()
	br := bufio.NewReader(conn)
	bw := bufio.NewWriter(conn)

	s := u.Path
	if q := u.RawQuery; len(q) > 0 {
		s += "?" + q
	}

	intro := fmt.Sprintf("%s %s HTTP/1.1\r\nHost: %s\r\n\r\n", req.Method, s, u.Host)
	fmt.Fprint(bw, intro)
	bw.Flush()

	// And now we've got the case of an unsupported transfer
	// encoding time for a raw fetch and header rewrite.
	tp := textproto.NewReader(br)

	protoLine, err := tp.ReadLine()
	if err != nil {
		return nil, fmt.Errorf("ReadLine error: %v", err)
	}
	// Time to parse the headers
	rhdr, err := tp.ReadMIMEHeader()
	if err != nil {
		return nil, fmt.Errorf("ReadMIMEHeader: %v", err)
	}
	hdr := http.Header(rhdr)
	// Now rewrite the header
	if te := hdr.Get("Transfer-Encoding"); te == "gzip" {
		hdr.Set("Content-Encoding", "gzip")
		delete(hdr, "Transfer-Encoding")
	}

	// After this rewrite, reassemble the already read parts back
	// and then get them to ReadResponse
	hdrBuf := new(bytes.Buffer)
	if err := hdr.Write(hdrBuf); err != nil {
		return nil, fmt.Errorf("Rewriting header to wire: %v", err)
	}

	reconstructed := io.MultiReader(strings.NewReader(protoLine+"\r\n"), hdrBuf, strings.NewReader("\r\n"), br)
	return http.ReadResponse(bufio.NewReader(reconstructed), req)
}

Response

HTTP/1.1 200 OK
Content-Type: text/html;charset=UTF-8
Date: Mon, 10 Dec 2018 16:52:57 GMT
Server: WildFly/10
Set-Cookie: JSESSIONID=4pq22naRmHI-EuX1FrYq5PUml_z1WOD4LPsdpp3K.sentraprdlb6:site-emissao-inst2; path=/AzulLeve
Set-Cookie: ASAzul-Zafira_SSL=FDABABAK; Expires=Mon, 10-Dec-2018 18:52:57 GMT; Path=/
Vary: Accept-Encoding
X-Powered-By: Undertow/1

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml"><head><link type="text/css" rel="stylesheet" href="/AzulLeve/javax.faces.resource/theme.css.jsf?ln=primefaces-redmond" /><link type="text/css" rel="stylesheet" href="/AzulLeve/javax.faces.resource/primefaces.css.jsf;jsessionid=4pq22naRmHI-EuX1FrYq5PUml_z1WOD4LPsdpp3K.sentraprdlb6:site-emissao-inst2?ln=primefaces&amp;v=5.3&amp;v=5.3&amp;v=5.3" /><script type="text/javascript" src="/AzulLeve/javax.faces.resource/jquery/jquery.js.jsf;jsessionid=4pq22naRmHI-EuX1FrYq5PUml_z1WOD4LPsdpp3K.sentraprdlb6:site-emissao-inst2?ln=primefaces&amp;v=5.3&amp;v=5.3&amp;v=5.3"></script><script type="text/javascript" src="/AzulLeve/javax.faces.resource/jquery/jquery-plugins.js.jsf;jsessionid=4pq22naRmHI-EuX1FrYq5PUml_z1WOD4LPsdpp3K.sentraprdlb6:site-emissao-inst2?ln=primefaces&amp;v=5.3&amp;v=5.3&amp;v=5.3"></script><script type="text/javascript" src="/AzulLeve/javax.faces.resource/primefaces.js.jsf;jsessionid=4pq22naRmHI-EuX1FrYq5PUml_z1WOD4LPsdpp3K.sentraprdlb6:site-emissao-inst2?ln=primefaces&amp;v=5.3&amp;v=5.3&amp;v=5.3"></script><script type="text/javascript">if(window.PrimeFaces){PrimeFaces.settings.projectStage='Development';}</script>

	<title>
		AZUL AUTO LEVE / POPULAR	
	</title>
	<meta http-equiv="expires" content="0" />
	<meta http-equiv="pragma" content="no-cache" />
	<meta http-equiv="cache-control" content="no-cache" />
	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
	<meta http-equiv="X-UA-Compatible" content="IE=9" />
	<meta http-equiv="X-UA-Compatible" content="IE=8" />
	<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
	<link rel="stylesheet" href="/AzulLeve/resources/css/application.css" type="text/css" />
				<script type="text/javascript" src="/AzulLeve/resources/js/AzulLeve.js">
		</script></head><body>
		<div name="mensagens"><div id="messages" class="ui-messages ui-widget" aria-live="polite"></div>			
		</div>	
		
		<main class="content">

		<p class="labelazul">AZUL AUTO LEVE</p><div id="j_idt8" class="ui-panel ui-widget ui-widget-content ui-corner-all" data-widget="widget_j_idt8"><div id="j_idt8_content" class="ui-panel-content ui-widget-content">
			<h3 class="errotitulo">Sessão expirada.</h3><img id="j_idt10" src="/AzulLeve/resources/img/fim_sessao.png;jsessionid=4pq22naRmHI-EuX1FrYq5PUml_z1WOD4LPsdpp3K.sentraprdlb6:site-emissao-inst2?pfdrid_c=true" alt="" class="erroimg" width="76" height="72" /></div></div><script id="j_idt8_s" type="text/javascript">PrimeFaces.cw("Panel","widget_j_idt8",{id:"j_idt8"});</script> 
		</main></body>
</html>

@bradfitz might cringe at me :)

@rv-dtomaz
Copy link
Author

Hi @odeke-em ...
Don`t worry, this will help me a lot...
Thanks

@monis0395
Copy link

Hello, @odeke-em

will this be supported soon? I am encountering the error

In the next cycle we can do a net/http hackathon where we examine all supported transfer encodings and perhaps add them.

Thanks!

@odeke-em
Copy link
Member

odeke-em commented Mar 7, 2019

@monis0395 thanks for the ping! Did you see my suggested workaround in #29162 (comment)?

For sure, I'll prioritize this and roll out a CL to fix it in the next 4 days.

@odeke-em odeke-em self-assigned this Mar 7, 2019
@monis0395
Copy link

Thank for the quick reply, sorry could respond earlier

yes I tried but it didn't work, just the error message got shorter 😛
from net/http: HTTP/1.x transport connection broken: unsupported transfer encoding "gzip"
to unsupported transfer encoding "gzip"

it's great to hear that it would be fix so soon 🙌🙌🙌

@monis0395
Copy link

Hi @odeke-em

I tried debugging it more and found that at line 86 the values are

protoLine: HTTP/1.1 500 Internal Server Error
err: nil

The endpoint which I am hitting is a http2 and content-type is text/event-stream is it because of that?

I dont if this helps but when I use fasthttp for that endpoint it works but I wanna avoid using fasthttp

Thanks

@gopherbot
Copy link

Change https://golang.org/cl/166517 mentions this issue: net/http: support gzip as a Transfer-Encoding

@monis0395
Copy link

Hi @odeke-em

I tried your code from https://golang.org/cl/166517, I copied tranfer.go to my local and checked my endpoint

Now I am getting 200 OK status 👍 but the contents are empty when I read them from response

any clue?

Thanks

@monis0395
Copy link

Hi @odeke-em

Yesterday I spent more time on this issue. I found the issue why the work around wasn't working from the suggested in #29162 (comment).

Instead of this

	intro := fmt.Sprintf("%s %s HTTP/1.1\r\nHost: %s\r\n\r\n", req.Method, s, u.Host)
	fmt.Fprint(bw, intro)
	bw.Flush()

	// And now we've got the case of an unsupported transfer
	// encoding time for a raw fetch and header rewrite.
        tp := textproto.NewReader(br)

       protoLine, err := tp.ReadLine()

I changed it to this

        intro := fmt.Sprintf("%s /%s HTTP/1.1\r\nHost:%s\r\n\r\n", req.Method, s, u.Host)
	fmt.Fprint(conn, intro)

	br := bufio.NewReader(conn)
	// And now we've got the case of an unsupported transfer
	// encoding time for a raw fetch and header rewrite.
        tp := textproto.NewReader(br)

       protoLine, err := tp.ReadLine()

Here is the full gist https://gist.github.com/monis0395/0202b2129e943989e83a828553077cde

But it still did not work when I am hitting to my server. I get protoLine = HTTP/1.1 500. I think some problem with the way I am connect to the server. I am completely new with textproto, can you please suggest where can i read more about it?

Also I tried the change u made in transfer.go it is working now. This time I added each of your change manually, last time I had download the file transfer.go. I don't know why it didn't work last time 😕

Good news is that It works now 😁

I want to use your change in my production code, so is it possible to use it without changing in the go/src/net/http/tranfer.go? Any bright ideas? Because I want to avoid changing the golang source code.

Thanks!

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@odeke-em
Copy link
Member

odeke-em commented Oct 1, 2019

Sorry for the late reply @monis0395, I was quite swamped in the past quarters but I've made some small updates on CL https://go-review.googlesource.com/c/go/+/166517 and I shall kindly ping @bradfitz to take a look so that perhaps we can include it for Go1.14.

@odeke-em
Copy link
Member

odeke-em commented Oct 1, 2019

I want to use your change in my production code, so is it possible to use it without changing in the go/src/net/http/tranfer.go? Any bright ideas? Because I want to avoid changing the golang source code.

Unfortunately you'd need to deploy a custom fork otherwise we'd have to write a custom server which gets tricky ;)

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@odeke-em odeke-em modified the milestones: Backlog, Go1.14 Nov 8, 2019
@odeke-em
Copy link
Member

odeke-em commented Nov 8, 2019

@monis0395 if you get gotip or what will become Go1.14 (whose release candidates will be made soon) you'll have this in production. Cheers and thank you for the feature request!

@gopherbot
Copy link

Change https://golang.org/cl/215757 mentions this issue: Revert "net/http: support gzip, x-gzip Transfer-Encodings"

@FiloSottile
Copy link
Contributor

Unfortunately the implementation looks broken for a Transfer-Encoding of "gzip" (without "chunked"), so I opened CL 215757 to roll it back from Go 1.14. The code in the original issue report fails with the following error:

net/http: HTTP/1.x transport connection broken: http: failed to gunzip body: unexpected EOF

Transfer-Encoding is one of the most delicate security surfaces of HTTP, and we've had a number of issues in the past with it, including request smuggling vulnerabilities, so I'd like to push back on implementing this, also considering most clients and servers use Content-Encoding instead.

@FiloSottile FiloSottile reopened this Jan 22, 2020
gopherbot pushed a commit that referenced this issue Jan 22, 2020
This reverts commit e6c12c3.

Reason for revert: the assumption that a T-E of "gzip" implies
"chunked" seems incorrect. The RFC does state that one "MUST apply
chunked as the final transfer coding" but that should be interpreted to
mean that a "chunked" encoding must be listed as the last one, not that
one should be assumed to be there if not. This is confirmed by the
alternative option to chunking on the server side being to "terminate
the message by closing the connection".

The issue seems confirmed by the fact that the code in the body of
#29162 fails with the following error:

    net/http: HTTP/1.x transport connection broken: http: failed to gunzip body: unexpected EOF

This late in the cycle, revert rather than fix, also because we don't
apparently have tests for the correct behavior.

Change-Id: I920ec928754cd8e96a06fb7ff8a53316c0f959e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/215757
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
@odeke-em
Copy link
Member

Since this was reverted for Go1.14, we'll take a look at it for the next cycle for Go1.15 and I'll move the milestone.

@odeke-em odeke-em modified the milestones: Go1.14, Backlog Jan 27, 2020
@FiloSottile
Copy link
Contributor

Transfer-Encoding is a very security sensitive surface. For example, that CL had almost certainly introduced a request smuggling vulnerability because an upstream Go server could be made to disagree with a proxy on chunking. I'd like to see more justification for why we need this feature, which AFAICT is not at all widely implemented.

@mark-kubacki
Copy link

Transfer-Encoding is theoretically interesting in the context of uploads, i. e. to have a smaller representation on the wire, but can be used to transmit zip bombs to exploit unbounded buffers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants