On 31/07/2013 7:22 p.m., Tsantilas Christos wrote:
On 07/30/2013 11:15 PM, Amos Jeffries wrote:
On 31/07/2013 6:47 a.m., Tsantilas Christos wrote:
On 07/30/2013 09:17 PM, Amos Jeffries wrote:
However Amos refers to an other case. For the following line:
"Simple Tokens"
we may want to retrieve the token "Simple
Do we have any example where this is required? (Not for regex, for
regex
we have an exception...)
In most places where we accept header values in the legacy parser.
Well the request_header_add already understand quoted strings.
The parse_http_header_replace should use the
ConfigParser::NextQuotedOrToEol.
I just show that there is a problem here. It will work as is. But the
code is not correct....
I don't think it will work as-is. If I were to be replacing ETag headers
they contain a single value with "" around it. Or any other header
holding a quoted-string field value, this is more likely with custom
headers. Then the NextToken it is currently using will corrupt the
resulting HTTP traffic.
The other functions hae only header names...
The ACLs doing matching on header values are all regex I think.
The ACLs yes.
But we are already planning to handle regexs as special cases ...
I have seen a few people who have things like "quoted string" for auth
realm, and since the old squid.conf sent the quotes out as-is they ended
up with helpers and browsers security records using the quoted form.
There might be any amount of unknown pain ahead if we suddenly start
stripping quotes off strings in the backward compatible parse mode.
The realm uses the parse_eol which uses the
ConfigParser::NextQuotedOrToEol method which handles such cases....
The point was that the "" are now being stripped whereas before they
were not.
This is true if the realm start from "
When the new parse mode is enabled that is fine. When the mode is
disabled we are likely breaking things.
If there is anything important which requires those quotes to be sent in
the HTTP request then the change will break it. The obvious case is
Digest auth where the realm is hashed into the nonces as-is and
federated remote servers may store that hashed value for re-use. If
Squid is no longer sending the same realm hash logins will fail in an
almost undetectable way.
There are people also doing similar things using the Basic auth realm in
nasty ways so I would not be surprised if a quote changing there broken
stuff for them as well.
OK.
What I am hearing is that ConfigParser::NextQuotedOrToEol() is missing a
check for ConfigParser::RecognizeQuotedValues to determine whether it
treats quoted values as the "ToEol" case. Anything which is using it
that used to have quoted-string support may need to preserve
RecognizeQuotedValues, set it to true, run the function then reset it to
the old value.
An opinion here.
Should the quoted-strings support enabled/disabled using the
configuration_includes_quoted_values on/off
configuration parameter, or do you believe that it should hardcoded in
realms parsing for example?
For example, should we use the following:
configuration_includes_quoted_values off
auth_param basic realm "Squid proxy-caching web server"
configuration_includes_quoted_values on
I that would prevent the intentional update to quoted-values for that
option.
Or just implement a new ConfigParser::NextToEol() method for such cases?
I think that making NextQuotedOrToEol() obey the RecognizeQuotedValues
flag will solve all these cases. The behaviour changing when it is
altered by the admin is not a big issue if we document it clearly which
ones are affected.
The problem here is more from changing the default behaviour silently,
than from what it is changed to.
Amos
