-
Notifications
You must be signed in to change notification settings - Fork 15
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
Heimdall "silently" responses with an error if the to be processed request exceeds the max buffer size #880
Comments
There is unfortunately no hook, which could be set to log a 431 error. And there are more like this. The responsible HTTP server implementation includes the following lines of code: for {
w, err := c.readRequest(ctx)
if c.r.remain != c.server.initialReadLimitSize() {
// If we read any bytes off the wire, we're active.
c.setState(c.rwc, StateActive, runHooks)
}
if err != nil {
const errorHeaders = "\r\nContent-Type: text/plain; charset=utf-8\r\nConnection: close\r\n\r\n"
switch {
case err == errTooLarge:
// Their HTTP client may or may not be
// able to read this if we're
// responding to them and hanging up
// while they're still writing their
// request. Undefined behavior.
const publicErr = "431 Request Header Fields Too Large"
fmt.Fprintf(c.rwc, "HTTP/1.1 "+publicErr+errorHeaders+publicErr)
c.closeWriteAndWait()
return
case isUnsupportedTEError(err):
// Respond as per RFC 7230 Section 3.3.1 which says,
// A server that receives a request message with a
// transfer coding it does not understand SHOULD
// respond with 501 (Unimplemented).
code := StatusNotImplemented
// We purposefully aren't echoing back the transfer-encoding's value,
// so as to mitigate the risk of cross side scripting by an attacker.
fmt.Fprintf(c.rwc, "HTTP/1.1 %d %s%sUnsupported transfer encoding", code, StatusText(code), errorHeaders)
return
case isCommonNetReadError(err):
return // don't reply
default:
if v, ok := err.(statusError); ok {
fmt.Fprintf(c.rwc, "HTTP/1.1 %d %s: %s%s%d %s: %s", v.code, StatusText(v.code), v.text, errorHeaders, v.code, StatusText(v.code), v.text)
return
}
publicErr := "400 Bad Request"
fmt.Fprintf(c.rwc, "HTTP/1.1 "+publicErr+errorHeaders+publicErr)
return
}
}
// further code For each of the errors handled in the switch statement, there is no way to have it logged by the implementation making use of that code |
There is a corresponding FR to allow logging in such cases. As long as there is no solution for this, there is no way to resolve the corresponding issue with heimdall. |
Preflight checklist
Describe the bug
This is actually not really a bug, but an annoying observability shortcoming.
If the request to heimdall (e.g. if heimdall is used as a proxy or a decision service) exceeds 4kB, which is the default for both, the request and the response buffers, heimdall responses with 431 without emitting any log statements.
Depending on the usage scenario, it is difficult to understand why the call to an upstream failed. Typically, there is a need to increase the log level of the proxy in front of heimdall to understand what went wrong.
How can the bug be reproduced
X-Too-Big
set to a value 4kB of size.Relevant configuration
To overcome that issue, there is a need to increase the buffer limits, e.g. like shown below.
Version
0.11.1-alpha
On which operating system are you observing this issue?
Linux
In which environment are you deploying?
Other
Additional Context
Affects any type of deployment
The text was updated successfully, but these errors were encountered: