Fix response-writer buffer behaviour

CHANGELOG.md

@@ -29,6 +29,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
   * [Basic Auth](./docs/REFERENCE.md#basic-auth-block) did not work if only the `htpasswd_file` attribute was defined ([#293](https://github.com/avenga/couper/pull/293))
   * Missing error handling for backend gzip header reads ([#291](https://github.com/avenga/couper/pull/291))
   * ResponseWriter fallback for possible statusCode 0 writes ([#291](https://github.com/avenga/couper/pull/291))
+  * ResponseWriter buffer behaviour; prepared chunk writes ([#301](https://github.com/avenga/couper/pull/301))
   * Proper client-request canceling ([#294](https://github.com/avenga/couper/pull/294))
 
 * [**Beta**](./docs/BETA.md)

server/writer/response.go

@@ -31,17 +31,16 @@ var (
 	_ logging.RecorderInfo = &Response{}
 
 	endOfHeader = []byte("\r\n\r\n")
+	endOfLine   = []byte("\r\n")
 )
 
 // Response wraps the http.ResponseWriter.
 type Response struct {
-	rw            http.ResponseWriter
-	headerBuffer  *bytes.Buffer
-	hijackedConn  net.Conn
-	httpStatus    []byte
-	httpLineDelim []byte
-	secureCookies string
-	statusWritten bool
+	hijackedConn     net.Conn
+	httpHeaderBuffer []byte
+	rw               http.ResponseWriter
+	secureCookies    string
+	statusWritten    bool
 	// logging info
 	statusCode      int
 	rawBytesWritten int
@@ -55,7 +54,6 @@ type Response struct {
 func NewResponseWriter(rw http.ResponseWriter, secureCookies string) *Response {
 	return &Response{
 		rw:            rw,
-		headerBuffer:  &bytes.Buffer{},
 		secureCookies: secureCookies,
 	}
 }
@@ -69,35 +67,23 @@ func (r *Response) Header() http.Header {
 func (r *Response) Write(p []byte) (int, error) {
 	l := len(p)
 	r.rawBytesWritten += l
-	if !r.statusWritten {
-		if len(r.httpStatus) == 0 {
-			r.httpStatus = p[:]
-			// required for short writes without any additional header
-			// to detect EOH chunk later on
-			if l >= 2 {
-				r.httpLineDelim = p[l-2 : l]
-			}
-			// Flush case in combination with bufio.Writer.
-			// httpStatus contains all bytes already.
-			if l > 4 && bytes.Equal(p[l-4:l], endOfHeader) {
-				i := bytes.Index(p, r.httpLineDelim)
-				r.headerBuffer.Write(p[i+2 : l-2]) // 2 = delimLength
-				r.flushHeader()
-			}
-
+	if !r.statusWritten { // buffer all until end-of-header chunk: '\r\n'
+		r.httpHeaderBuffer = append(r.httpHeaderBuffer, p...)
+		idx := bytes.Index(r.httpHeaderBuffer, endOfHeader)
+		if idx == -1 {
 			return l, nil
 		}
 
-		// End-of-header
-		// http.Response.Write() EOH chunk is: '\r\n'
-		if bytes.Equal(r.httpLineDelim, p) {
-			r.flushHeader()
-		}
+		r.flushHeader()
 
-		if l >= 2 {
-			r.httpLineDelim = p[l-2 : l]
+		bufLen := len(r.httpHeaderBuffer)
+		// More than http header related bytes? Write body.
+		if !bytes.HasSuffix(r.httpHeaderBuffer, endOfLine) && bufLen > idx+4 {
+			n, writeErr := r.rw.Write(r.httpHeaderBuffer[idx+4:]) // len(endOfHeader) -> 4
+			r.bytesWritten += n
+			return l, writeErr
 		}
-		return r.headerBuffer.Write(p)
+		return l, nil
 	}
 
 	n, writeErr := r.rw.Write(p)
@@ -131,12 +117,13 @@ func (r *Response) Flush() {
 }
 
 func (r *Response) flushHeader() {
-	reader := textproto.NewReader(bufio.NewReader(r.headerBuffer))
+	reader := textproto.NewReader(bufio.NewReader(bytes.NewBuffer(r.httpHeaderBuffer)))
+	headerLine, _ := reader.ReadLineBytes()
 	header, _ := reader.ReadMIMEHeader()
 	for k := range header {
 		r.rw.Header()[k] = header.Values(k)
 	}
-	r.WriteHeader(r.parseStatusCode(r.httpStatus))
+	r.WriteHeader(r.parseStatusCode(headerLine))
 }
 
 // WriteHeader wraps the WriteHeader method of the <http.ResponseWriter>.

server/writer/response_test.go

@@ -0,0 +1,116 @@
+package writer_test
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/avenga/couper/internal/test"
+	"github.com/avenga/couper/server/writer"
+)
+
+func TestResponse_ChunkWrite(t *testing.T) {
+	helper := test.New(t)
+
+	rec := httptest.NewRecorder()
+	w := writer.NewResponseWriter(rec, "")
+
+	content := []byte("HTTP/1.1 404 Not Found\r\n\r\nBody")
+	for i := 0; i < len(content); i++ {
+		_, err := w.Write([]byte{content[i]})
+		helper.Must(err)
+	}
+
+	res := rec.Result()
+	if res.StatusCode != http.StatusNotFound || w.StatusCode() != http.StatusNotFound {
+		t.Errorf("Want: %d, got: %d", http.StatusNotFound, res.StatusCode)
+	}
+
+	b, err := io.ReadAll(res.Body)
+	helper.Must(err)
+
+	if !bytes.Equal(b, []byte("Body")) {
+		t.Errorf("Expected body content, got: %q", string(b))
+	}
+
+	if w.WrittenBytes() != 4 {
+		t.Errorf("Expected 4 written bytes, got: %d", w.WrittenBytes())
+	}
+}
+
+func TestResponse_ProtoWrite(t *testing.T) {
+	helper := test.New(t)
+
+	rec := httptest.NewRecorder()
+	w := writer.NewResponseWriter(rec, "")
+
+	response := &http.Response{
+		StatusCode: http.StatusOK,
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"Test": []string{"Value"},
+		},
+		Body:          io.NopCloser(bytes.NewBufferString("testContent")),
+		ContentLength: 11,
+	}
+
+	helper.Must(response.Write(w))
+
+	res := rec.Result()
+	if res.StatusCode != http.StatusOK || w.StatusCode() != http.StatusOK {
+		t.Errorf("Want: %d, got: %d", http.StatusOK, res.StatusCode)
+	}
+
+	if res.Header.Get("Content-Length") != "11" {
+		t.Error("Expected Content-Length header")
+	}
+
+	if res.Header.Get("Test") != "Value" {
+		t.Errorf("Expected Test header, got: %q", res.Header.Get("Test"))
+	}
+
+	b, err := io.ReadAll(res.Body)
+	helper.Must(err)
+
+	if !bytes.Equal(b, []byte("testContent")) {
+		t.Errorf("Expected body content, got: %q", string(b))
+	}
+
+	if w.WrittenBytes() != 11 {
+		t.Errorf("Expected 11 written bytes, got: %d", w.WrittenBytes())
+	}
+}
+
+func TestResponse_ProtoWriteAll(t *testing.T) {
+	helper := test.New(t)
+
+	rec := httptest.NewRecorder()
+	w := writer.NewResponseWriter(rec, "")
+
+	response := &http.Response{
+		StatusCode: http.StatusForbidden,
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"Test": []string{"Value"},
+		},
+	}
+
+	buf := &bytes.Buffer{}
+	helper.Must(response.Write(buf))
+
+	_, err := buf.WriteTo(w)
+	helper.Must(err)
+
+	res := rec.Result()
+	if res.StatusCode != http.StatusForbidden || w.StatusCode() != http.StatusForbidden {
+		t.Errorf("Want: %d, got: %d", http.StatusOK, res.StatusCode)
+	}
+
+	if res.Header.Get("Test") != "Value" {
+		t.Errorf("Expected Test header, got: %q", res.Header.Get("Test"))
+	}
+}