fix issue 555 about request header context-length missing (easegress-…

…io#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 <[email protected]>

Co-authored-by: Bomin Zhang <[email protected]>

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)

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

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()