Merge pull request #291 from avenga/fix-status-0

Fix statusCode 0 write panic

CHANGELOG.md

@@ -25,6 +25,8 @@ Unreleased changes are available as `avenga/couper:edge` container.
   * Documentation of [`request.query.<name>`](./docs/REFERENCE.md#request) ([#278](https://github.com/avenga/couper/pull/278))
   * Missing access log on some error cases ([#267](https://github.com/avenga/couper/issues/267))
   * Panic during backend origin / url usage with previous parse error ([#206](https://github.com/avenga/couper/issues/206))
+  * 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))
 
 * [**Beta**](./docs/BETA.md)
   * OAuth2 Authorization Code Grant Flow: [`beta_oauth2 {}` block](./docs/REFERENCE.md#oauth2-ac-block-beta);  [`beta_oauth_authorization_url()`](./docs/REFERENCE.md#functions) and [`beta_oauth_verifier()`](./docs/REFERENCE.md#functions) ([#247](https://github.com/avenga/couper/pull/247))

eval/buffer.go

@@ -34,6 +34,10 @@ func (i BufferOption) GoString() string {
 func MustBuffer(body hcl.Body) BufferOption {
 	result := BufferNone
 
+	if body == nil {
+		return result
+	}
+
 	attrs, err := body.JustAttributes()
 	if err != nil {
 		return result

eval/context.go

@@ -185,6 +185,7 @@ func (c *Context) WithBeresps(beresps ...*http.Response) *Context {
 		if n, ok := bereq.Context().Value(request.RoundTripName).(string); ok {
 			name = n
 		}
+
 		p := bereq.URL.Port()
 		if p == "" {
 			if bereq.URL.Scheme == "https" {
@@ -194,6 +195,7 @@ func (c *Context) WithBeresps(beresps ...*http.Response) *Context {
 			}
 		}
 		port, _ := strconv.ParseInt(p, 10, 64)
+
 		body, jsonBody := parseReqBody(bereq)
 		bereqs[name] = cty.ObjectVal(ContextMap{
 			Method:   cty.StringVal(bereq.Method),

handler/endpoint_test.go

@@ -13,13 +13,15 @@ import (
 	"github.com/hashicorp/hcl/v2/hclsimple"
 	logrustest "github.com/sirupsen/logrus/hooks/test"
 
+	"github.com/avenga/couper/config/request"
 	"github.com/avenga/couper/errors"
 	"github.com/avenga/couper/eval"
 	"github.com/avenga/couper/handler"
 	"github.com/avenga/couper/handler/producer"
 	"github.com/avenga/couper/handler/transport"
 	"github.com/avenga/couper/internal/test"
 	"github.com/avenga/couper/server/writer"
+	"github.com/sirupsen/logrus"
 )
 
 func TestEndpoint_RoundTrip_Eval(t *testing.T) {
@@ -242,7 +244,7 @@ func TestEndpoint_RoundTripContext_Variables_json_body(t *testing.T) {
 	}
 }
 
-// TestProxy_SetRoundtripContext_Null_Eval tests the handling with non existing references or cty.Null evaluations.
+// TestProxy_SetRoundtripContext_Null_Eval tests the handling with non-existing references or cty.Null evaluations.
 func TestEndpoint_RoundTripContext_Null_Eval(t *testing.T) {
 	helper := test.New(t)
 
@@ -364,3 +366,89 @@ func TestEndpoint_RoundTripContext_Null_Eval(t *testing.T) {
 
 	}
 }
+
+type mockProducerResult struct {
+	rt http.RoundTripper
+}
+
+func (m *mockProducerResult) Produce(_ context.Context, r *http.Request, results chan<- *producer.Result) {
+	if m == nil || m.rt == nil {
+		close(results)
+		return
+	}
+
+	res, err := m.rt.RoundTrip(r)
+	results <- &producer.Result{
+		RoundTripName: "default",
+		Beresp:        res,
+		Err:           err,
+	}
+	close(results)
+}
+
+func TestEndpoint_ServeHTTP_FaultyDefaultResponse(t *testing.T) {
+	log, hook := test.NewLogger()
+
+	origin := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		png := []byte(`�PNG
+
+
+IHDRH0=���gAMA���a	pHYs��
�B��tEXtSoftwarePaint.NET v3.5.100�r�pIDAThC���	�0���b!K�$�������1x={+��^��h
+�)��6���z�Qj�h
+�)��0�N4��FS�7l�5��"Ma4��F�=q���ь�FS�7l|�Ұ��nW�i�0IEND�B`)
+
+		rw.Header().Set("Content-Encoding", "gzip")  // wrong
+		rw.Header().Set("Content-Type", "text/html") // wrong
+		rw.Header().Set("Cache-Control", "no-cache, no-store, max-age=0")
+
+		_, err := rw.Write(png)
+		if err != nil {
+			t.Error(err)
+		}
+	}))
+	defer origin.Close()
+
+	rt := transport.NewBackend(
+		test.NewRemainContext("origin", origin.URL), &transport.Config{},
+		&transport.BackendOptions{}, log.WithContext(context.Background()))
+
+	mockProducer := &mockProducerResult{rt}
+
+	ep := handler.NewEndpoint(&handler.EndpointOptions{
+		Context:  hcl.EmptyBody(),
+		Error:    errors.DefaultJSON,
+		Requests: mockProducer,
+		Proxies:  &mockProducerResult{},
+	}, log.WithContext(context.Background()), nil)
+
+	ctx := context.Background()
+	req := httptest.NewRequest(http.MethodGet, "http://", nil).WithContext(ctx)
+	ctx = eval.NewContext(nil, nil).WithClientRequest(req)
+	ctx = context.WithValue(ctx, request.UID, "test123")
+
+	rec := transport.NewRecorder()
+	rw := writer.NewResponseWriter(rec, "")
+	ep.ServeHTTP(rw, req.Clone(ctx))
+	res, err := rec.Response(req)
+	if err != nil {
+		t.Error(err)
+	}
+	if res.StatusCode == 0 {
+		t.Errorf("Fatal error: response status is zero")
+		if res.Header.Get("Couper-Error") != "internal server error" {
+			t.Errorf("Expected internal server error, got: %s", res.Header.Get("Couper-Error"))
+		}
+	} else if res.StatusCode != http.StatusOK {
+		t.Errorf("Expected status ok, got: %v", res.StatusCode)
+	}
+
+	for _, e := range hook.AllEntries() {
+		if e.Level != logrus.ErrorLevel {
+			continue
+		}
+		if e.Message != "backend error: body reset: gzip: invalid header" {
+			t.Errorf("Unexpected error message: %s", e.Message)
+		}
+	}
+
+}

handler/transport/backend.go

@@ -1,9 +1,11 @@
 package transport
 
 import (
+	"bytes"
 	"compress/gzip"
 	"context"
 	"encoding/base64"
+	"io"
 	"net/http"
 	"net/url"
 	"strings"
@@ -115,7 +117,9 @@ func (b *Backend) RoundTrip(req *http.Request) (*http.Response, error) {
 		return nil, err
 	}
 
-	setGzipReader(beresp)
+	if err = setGzipReader(beresp); err != nil {
+		b.upstreamLog.LogEntry().WithContext(req.Context()).WithError(err).Error()
+	}
 
 	if !isProxyReq {
 		removeConnectionHeaders(req.Header)
@@ -310,13 +314,30 @@ func setUserAgent(outreq *http.Request) {
 	}
 }
 
-func setGzipReader(beresp *http.Response) {
-	if strings.ToLower(beresp.Header.Get(writer.ContentEncodingHeader)) == writer.GzipName {
-		if src, err := gzip.NewReader(beresp.Body); err == nil {
-			beresp.Header.Del(writer.ContentEncodingHeader)
-			beresp.Body = eval.NewReadCloser(src, beresp.Body)
-		}
+// setGzipReader will set the gzip.Reader for Content-Encoding gzip.
+// Invalid header reads will reset the response.Body and return the related error.
+func setGzipReader(beresp *http.Response) error {
+	if strings.ToLower(beresp.Header.Get(writer.ContentEncodingHeader)) != writer.GzipName {
+		return nil
+	}
+
+	buf := &bytes.Buffer{}
+	_, err := buf.ReadFrom(beresp.Body) // TODO: may be optimized with limitReader etc.
+	if err != nil {
+		return errors.Backend.With(err)
 	}
+	b := buf.Bytes()
+
+	var src io.Reader
+	src, err = gzip.NewReader(buf)
+	if err != nil {
+		src = bytes.NewBuffer(b)
+		err = errors.Backend.With(err).Message("body reset")
+	}
+
+	beresp.Header.Del(writer.ContentEncodingHeader)
+	beresp.Body = eval.NewReadCloser(src, beresp.Body)
+	return err
 }
 
 // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h.

internal/test/log.go

@@ -7,12 +7,14 @@ import (
 	logrustest "github.com/sirupsen/logrus/hooks/test"
 
 	"github.com/avenga/couper/errors"
+	"github.com/avenga/couper/logging"
 )
 
 func NewLogger() (*logrus.Logger, *logrustest.Hook) {
 	log := logrus.New()
 	log.Out = io.Discard
 	log.AddHook(&errors.LogHook{})
+	log.AddHook(&logging.ContextHook{})
 	hook := logrustest.NewLocal(log)
 	return log, hook
 }

logging/context_hook.go

@@ -0,0 +1,26 @@
+package logging
+
+import (
+	"github.com/sirupsen/logrus"
+
+	"github.com/avenga/couper/config/request"
+)
+
+var _ logrus.Hook = &ContextHook{}
+
+type ContextHook struct {
+}
+
+func (c ContextHook) Levels() []logrus.Level {
+	return logrus.AllLevels
+}
+
+func (c ContextHook) Fire(entry *logrus.Entry) error {
+	_, exist := entry.Data["uid"]
+	if entry.Context != nil && !exist {
+		if uid := entry.Context.Value(request.UID); uid != nil {
+			entry.Data["uid"] = uid
+		}
+	}
+	return nil
+}

main.go

@@ -197,6 +197,7 @@ func newLogger(format string, pretty bool) *logrus.Entry {
 	logger.Out = os.Stdout
 
 	logger.AddHook(&errors.LogHook{})
+	logger.AddHook(&logging.ContextHook{})
 
 	if testHook != nil {
 		logger.AddHook(testHook)

server/writer/response.go

@@ -9,6 +9,7 @@ import (
 	"net/textproto"
 	"strconv"
 
+	"github.com/avenga/couper/errors"
 	"github.com/avenga/couper/eval"
 	"github.com/avenga/couper/logging"
 	"github.com/hashicorp/hcl/v2"
@@ -122,7 +123,13 @@ func (r *Response) WriteHeader(statusCode int) {
 
 	r.configureHeader()
 	r.applyModifier()
-	r.rw.WriteHeader(statusCode)
+
+	if statusCode == 0 {
+		r.rw.Header().Set(errors.HeaderErrorCode, errors.Server.Error())
+		r.rw.WriteHeader(errors.Server.HTTPStatus())
+	} else {
+		r.rw.WriteHeader(statusCode)
+	}
 	r.statusWritten = true
 	r.statusCode = statusCode
 }