From d98176f2aa8741e6239feca1d779ea61d90700db Mon Sep 17 00:00:00 2001 From: SU Chen Date: Tue, 7 Jun 2022 15:18:13 +0800 Subject: [PATCH] fix issue 555 about request header context-length missing (#649) * fix issue 555 about request header context-length missing * add more test * update comments * update comments * update pkg/filter/proxy/proxy.go comment Co-authored-by: Bomin Zhang Co-authored-by: Bomin Zhang --- pkg/filter/proxy/proxy.go | 17 +++++++++++++++++ pkg/filter/proxy/request.go | 10 ++++++++++ pkg/filter/proxy/request_test.go | 3 +++ 3 files changed, 30 insertions(+) diff --git a/pkg/filter/proxy/proxy.go b/pkg/filter/proxy/proxy.go index 6879ef3f20..737409c48c 100644 --- a/pkg/filter/proxy/proxy.go +++ b/pkg/filter/proxy/proxy.go @@ -343,6 +343,23 @@ func (b *Proxy) fallbackForCodes(ctx context.HTTPContext) bool { } // Handle handles HTTPContext. +// When we create new request for backend, we call http.NewRequestWithContext method and use context.Request().Body() as body. +// Based on golang std lib comments: +// https://github.com/golang/go/blob/95b68e1e02fa713719f02f6c59fb1532bd05e824/src/net/http/request.go#L856-L860 +// If body is of type *bytes.Buffer, *bytes.Reader, or +// *strings.Reader, the returned request's ContentLength is set to its +// exact value (instead of -1), GetBody is populated (so 307 and 308 +// redirects can replay the body), and Body is set to NoBody if the +// ContentLength is 0. +// +// So in this way, http.Request.ContentLength will be 0, and when http.Client send this request, it will delete +// "Content-Length" key in header. We solve this problem by set http.Request.ContentLength equal to +// http.Request.Header["Content-Length"] (if it is presented). +// Reading all context.Request().Body() and create new request with bytes.NewReader is another way, but it may cause performance loss. +// +// It is important that "Content-Length" in the Header is equal to the length of the Body. In easegress, when a filter change Request.Body, +// it will delete the header of "Content-Length". So, you should not worry about this when using our filters. +// But for customer filters, developer should make sure to delete or set "Context-Length" value in header when change Request.Body. func (b *Proxy) Handle(ctx context.HTTPContext) (result string) { result = b.handle(ctx) return ctx.CallNextHandler(result) diff --git a/pkg/filter/proxy/request.go b/pkg/filter/proxy/request.go index bbf25c9459..f14172d0d9 100644 --- a/pkg/filter/proxy/request.go +++ b/pkg/filter/proxy/request.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "net/http" + "strconv" "sync" "time" @@ -30,6 +31,7 @@ import ( "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/util/fasttime" + "github.com/megaease/easegress/pkg/util/httpheader" ) type ( @@ -80,6 +82,14 @@ func (p *pool) newRequest( stdr.Host = r.Host() } + // Based on comments in proxy.Handle, we update stdr.ContentLength here. + if val := stdr.Header.Get(httpheader.KeyContentLength); val != "" { + l, err := strconv.Atoi(val) + if err == nil { + stdr.ContentLength = int64(l) + } + } + req.std = stdr return req, nil diff --git a/pkg/filter/proxy/request_test.go b/pkg/filter/proxy/request_test.go index f1e398d8b0..20b4365f63 100644 --- a/pkg/filter/proxy/request_test.go +++ b/pkg/filter/proxy/request_test.go @@ -20,6 +20,7 @@ package proxy import ( "bytes" "net/http" + "strconv" "strings" "testing" "time" @@ -57,8 +58,10 @@ func TestRequest(t *testing.T) { p := pool{} sr := strings.NewReader("this is the raw body") + ctx.Request().Header().Add(httpheader.KeyContentLength, strconv.Itoa(sr.Len())) req, _ := p.newRequest(ctx, &server, sr, requestPool, httpStatResultPool) defer requestPool.Put(req) // recycle request + assert.Equal(int64(sr.Len()), req.std.ContentLength) req.start() tm := req.startTime()