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

Fix statusCode 0 write panic #291

Merged
merged 6 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions eval/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions eval/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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),
Expand Down
90 changes: 89 additions & 1 deletion handler/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}
}

}
35 changes: 28 additions & 7 deletions handler/transport/backend.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package transport

import (
"bytes"
"compress/gzip"
"context"
"encoding/base64"
"io"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -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()
alex-schneider marked this conversation as resolved.
Show resolved Hide resolved
}

if !isProxyReq {
removeConnectionHeaders(req.Header)
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions internal/test/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
26 changes: 26 additions & 0 deletions logging/context_hook.go
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 8 additions & 1 deletion server/writer/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down