[squid-dev] [PATCH] implement RFC3986

[Alex Rousskov]

On 02/20/2016 11:27 AM, Kinkie wrote:

> Sorry to bring this topic up again, but honestly I don't understand
> your position.
> 
> I believe that the deadlock we currently are in

There is no deadlock. I think the design decision to use templated
escape functions to accomodate std::string-using helpers was wrong, but
I am not bold enough to stop you from committing it. Much worse code
goes in despite my objections or without proper audit.


> is caused by the fact
> that we are discussing about two topics at the same time.
> One is: should Rfc3986 target both SBuf and std::string, if the
> performance and complexity penalty in doing so is none or small? I
> (and I believe Amos) think it's a good, or at least not a bad idea.

Any "Should I do X if X is good?" question is pointless. The question is
"Should I do X?". I have already outlined why I think doing X is harmful.


> The other is: should we use std::string or SBuf or c-strings in
> helpers? My opinion is that std::string would be preferable, yours is
> that SBuf is preferable, if any change is performed over the current
> state of using c-strings. I think change over current state is a good
> idea, you - I believe - are averse enough to using std::string that
> would prefer no change.

I think helpers should use whatever works best for them, given the
available Squid APIs and other factors. The question is not about what
helpers should use but whether Squid code should bend over backwards
(e.g., providing a templated function to escape strings and adjust SBuf
to work with that function) to accommodate a helper. The answer, IMO, is
"no". It is much better to

* provide SBuf to helpers that want top performance

and

* provide a trivial std::string-escaping function (that uses
SBuf-escaping function internally) to helpers that want to use C-strings
or std::strings.


> Can we try to decouple the two discussions and see if this helps us
> reach a consensus?
> 
> My point of view is:
> - having a more generic API costs us nothing - the code is compact,
> readable and efficient regardless the genericity, so we should merge
> Rfc3986.

A single template costs virtually nothing. I speculate that the design
decision to treat std::string-using helpers as the primary driving
factor for Squid APIs will cost a lot long-term.

This is very similar to assert(string_size < 64K). The cost of that
single line was nothing when it was written. However, the cost of the
design decision that Strings can self-limit their size is a growing
collection of CVEs. IMO, you are opening a Pandora box. The cost of
lifting the lid is indeed negligible, but that cost is not what I am
worried about.


> - there is no performance or readability benefit in using SBuf in
> helpers due to the way client code is structured.

I agree regarding readability. SBuf is about the same or a little worse
than std::string.

If you are right about performance and safety across all helpers, then
there is no benefit in using SBuf in Squid.


> Also, we would need
> to stub a lot of functionality it relies on (e.g. mempools,
> statistics, cachemgr). 

I am not sure that is true, but even if it is true, it should not be
important for this discussion. The vast majority of such refactoring,
would benefit Squid. For example, if our SBuf code depends on cache
manager, then we made a mistake that should be fixed.

Alex.