ConnStateData::tunnelOnError() ignored its method parameter and always
called initiateTunneledRequest() with METHOD_NONE. buildFakeRequest()
then set HttpRequest::method to METHOD_NONE. Squid does not support such
HttpRequest objects well because quite a bit of code assumes that
HttpRequest::method must be known. For example, depending on
configuration and other factors, Squid may assert.
Moreover, many Squids using the on_unsupported_protocol directive also
have special rules for handling tunnels and those rules may not work as
intended for these METHOD_NONE transactions.
Squid now uses CONNECT method when it creates a CONNECT-like request
that facilitates on_unsupported_protocol tunneling. This helps meet code
expectations about HttpRequest::method being defined and natural admin
expectations about tunneled requests having a CONNECT method.
Admins that want to distinguish on_unsupported_protocol tunnels from
other tunnels can use ACL annotations (for now). If needed, one can add
a better/dedicated way of identifying on_unsupported_protocol tunnels.
Also removed the method parameter from clientTunnelOnError() and related
methods. That method was extracted from a low-level parser field and
- for cases where the higher-level code deemed input to be non-HTTP, it
was wrong to use essentially garbage/non-HTTP chars as a method name;
- for other cases, the method is available via HttpRequest::method.
TODO: We may be able to remove more duplicated parameters or unnecessary
checks on this code path: Perhaps clientReplyContext::setReplyToError()
method parameter can be retrieved from errstate->request? Perhaps
errstate->request itself is always the same as http->request?