>> string "fixed-x11-req%rodents.montreal.qc.ca@localhost"
> You could rename "token" to "display id", as it's meant to identify
> an X11 display.
Calling it "display id" would, I suspect, confuse people into thinking
it's meant to be the display number (eg, the 2 in "hostname:2.0").
>> byte[6] first six bytes of the client X setup packet (the byte
>> order and version numbers)
> I think I understand why you want to include the part of the initial
> X message that comes before the authentication data, but it does make
> the request more dependent on X protocol details than I really like.
You do have a good point there. I hadn't really thought about it much;
even though I have trouble imagining this causing any problem (I don't
really think X is going to change that in the foreseeable future), I do
agree that the two protocols shouldn't be intertwined this much.
While it seems silly from an efficiency point of view, I'm now inclined
to remove that from the CHANNEL_OPEN entirely and put it in the channel
data stream, probably with zero-length authorization data (that is, the
ssh protocol as it appears on the wire then knows nothing about the X11
protocol - only the manipulation of the data stream at each end has any
knowledge of X).
> I think I'd prefer something like
> string prefix
> The prefix is the initial part of the X11 client's initial message,
> before the authentication data.
Who says that in the revision that changes this, the part of interest
will remain a prefix?
> It's usually 6 bytes, and the first byte is the byte-order mark,
> 'B' for a big-endian client, 'L' for a little endian client. (XXX
> Some X implementations seem to use 'b" and 'l' instead. I'm not
> sure if that is something we need to care about, or what a
> canonical X reference says about the case of the byte order
> characters).
The R4 protocol document says
#x42 MSB first
#x6C LSB first
Those are ASCII 'B' and 'l'; while this is not coincidence, 'b' and 'L'
are invalid X protocol, same as 'q' and '&'.
> As for the connection information, I don't quite like having the
> structure of the request to depend on the connection type. If you
> *really* want to support arbitrary connection information, I think
> I'd prefer something like
> string connection type ("tcpip" or "foo%example.com@localhost")
> string address blob (internal structure depends on the type).
I suppose I can understand that. I don't quite see what's wrong with
the way I did it, but it's certainly not a difference I'll make a fuss
over.
> But we already have at least threee types of addresses mapped into a
> string format (dns, ipv4, ipv6),
Not quite - those name (in three different ways) the machine, but not
the per-machine demultiplexing information (ports for TCP), which the
protocol shoehorns into a uint32. DECnet, for example, uses strings
for the demultiplexer names, not small integers.
> so perhaps it's simpler to use just a pair <string, number>, where
> the number part need not be used for connection types where it isn't
> relevant.
But there's no standard way - as far as I know - to combine a DECnet
node name and demultiplexing string (I don't know the canonical DECnet
terminology for them) into a single string.
> I think it is unfortunate to introduce a new type of address
> abstraction just for x11 forwarding; if the address abstraction in
> the connection document isn't general enough, it's better to fix it
> there (or in the architecture spec).
True. But fixing it everywhere is not practical for me alone, as it
would completely break interoperability. I would certainly support an
effort to fix the whole protocol (I would be inclined to do it by
converting all the <host(string),port(uint32)> pairs into
<addresstype(string),info(string)> pairs, where the address type is
something like "tcp" and the string is something like (for tcp) the
<host(string),port(uint32)> pair we use now).
We already have this issue, when dealing with AF_LOCAL forwarded X
connections: the addressing scheme in use is insufficient to the task.
>> string "cancel-x11-req%rodents.montreal.qc.ca@localhost"
> You should also say what should happen when a channel is closed.
That's true. I'm inclined to agree with you, that closing the session
implicitly cancels, but neither close nor explicit cancel breaks
existing forwadred channels.
> Finally, one alternative to using a special token to identify
> displays is to just add the id of the associated session to the "x11"
> CHANNEL_OPEN message,
Yes. I considered that, briefly. The principal reason I didn't do it
was practical: the way my implementation is layered, that's somewhat
inconvenient information to get at.
> This is equivalent to the new display id token, under the assumptions
> that
> No new x11 channels can be opened after the session channel is
> closed.
> A session can only have a single forwarded x11 display.
> Both of these make sense to me.
The former sounds more reasonable to me than the latter. I'm rather
uncomfortable wiring the assumption that a session has at most one X
display into the protocol. (I already have three programs that talk to
two X displays, and one that cna talk to an effectively unlimited
number...while at present all but at most one of those displays must be
named on the command line, it seems to me it would hamper innovation to
assume this will always be the case.)
/~\ The ASCII der Mouse
\ / Ribbon Campaign
X Against HTML mouse%rodents.montreal.qc.ca@localhost
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B