On 18/07/2013 4:12 p.m., Alex Rousskov wrote:
On 07/17/2013 08:16 PM, Amos Jeffries wrote:
On 18/07/2013 1:36 p.m., Alex Rousskov wrote:
On 07/06/2013 08:22 AM, Amos Jeffries wrote:
+ debugs(3, DBG_CRITICAL, "FATAL: " <<
URLScheme(s->transport.protocol).const_str() << "_port: missing ']'
on IPv6 address: " << token);
+ debugs(3, DBG_CRITICAL, "FATAL: " <<
URLScheme(s->transport.protocol).const_str() << "_port: missing Port
in: " << token);
+ debugs(3, DBG_CRITICAL, "FATAL: " <<
URLScheme(s->transport.protocol).const_str() << "_port: IPv6 is not
available.");
+ debugs(3, 3, URLScheme(s->transport.protocol).const_str() <<
"_port: found Listen on Port: " << port);
+ debugs(3, DBG_CRITICAL, "FATAL: " <<
URLScheme(s->transport.protocol).const_str() << "_port: missing Port:
" << token);
+ debugs(3, DBG_CRITICAL, "FATAL: " <<
URLScheme(s->transport.protocol).const_str() << "_port: Port cannot
be 0: " <<
...
Can you use ostream << printing of ProtocolType instead of repeating
these manipulations?
Unfortunately no ...
Then please add a simple function or method to encapsulate these
repeated manipulations.
Maybe I should explain.
* Adding the operator claims collision with a default
ostream::operator<<(unsigned int).
* ProtocolType being an enum *always* require manipulations to display
its string equivalent.
* any given call to the parse function will only hit one of these
debugs() AND may have a different transport.protocol value, even if the
PortCfg is the same.
* the URLScheme().const_str() *is* the wrapper keeping the repeated
manipulation actions away from the above code lines.
I can try adding an operator << to the URLScheme object to get rid of
the const_str() part, but the rest is needless optimization.
This both limits the possible parameter values to one of HTTP, HTTP/1.1,
HTTPS, or HTTPS/1.1
+ * Unknown transport type representations are ignored.
Should not Squid reject invalid configurations that use protocols that
Squid does not understand and [no longer] uses? What will happen if an
admin makes a one-key-off typo and says protocol=httpd in squid.conf?
Will Squid behavior in that case change after your patch?
Yes. Previously Squid would silently accept it and general URLs with
"httpd://" scheme names to upstream proxies, resulting in rejected
traffic and some problems outside of the broken proxy.
After patch Squid will report the WARNING at level 1 (and -k parse) then
use the previous (usually default) scheme for that port. Ensuring that
all traffic generated from the proxy is at least a valid supported
protocol, instead of causing problems for the upstream network(s).
As always, I think Squid should reject the invalid config instead of
generating traffic that is likely to differ from what the admin wants.
Okay, fine by me. self_destruct instead of debugs() removes a dependency
issues I hit when building this on clang. So I'll go along with that.
Amos
