Return HTTP response to RX_OVERFLOW
Resolves https://github.com/varnishcache/varnish-cache/issues/2735
Before this PR, Varnish would silently close the connection to a client that triggered an RX_OVERFLOW (too long URI), this PR allows configuring Varnish to respond to RX_OVERFLOW with a minimal HTTP response that will contain a chosen HTTP code instead.
Its done by adding a new feature flag in order to enable the feature and a new parameter in order to set the HTTP code that will be sent in the response.
bugwash: we would appreciate some more research regarding which scenarios would benefit from a similar mechanism. Also, the argument has been made that, once we send a response at all, we might as well do so through vcl_synth, but then the question arises what to present to VCL as the request (url, proto, headers).
@cartoush volunteered to spend some time on researching these questions, thank you!
bugwash: we would appreciate some more research regarding which scenarios would benefit from a similar mechanism. Also, the argument has been made that, once we send a response at all, we might as well do so through
vcl_synth, but then the question arises what to present to VCL as the request (url, proto, headers).@cartoush volunteered to spend some time on researching these questions, thank you!
Sorry for the delay, I did some thinking on that and I think one could argue all cases where an error causes a connection to close silently could benefit from that.
But generalizing that using vcl_synth might not be appropriate, as you can do restart for example which be quite dangerous, something better could be creating a vcl_invalid subroutine which only gives access to what's relevant to handle the error.
In this case the default vcl behavior would be:
sub vcl_invalid {
return (close);
}
and in order to handle the case here we would have something like:
sub vcl_invalid {
if (req.error.reason == REQ_HTTP_OVERFLOW) {
resp.status == 413;
return (deliver);
}
return (close);
}
I think the the whole req should not be usable in vcl_invalid but req.error should be populated with all necessary information to do the processing.
But then this would have to take place in another PR as this requires quite a bit of planning in order to decide what can be done in vcl_invalid.
bugwash: rework to parameter only, then OK
This is not as trivial as I thought to support the status range 400..499 or 0.
has been discussed again, but the decision remains the same...
I renamed the parameter to http_req_overflow_status for consisteny with http_req_size.