Re: [RFC] FTP gw source layout

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Wed, 22 Jan 2014 11:54:12 +1300

On 2014-01-22 10:37, Alex Rousskov wrote:
> [ A specific revised proposal is at the end of this email, after the
> discussion. ]
>
>
> On 01/20/2014 03:30 PM, Amos Jeffries wrote:
>> On 2014-01-21 07:45, Alex Rousskov wrote:
>>> I propose the following arrangement for the official submission:
>>>
>>> src/out/FtpServer.* # code shared by old and new FTP clients
>>> src/out/FtpToHttp.* # old FTP client
>>> src/out/FtpGateway.* # new FTP gw client
>>> src/in/FtpGateway.* # new FTP gw server
>>> src/ftp/* # FTP code shared by client and server sides
>
>
>> These are all backwards conceptually.
>
> Yes, courtesy of the established "Squid 'server side' is for clients
> that we are going to call servers" terminology in the existing Squid
> code.
>

I'm looking at a higher level of abstraction than those quirks. Even if
one swaps the client/server terms backwards these terminology have no
reason to change (ie. one gateways from server to client as much as from
client to server).

[ On a side note shall we make a proper effort to fix that confusion by
deprecating our use of the terms and call bits client or server by
functionality? it would be as easy to describe these area-based parts as
client-facing or server-facing areas of Squid. We already talk of ICAP
as a "client" despite being both "client-side" and "server-side".

Your updated proposal below would seem to be a good move in that
direction. If we can agree to stop saying/writing those terms and start
replacing documentation we can probably make naming decisions a lot less
confusing in the near future. ]

>
>> * Gateway - translating in/ transport to out/ transport. Seems to
>> describe what the old client code does pretty closely both against the
>> dictionary and RFC terminology definitions for gateway.
>
>> * Relay - passing in/ transport to same out/ transport. Seems to
>> describe what the new code does if we ignore the messy internal
>> representation mapping.
>
> I did try to figure out what FTP standards or other authoritative
> FTP-specific documents say about this, and found pretty much nothing.
> In
> a general sense, not specific to the established FTP terminology
> (whatever it is!), I agree that "relay" is a more suitable term for
> what
> we are doing.

FTP has no protocol concept of proxying. So I would not expect anything
to exist in those. This is generic/abstract systems terminology so you
may find the terms defined in dictionaries, the basic Internet RFC the
cover terminology or specific protocols like HTTP/DNS/SMTP/SIP which
have the concept of proxying.

>
> Keep in mind that "relay" (with various filtering capabilities offered
> by Squid) is essentially a "proxy". This project was called Native FTP
> Proxy.

Essentially is the word. With one fine distinction: Relay is transparent
in the protocol (essentially 'dumb'), Proxy may adjust and make changes
to the message semantics as they go through.

>
> [FWIW, the term "FTP Gateway" was suggested by Henrik during initial
> RFC
> review. Henrik thought that using HTTP semantics internally means we
> are
> a "gateway". I changed the project name in order to avoid having an
> argument. Technically, it is not really clear whether we are using HTTP
> semantics internally or not. We are using HTTP structures, but
> semantics
> is a very gray area IMO.]

[ This last point is one reason I really want to make the move from a
structures based internal representation to passing around frames. That
was we can simply call each of these FTP messages a frame (might even
stay in FTP native format) and be done with it. ]

>
> I am OK with Relay if others think it is better in this context, but I
> may have an even better suggestion below.

Relay is fine with me.

>
>> * out/FtpServer contains FTP clients. Lets not do this.
>
> We are already doing that for the existing server-side code, which is
> all based on the Server[StateDataInfo] class. On one hand, I think we
> should be consistent: Either rename every class or keep the current
> naming scheme, even if we know it is flawed. On the other hand, causing
> and dealing with so much renaming pain just for the sake of consistency
> feels wrong.

Unless we stop with the improving of code we are going to go through the
pain eventually and spread over an arbitrary span of time. The
SourceLayout project alone seeks to do that "rename every class", either
by actual rename or replacement with a better class.

>
> My revised proposal below lessens that pain at the expense of some
> inconsistency with the old code.
>
>
>> Yes it does kind
>> of make sense if one has a lot of knowledge about Squid. But for most
>> it
>> will be amazingly confusing. The "gadgets" terminology we started
>> using
>> fits here, so I propose FtpGadgets.* to help reduce confusion.
>
> "Gadgets" names a collection of handy little things. It does not work
> at
> all for a base FTP client class IMO.

Okay. Anything better? or are we stuck with FtpStateData for a while
longer?

>
>>> One alternative worth considering is accumulating FTP agent (client
>>> or
>>> server) code in their own directories, better separating them from
>>> HTTP
>>> agents (and agents for other protocols on the same side), even though
>>> we
>>> have not done that for HTTP yet:
>>>
>>> src/out/ftp/Server.* # code shared by old and new FTP clients
>>> src/out/ftp/ToHttp.* # old FTP client
>>> src/out/ftp/Gateway.* # new FTP gw client
>>> src/in/ftp/Gateway.* # new client-side FTP gw code
>>
>> For now I think we leave this, it can be done later if we end up with
>> similar models for other protocols. (FYI: I kind of dislike the whole
>> fake-HTTP request mapping, but lacking a better alternative am not
>> arguing against it at this point).
>
> Same here.
>
>
>
>> Agreed. I am still finding all the names we can think of feel a bit
>> contrived. At least in/ and out/ are shorter and fit by way of being
>> more general than our somewhat squid-specific terminology. We can
>> always
>> rename later if something better is thought up.
>
> OK, so given your, Christos, and Henrik responses (thank you
> everybody!), I revise my proposal as follows:
>
>
> 1) The new Feature is going to be called "Native FTP Proxy". The old
> "FTP Gateway" code name is dropped.
>

Ok.

> 2) We are going to use the following layout for the FTP code:
>
> src/servers/FtpServer.* # new FTP server, relaying FTP
> src/clients/FtpClient.* # code shared by old and new FTP clients
> src/clients/FtpGateway.* # old FTP client, translating back to HTTP
> src/clients/FtpNative.* # new FTP client, relaying FTP
> src/ftp/* # FTP stuff shared by clients and servers
>

Ok.

> 3) The remaining non-FTP code will be eventually moved into appropriate
> files and directories inside src/clients/ and src/servers/ structure,
> and the corresponding files/classes will be renamed at that time. Doing
> so is outside the FTP Gateway project scope and is likely to happen one
> class (or one group of files) at a time.

Sure.

+1 on this proposal.

>
>
> Thank you,
>
> Alex.
>>> [1] http://wiki.squid-cache.org/Features/FtpGateway
>>> [2] https://code.launchpad.net/~measurement-factory/squid/ftp-gw
>>> [3]
>>> http://www.squid-cache.org/mail-archive/squid-dev/201303/0054.html
Received on Tue Jan 21 2014 - 22:54:34 MST

This archive was generated by hypermail 2.2.0 : Wed Jan 22 2014 - 12:00:13 MST