ADC 0.12 open issues

[FarCry]

Most of the issues and open questions regarding the current ADC protocol draft (V0.12) have already been discussed on the developer's hub, but I'd like to repeat them here, where they are less likely to be forgotten and perhaps reach others to comment on them:

2.4.1 Session ID
"The hub may reuse SIDs as they become free when clients disconnect." - This sentence can probably be removed, since it is already sufficiently clear from "They identify a unique user on a single hub". I'm mentioning it, because it is an implementation hint which can be misleading without further explanation. Too greedy reuse of SIDs by the hub can lead to problems in the form of race conditions, where a command sent by one client reaches the wrong target because the intended target was disconnected and the SID reclaimed for a newly connected client. Suggesting precautions against such problems is beyond the scope of the protocol specs, and so I'd prefer the implementation hint be removed completely.

2.4.2 Private ID
The suggested way of generating PIDs is too complicated compared to its value for the security of the protocol. DC++ does not use it and other implementations will probably also revert to alternative methods of generating a random PID. The specs should not provoke such violations by being overly complicated. It would however be nice to be able to specify some minimum level of randomness of a PID. Perhaps some math expert could help us out here.

4 BASE messages
The message constraints have been changed in one of the last revisions from direct specification of allowed types for each command to a loose context grouping. This may have made editing the draft easier when types were added or changed, but it's not clear enough as a guideline for implementations anymore. I don't think it should be left up to a hub implementation wether it forwards commands like BRCM or BCTM, as that opens the doors for abuse and puts client implementers in a difficult position.

4.3.1 STA
"43 Required INF field missing/bad, flag “FL” specifies the name of field." should probably be split into "field missing" and "field bad" and made independent of the "INF" context, so we have two independent generic errors for wrong or missing parameters which a client could in some cases handle automatically.

4.3.5 INF
"For OP, AW, BO, HI and HU, unspecified values are reserved for the future." - This should either be refined or removed, as it leaves the question open how those reserved values should be treated and thus renders the specified values useless. If an extended client sends "OP2" in the future, should a BASE client read that as "OP" (empty or 0) or "OP1"? The reserved values fail to deliver the information of the original values.
It would also perhaps be interesting to put the information carried by OP, RG, BO and HI into one single field (like CT for Client Type).

4.3.5 MSG
The purpose of the PM field and <group-SID> parameter is not sufficiently explained. The description also seems to suggest that a chat-room bot should use the SID of the clients from which the message originated as the my_sid of the underlying message layout and put its own SID into the PM field for forwarding. I think that is putting the (perceived) layering of the protocol on its head, as the basic (type-related) parameters should be verifyable without looking at the transported command type. The my_sid should always be the SID of the client that sends the (ADC) message.

4.3.8 CTM and 4.3.9 CTM
It's not consistent with the rest of the protocol that a named parameter should be in the list of mandatory parameters. The token for these commands should either be an unnamed mandatory parameter or a named optional parameter.

4.3.13 DSC
It has been discussed to remove this command and if I remember correctly, arnetheduck agreed to do so. I'm mentioning it just to make sure

6.3.1 ZLIB-FULL
ZOF is not needed.

There are certainly some things I have forgotten. I remember at least one problem in the Message syntax section, but I couldn't find it in a quick review. I hope others will complete this list. I didn't use the Wiki in hope for a wider audience and more input (I personally never check the Wiki unless someone points me to it). Once the currently known issues are resolved, I think arnetheduck should post a "release candidate" revision and perhaps post a last "call for suggestions" here with a proper deadline, so that ADC gets a V1.0 protocol specification soon.

[ullner]

I'd like to remove the following, based on that I don't think the protocol should explicitly state what clients or hubs should or shouldn't do (or be able to). The line between a mere suggestion and a demand is thin, which makes me hesitant to (not) include certain parts of the current draft.

Applications should be prepared to handle at least 64-bit signed integers and 64-bit floating point numbers.

Hubs should not discriminate agains clients based on their VE tag but instead rely on SUP when it comes to which clients should be allowed (for example, “we only want regex clients”).

When sent for hub, this is the nick that should be displayed before messages from the hub

When sent by hub, this string should be displayed in the window title of the hub window (if one exists).

[FarCry]

Unless I have missed the obvious, there is also a problem with client-to-client download connections, especially from the perspective of passive mode clients.

With an active mode client (AC) as source and a passive mode client (PC) as downloader, the connection request always looks like this over the hub connection:

Code: Select all

    PC -> AC DRCM...
    AC -> PC DCTM...

After that, the passive mode client connects to the specified port of the active mode client, which always puts him in the role of the client in the client-to-client handshake:

Code: Select all

    PC -> AC CSUP...
    AC -> PC CSUP... CINF...
    PC -> AC CINF...

That's where the protocol specification ends. The example at the bottom of the draft suggests that it's now the server's (active mode client's) turn to respond. This doesn't work well for the PC, as there is no clean way for the AC to pass control to the PC. Having the order undefined and just letting both sides send a CGET concurrently is not a good solution. A CSTA could be used to pass control, but there still could be a lot of unnecessary roundtrips for each attempt, as the AC will always start with CGETs from his side, if it also wants files from the PC.

Alternatively, the connection token could be used to determine which client is supposed to speak first after the handshake - whoever issued the token requested the connection and should thus be allowed to send the CGET first. In this case, the token cannot be optional in RCM and CTM, of course.

[arnetheduck]

2.4.1 - changed (also added that 0 is always the hub itself).
2.4.2 - changed to no longer mandate it.
4 - I believe it should be up to the implementation - I'd also suggest a wiki page with annotations to the spec where implementation suggestions are added.
4.3.1 - split into "FM" and "FB".
4.3.5 - Removed. Merged some of them.
4.3.8 - Removed "TO".
4.3.13 - Removed.
6.3.1 - Removed.

Passive clients:

It is always the client that sends the first CTM/RCM command that is given control of the connection once the NORMAL state has been reached.

I still need to think about the PM's, but one can argue that the sender of a message is the client of the user that actually wrote the message, while the PM field contains just a reply-to address, much like a mailing list. This of course makes sense if groups are implemented on the hub side, not by a bot or something like that...

[ivulfusbar]

Alternatively, the connection token could be used to determine which client is supposed to speak first after the handshake - whoever issued the token requested the connection and should thus be allowed to send the CGET first. In this case, the token cannot be optional in RCM and CTM, of course.

This is what i have always thought would be the best solution. To use the TO-token to remove the direction-battle.

As a server in c-c connections, I think its in the servers interest to get a token since its the only information used to identify the party in a secure manner.

[Jove]

I still need to think about the PM's, but one can argue that the sender of a message is the client of the user that actually wrote the message, while the PM field contains just a reply-to address, much like a mailing list. This of course makes sense if groups are implemented on the hub side, not by a bot or something like that...

An interesting question: Should the group support (ie, the PM parameter) be part of base? If not, then this supports FarCry's suggestion and the PM parameter can be moved to an extension.

Also allowing groups to be implemented by bots is sensible too.It adds to the flexibility of the protocol.
Personally i would love to see those annoying trivia bots running in their own seperate room...

As a server in c-c connections, I think its in the servers interest to get a token since its the only information used to identify the party in a secure manner.

I agree.

[FarCry]

2.4.1 - changed (also added that 0 is always the hub itself).

We've been there before, haven't we? What does the hub need an SID for?

[FarCry]

The distinction between U4 and U6 for INF fields seems to be redundant, because using both is impractical and the (verifiable) protocol version is already given by I4/I6. The port field could simply be UP for "UDP port".