[squid-dev] [PREVIEW] Fetch missing certificates

[Alex Rousskov]

On 02/09/2016 09:59 AM, Amos Jeffries wrote:

> "Download" activity is a ::Client class type of activity, which is about
> managing the Squid end of a Squid->server connection.

I am afraid you got it backwards: Downloader is a request source (and a
response sink). Downloader does not actually care about the
Squid-to-server connection! It is up to Squid to manage Downloader
request and deliver the response back to Downloader (with or without the
connection to the origin server or peer).


> I am aware there is a need for the store logics to be involved. But
> surely StoreEntry and/or clientStreams does not require the whole
> ::Server 'side' of Squid to be involved.

Besides StoreEntry and/or clientStreams, download requests should go
through the regular doCallout() processing sequence including StoreID,
ICAP, and other complex things. Thus, I am sure Downloader needs to be
in the same group of classes as ConnStateData.


> Please no more ConnStateData derived classes. ::Server is the head of
> that hierarchy and the servers/ classes as the per-protocol children.

As for the "Server" parent specifically, I cannot yet say whether your
request is valid. As I said earlier in another thread, I need more time
to study what that unaudited class is and what it should be. Based on
the current "used to manage connections from clients" description alone,
it is obviously the wrong parent for Downloader, but I do not claim that
the description itself is correct.

FWIW, the introduction of that Server class is the primary reason we
have not posted the long-polished "Fetch missing certificates" patch for
review...


> Looking at the patch I also see you have added a thing called
> BinaryTokenizer, but the Tokenizer we already have operates on binary
> octets. All that seems needed is some new accessor(s) for fetching the
> fixed-width binary fields alongside the current int64() ASCII->integer
> decoder.

* Tokenizer API is about sequences of characters -- it is built around
CharacterSet. When it retrieves a number, it may retrieve "012345"
characters.

* BinaryTokenizer is about sequences of bits. When it retrieves a
number, it may retrieve 64 bits.

While it all boils down to "binary octets" at some level, it would be a
design mistake to mix the two concepts in one class IMO; just like it
would be a mistake to add "some accessors" to HTTP parser to parse FTP
messages, even though both HTTP and FTP deal with the same kind of input
token streams.

It is possible to create a common Tokenizer parent for TextTokenizer and
BinaryTokenizer, but I think we should wait for BinaryTokenizer to
mature first. It is possible that it (or another *Tokenizer class using
it) will start dealing with not-byte-aligned bits. If that happens,
building a common parent for all tokenizers would be awkward and
expensive performance-wise.


Alex.