Skip to content

Implement response plugins to run on all responses, including error c… #7689

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.

Sign up for GitHub

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

Open
sedkis wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement response plugins to run on all responses, including error c… #7689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
103 changes: 91 additions & 12 deletions gateway/handler_error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@

var TykErrors = make(map[string]config.TykError)

type responseChainContextKey struct{}

var responseChainContextKeyValue = responseChainContextKey{}

func errorAndStatusCode(errType string) (error, int) {
err := TykErrors[errType]
return errors.New(err.Message), err.Code
Expand Down Expand Up @@ -70,6 +74,46 @@
}
}

func applyResponseWriterHeaderDelta(before, after, target http.Header) {
if target == nil {
return
}

for key, afterValues := range after {
beforeValues, ok := before[key]
if !ok || !stringSliceEqual(beforeValues, afterValues) {
target[key] = cloneHeaderValues(afterValues)
}
}

for key := range before {
if _, ok := after[key]; !ok {
delete(target, key)
}
}
}

func stringSliceEqual(a, b []string) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}

func cloneHeaderValues(values []string) []string {
if values == nil {
return nil
}
out := make([]string, len(values))
copy(out, values)
return out
}

// APIError is generic error object returned if there is something wrong with the request
type APIError struct {
Message htmltemplate.HTML
Expand Down Expand Up @@ -110,8 +154,6 @@
}

w.Header().Set(header.ContentType, contentType)
response.Header = http.Header{}
response.Header.Set(header.ContentType, contentType)
templateName := "error_" + strconv.Itoa(errCode) + "." + templateExtension

// Try to use an error template that matches the HTTP error code and the content type: 500.json, 400.xml, etc.
Expand All @@ -128,27 +170,23 @@
templateName = defaultTemplateName + "." + defaultTemplateFormat
tmpl = e.Gw.templates.Lookup(templateName)
w.Header().Set(header.ContentType, defaultContentType)
response.Header.Set(header.ContentType, defaultContentType)

}

//If the config option is not set or is false, add the header
if !e.Spec.GlobalConfig.HideGeneratorHeader {
w.Header().Add(header.XGenerator, "tyk.io")
response.Header.Add(header.XGenerator, "tyk.io")
}

// Close connections
if e.Spec.GlobalConfig.CloseConnections {
w.Header().Add(header.Connection, "close")
response.Header.Add(header.Connection, "close")

}

response.Header = cloneHeader(w.Header())

// If error is not customized write error in default way
if errMsg != errCustomBodyResponse.Error() {
w.WriteHeader(errCode)
response.StatusCode = errCode
var tmplExecutor TemplateExecutor
tmplExecutor = tmpl

Expand All @@ -162,11 +200,52 @@
tmplExecutor = rawTmpl
}

var log bytes.Buffer
var errBody bytes.Buffer

tmplExecutor.Execute(&errBody, &apiError)

Check failure on line 205 in gateway/handler_error.go

View workflow job for this annotation

GitHub Actions / lint 

Error return value of `tmplExecutor.Execute` is not checked (errcheck)

rsp := io.MultiWriter(w, &log)
tmplExecutor.Execute(rsp, &apiError)
response.Body = ioutil.NopCloser(&log)
response.StatusCode = errCode
response.Body = ioutil.NopCloser(bytes.NewReader(errBody.Bytes()))
response.Request = r

Check warning on line 210 in gateway/handler_error.go

View check run for this annotation

probelabs / Visor: architecture

logic Issue 

An error from reading the modified response body from a response plugin is silently ignored. If a plugin provides a faulty response body stream (e.g., an error-producing io.ReadCloser), the gateway will fall back to sending the original error body without any indication that something went wrong in the response chain.
Raw output 
Log the error from `io.ReadAll` to provide visibility into failures within response plugins. This will aid in debugging custom plugins that may have issues generating a response body.
if len(e.Spec.ResponseChain) > 0 && r.Context().Value(responseChainContextKeyValue) != true {
writerHeadersBefore := cloneHeader(w.Header())

setCtxValue(r, responseChainContextKeyValue, true)

Check warning on line 214 in gateway/handler_error.go

View check run for this annotation

probelabs / Visor: security

security Issue 

The code reads the entire response body from a response plugin using `io.ReadAll` without any size limit. A buggy or malicious plugin could provide an infinitely streaming body, causing the gateway to exhaust its memory and leading to a Denial of Service (DoS). The gateway's error handling mechanism should be resilient against misbehaving plugins.
Raw output 
Limit the amount of data read from the plugin-provided response body. Wrap `response.Body` in an `io.LimitedReader` before calling `io.ReadAll` to enforce a maximum size for the error response body. A sane default limit, such as 1MB, should be used.
defer setCtxValue(r, responseChainContextKeyValue, false)

Check warning on line 215 in gateway/handler_error.go

View check run for this annotation

probelabs / Visor: performance

performance Issue 

The response body from the response chain is fully read into memory using `io.ReadAll`. If a response plugin returns a large or streaming body, this will cause high memory consumption and negate the benefits of streaming. While error responses are typically small, plugins can now modify them, potentially making them large.
Raw output 
To support streaming and reduce memory usage, consider using `io.TeeReader` to simultaneously write the response to the client and capture it for analytics, instead of buffering the entire body in memory first. This would involve streaming the body directly to the `http.ResponseWriter` using `io.Copy` after the headers have been written.

session := ctxGetSession(r)
handled, err := handleResponseChain(e.Spec.ResponseChain, w, response, r, session)
if handled {
// Custom response hook errors invoke their own ErrorHandler (with analytics).
return
}
if err != nil {
e.Logger().Error("Response chain failed! ", err)
}

errCode = response.StatusCode
if response.Body != nil {
if modifiedBody, err := io.ReadAll(response.Body); err == nil {
errBody.Reset()
errBody.Write(modifiedBody)
}
_ = response.Body.Close()
}

applyResponseWriterHeaderDelta(writerHeadersBefore, w.Header(), response.Header)

for k := range w.Header() {
w.Header().Del(k)
}
copyHeader(w.Header(), response.Header, e.Gw.GetConfig().IgnoreCanonicalMIMEHeaderKey)
}

w.WriteHeader(errCode)
response.StatusCode = errCode
w.Write(errBody.Bytes())

Check failure on line 246 in gateway/handler_error.go

View workflow job for this annotation

GitHub Actions / lint 

Error return value of `w.Write` is not checked (errcheck)
response.ContentLength = int64(errBody.Len())
response.Body = ioutil.NopCloser(bytes.NewReader(errBody.Bytes()))
}
}

Expand Down
Loading
Loading