On Tue, Oct 10, 2017 at 2:17 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Hi, > > Brandon Williams wrote: > >> Given some of this discussion though, maybe we want to change the >> semantics of 'protocol.version' so that both servers and clients respect >> it. I have no preference for this, but I agree with Jonathans reasoning. >> As of right now, these patches have the server always allow >> protocol v0 and v1? Though that doesn't do much right now since v1 is >> the same as v0. I would strongly prefer if the user configures "v0 and v1", being explicit, if the client wants to talk either of them. (v0 is not any more special than any future protocol after all -- from the users perspective, configuring protocol.version) On the wire transfer we may want to omit the v0, but for simplicity we could just dump the clients config of "v0 and v1" onto the wire, and the server would either ignore the "v0 and" part (when speaking v1), or ignore it completely (old server, speaking v0). Given this model, we have * a strict whitelist clientside, * the ordering decision can be deferred until later, when we have an actual v2. >> One other considerations that I should probably handle is that a client >> doesn't do any verification right now to ensure that the protocol >> version the server selects was indeed the protocol that the client >> requested. Is that something you think we need to check for? Yes, we would want to see if the protocol version matches the configured white list. ("Once we have a v2, I would no longer want a server talking v0,v1 to me, because I consider v0 insecure[1], and I personally want to rather have no communication than a v0 protocol. After all I configured 'protocol.version = v2' only") [1] e.g. https://public-inbox.org/git/1477690790.2904.22.camel@xxxxxxxxxxxxxxxxx/ > Do you mean in tests, or are you referring to something else? A test would be lovely. Thanks, Stefan