Skip to content

Commit

Permalink
buffer options (#710)
Browse files Browse the repository at this point in the history
* test for error handler affecting endpoint's buffer options

* fix: error handler affecting endpoint's buffer options

* changelog entry
  • Loading branch information
johakoch authored Feb 22, 2023
1 parent be46a07 commit 8eccd50
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* Erroneously sending an empty [`Server-Timing` header](https://docs.couper.io/configuration/command-line#oberservation-options) ([#700](https://github.com/avenga/couper/pull/700))
* url scheme while using the [`tls` block](https://docs.couper.io/configuration/block/server_tls) ([#703](https://github.com/avenga/couper/issues/703))
* For [OIDC](https://docs.couper.io/configuration/block/oidc), trying to request userinfo from a non-existing (not required, though recommended) userinfo endpoint ([#709](https://github.com/avenga/couper/pull/709))
* Use of [`backend_responses`'](https://docs.couper.io/configuration/variables#backend_responses) `body` or `json_body` properties in api-level [error handlers](https://docs.couper.io/configuration/block/error_handler) ([#710](https://github.com/avenga/couper/pull/710))
* Some `..._file` attributes missing for path absolutizing ([#713](https://github.com/avenga/couper/pull/713))
* `WWW-Authenticate` header `realm` param value for [`basic_auth`](https://docs.couper.io/configuration/block/basic_auth) ([#715](https://github.com/avenga/couper/pull/715))

Expand Down
8 changes: 5 additions & 3 deletions config/runtime/error_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import (
)

func newErrorHandler(ctx *hcl.EvalContext, conf *config.Couper, opts *protectedOptions, log *logrus.Entry,
defs ACDefinitions, references ...string) (http.Handler, error) {
defs ACDefinitions, references ...string) (http.Handler, eval.BufferOption, error) {
kindsHandler := map[string]http.Handler{}
var ehBufferOption eval.BufferOption
for _, ref := range references {
definition, ok := defs[ref]
if !ok {
Expand Down Expand Up @@ -65,7 +66,7 @@ func newErrorHandler(ctx *hcl.EvalContext, conf *config.Couper, opts *protectedO

epOpts, err := NewEndpointOptions(ctx, epConf, nil, opts.srvOpts, log, conf, opts.memStore)
if err != nil {
return nil, err
return nil, eval.BufferNone, err
}
if epOpts.ErrorTemplate == nil || h.ErrorFile == "" {
epOpts.ErrorTemplate = opts.epOpts.ErrorTemplate
Expand All @@ -83,8 +84,9 @@ func newErrorHandler(ctx *hcl.EvalContext, conf *config.Couper, opts *protectedO
epOpts.LogHandlerKind = "error_" + k
epOpts.IsErrorHandler = true
kindsHandler[k] = handler.NewEndpoint(epOpts, log, nil)
ehBufferOption |= epOpts.BufferOpts
}
}

return handler.NewErrorHandler(kindsHandler, opts.epOpts.ErrorTemplate), nil
return handler.NewErrorHandler(kindsHandler, opts.epOpts.ErrorTemplate), ehBufferOption, nil
}
7 changes: 4 additions & 3 deletions config/runtime/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
if parentAPI != nil && parentAPI.CatchAllEndpoint == endpointConf {
protectedHandler = epOpts.ErrorTemplate.WithError(errors.RouteNotFound)
} else {
epErrorHandler, err := newErrorHandler(confCtx, conf, &protectedOptions{
epErrorHandler, ehBufferOption, err := newErrorHandler(confCtx, conf, &protectedOptions{
epOpts: epOpts,
memStore: memStore,
srvOpts: serverOptions,
Expand All @@ -343,6 +343,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
}
if epErrorHandler != nil {
epOpts.ErrorHandler = epErrorHandler
epOpts.BufferOpts |= ehBufferOption
}
epHandler = handler.NewEndpoint(epOpts, log, modifier)

Expand All @@ -355,7 +356,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
protectedHandler = epHandler
} else {
permissionsControl := ac.NewPermissionsControl(requiredPermissionExpr)
permissionsErrorHandler, err := newErrorHandler(confCtx, conf, &protectedOptions{
permissionsErrorHandler, _, err := newErrorHandler(confCtx, conf, &protectedOptions{
epOpts: epOpts,
memStore: memStore,
srvOpts: serverOptions,
Expand Down Expand Up @@ -646,7 +647,7 @@ func configureProtectedHandler(m ACDefinitions, conf *config.Couper, ctx *hcl.Ev
opts *protectedOptions, log *logrus.Entry) (http.Handler, error) {
var list ac.List
for _, acName := range parentAC.Merge(handlerAC).List() {
eh, err := newErrorHandler(ctx, conf, opts, log, m, acName)
eh, _, err := newErrorHandler(ctx, conf, opts, log, m, acName)
if err != nil {
return nil, err
}
Expand Down
19 changes: 13 additions & 6 deletions server/http_error_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,12 @@ func TestErrorHandler_Backend(t *testing.T) {
}
}

func Test_StoreInvalidBackendResponse(t *testing.T) {
func Test_ErroneousBackendResponse(t *testing.T) {
client := test.NewHTTPClient()

shutdown, hook := newCouper("testdata/integration/error_handler/06_couper.hcl", test.New(t))
defer shutdown()

type testcase struct {
name string
file string
path string
expBody string
expStatus int
Expand All @@ -221,9 +220,13 @@ func Test_StoreInvalidBackendResponse(t *testing.T) {
}

for _, tc := range []testcase{
{"/anything", `{"req_path":"/anything","resp_ct":"application/json","resp_json_body_query":{},"resp_status":200}`, 418, 200, "status is not supported"},
{"store invalid backend response", "06_couper.hcl", "/anything", `{"req_path":"/anything","resp_ct":"application/json","resp_json_body_query":{},"resp_status":200}`, 418, 200, "status is not supported"},
{"api-level error handlers affect endpoint's buffer options", "08_couper.hcl", "/anything", `{"resp_json_status":200}`, 418, 200, ""},
} {
t.Run(tc.path, func(st *testing.T) {
shutdown, hook := newCouper("testdata/integration/error_handler/"+tc.file, test.New(t))
defer shutdown()

helper := test.New(st)
hook.Reset()

Expand Down Expand Up @@ -259,7 +262,11 @@ func Test_StoreInvalidBackendResponse(t *testing.T) {
func getBackendLogStatusAndValidation(hook *logrustest.Hook) (int, string) {
for _, entry := range hook.AllEntries() {
if entry.Data["type"] == "couper_backend" {
return entry.Data["status"].(int), entry.Data["validation"].([]string)[0]
var validation string
if val, ok := entry.Data["validation"].([]string); ok && len(val) == 1 {
validation = val[0]
}
return entry.Data["status"].(int), validation
}
}

Expand Down
28 changes: 28 additions & 0 deletions server/testdata/integration/error_handler/08_couper.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
server {
api {
endpoint "/anything" {
request "r" {
url = "${env.COUPER_TEST_BACKEND_ADDR}/"
backend {
timeout = "1ns"
}
body = "foo"
}

proxy {
backend {
origin = "${env.COUPER_TEST_BACKEND_ADDR}"
}
}
}

error_handler "backend_timeout" {
response {
status = 418
json_body = {
resp_json_status = backend_responses.default.json_body.ResponseStatus
}
}
}
}
}

0 comments on commit 8eccd50

Please sign in to comment.