> On Oct 1, 2013, at 1:13 AM, Laurens Van Houtven <_...@lvh.io> wrote:
>
>> On Tue, Oct 1, 2013 at 12:59 AM, Glyph <gl...@twistedmatrix.com> wrote:
>>
>> Most of the code I can think of that wants to use that really wants the
>> transport rather than the "protocol",
>
> Yes, but having the protocol would also immediately give you access to the
> transport, and, from what I understand in most cases of AMP, also everything
> else :)
The problem here is that you can do AMP decoding without a protocol anywhere in
sight. A CommandLocator by itself, passed appropriate boxes to the callables
returned by locateResponder, is capable of speaking AMP just fine, as long as
you don't care about speaking it to an actual byte stream :-). (And ostensibly
this is one way to speak AMP over transports other than BinaryBoxProtocol.)
In the case of AMP routes (something it would be very nice to integrate into
the main protocol), you have multiple command locators per transport, and each
one might have its own properties that would be interesting to Argument types,
This is why putting requirements on the thing-doing-the-parsing in the Argument
definition makes sense; the Command could interrogate its prospective Responder
class, asking each of its Argument objects if it will be able to satisfy their
requirements in turn, at the time that CommandLocator.__metaclass__.__new__
gets invoked, rather than once your protocol is already trying to respond to
commands.
>> but nothing within AMP itself actually uses those arguments; in fact,
>> searching the usual suspects (epsilon, vertex) I can't even find any
>> Arguments that use the 'proto' argument for anything useful.
>
> I suppose it's too late to get "proto" to actually mean "proto" and not
> "boxSender"?
I believe it currently means "responderLocator". _wrapWithSerialization is
where the magic happens if you want to confirm.
> It would definitely be a backwards-incompatible change, and I do actually
> have some code that somehow relied on it being the boxSender (actually, I
> think I saved that code in txampext, mostly, except I renamed that thing
> "proto").
It's probably too late for this method, but you can always add a new one :-)
>> If I recall, I believe the idea behind it was to allow an AMP responder
>> within Vertex to return the peer's IP address back to the peer, from within
>> an authenticated AMP route that (because it was a route) wasn't necessarily
>> connected directly to the transport (and therefore couldn't just do
>> self.transport.getPeer()). Ironically I don't think it'll actually work for
>> that now :-).
>>
>> When we pull the authentication logic in from
>> <http://bazaar.launchpad.net/~divmod-dev/divmod.org/trunk/view/head:/Epsilon/epsilon/ampauth.py>,
>> you might write a responder that's interested in authentication information
>> that lives in some other relation to the protocol.
>
> I wrote very similar deep-in-AMP auth logic once, and did look at that code
> (but ended up not using it because I use TLS, so I don't need hand-rolled
> challenge response or OTP systems).
The hand-rolled CR/OTP crap is really not supposed to be the interesting part.
Really, that should just be SASL. The interesting part is the integration with
cred.
(Augh, and we didn't know the difference between a one-time password and a
one-time pad, and the wrong word is right there in the wire format... Augh augh
augh)
> When you say "other relation to the protocol", does that mean "it can be the
> protocol because the protocol will have some kind of reference to it"?
Notice that in that authentication code, CredReceiver *sets self.boxReceiver*
to the result of portal.login. It hands off processing of the parsed AMP boxes
to another IBoxReceiver. So the thing parsing the commands - the
post-authentication protocol - is what the Arguments will currently have direct
access to (as that will be the CommandLocator as well) but that object will
have no transport; only a boxSender.
>> So in order to fix fromBox/toBox, we need to do a fix that firms up that
>> contract and perhaps exposes more than a Protocol object. The *recommended*
>> API should be more or less like what ExposingArgument is doing - specify an
>> Argument that asks for a particular attribute of the transport or the
>> protocol or the authentication context or whatever, the implementation
>> details may involve other lower-level public APIs.
>
> That still sounds like it can be done by making "proto" actually the proto
> ;-) So, basically, the question is if "proto" being the locator is a bug that
> I can fix, or an interface that I can't.
You could add a new interface where it's "fixed", but given the case I just
described above, what does "fixed" mean?
>>> My contributions to AMP have been more of the defect-findy kind, but I
>>> could certainly turn them more into the code-contributy kind. I imagine I'm
>>> not the first person to want tests for command classes
>>> (https://github.com/lvh/txampext/blob/master/txampext/commandtests.py) or a
>>> nested AMP box
>>> (https://github.com/lvh/txampext/blob/master/txampext/nested.py).
>>
>> That would be cool. And, you know, that auth thing I said :-).
>
> If I can change "proto" to mean "actually the protocol not something else"
> then that seems plenty easy to add, and it would definitely be cool if people
> don't have to mess with this nonsense themselves for something as ostensibly
> simple as having access to the protocol :-)
Keep in mind that in the authentication case I mentioned, your post-auth object
may well subclass AMP and therefore "actually" be a protocol; but it still
won't have a transport. What do you propose happen in that case?
-g
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
