From c832ce3ddcb0902c6753f8efd987379a4b72c115 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 28 Feb 2023 09:38:36 +1000 Subject: [PATCH] protocol: split the handshake version negotiation from the other interfaces Previously, we'd send one interface_version event for "ei_handshake" immediately but all others after the client requests handshake.finish. This was too confusing to document and not clear how it would work, so let's make this simpler by splitting it up. There is now a handshake_version event from the server, sent immediately on connection that denotes the maximum version number for the interface. And a handshake_version request from the client which must be the first one by the client. --- proto/protocol.xml | 67 ++++++++++++++++++++++++++---------------- src/libei-handshake.c | 33 ++++++++++++++------- src/libeis-handshake.c | 47 +++++++++++++++++++++++++++-- test/test_protocol.py | 8 +++-- 4 files changed, 114 insertions(+), 41 deletions(-) diff --git a/proto/protocol.xml b/proto/protocol.xml index 58bf630..e3c21f8 100644 --- a/proto/protocol.xml +++ b/proto/protocol.xml @@ -101,11 +101,13 @@ In summary, a typical client connection does: - connect to the socket - - read ei_handshake.version for object id 0 - - send ei_handshake.interface for "ei_connection" - - send ei_handshake.interface for all other supported interfaces + - read ei_handshake.handshake_version for object id 0 + - send ei_handshake.handshake_version for object id 0 + - send ei_handshake.interface_version for all other supported interfaces - optionally: send ei_handshake.name and ei_handshake.context_type - send ei_handshake.finish + - send ei_handshake.finish + - optionally: receive ei_handshake.interface_version for named interfaces - receive the ei_connection.connection event and create that object - receive ei_connection.seat (if any seats are available) - .... @@ -149,6 +151,24 @@ + + + Notifies the EIS implementation that this client supports the + given version of the ei_handshake interface. The version number + must be less than or equal to the version in the + handshake_version event sent by the EIS implementation when + the connection was established. + + Immediately after sending this request, the client must assume the negotiated + version number for the ei_handshake interface and the EIS implementation + may send events and process requests matching that version. + + This request must be sent exactly once and it must be the first request + the client sends. + + + + Notify the EIS implementation that configuration is complete. @@ -218,12 +238,8 @@ failing to do so will result in the EIS implementation disconnecting the client on ei_handshake.finish. - If the "ei_handshake" version is given, the interface of this - object is upgraded to the given version. Otherwise, the - ei_handshake version defaults to 1. - - A client must not provide a "ei_handshake" version higher - than the EIS implementation sent immediately after connection. + This request must not be sent for the "ei_handshake" interface, use + the handshake_version request instead. Note that an EIS implementation may consider some interfaces to be required and immediately ei_connection.disconnect a client @@ -237,33 +253,32 @@ + + + This event is sent exactly once and immediately after connection + to the EIS implementation. + + In response, the client must send the handshake_version request + with any version up to including the version provided in this event. + See the handshake_version request for details on what happens next. + + + Notifies the client that the EIS implementation supports the given named interface with the given maximum version number. - This event is sent immediately after connection to the EIS implementation - for the "ei_handshake" interface. In response, the client may - send the interface_version request for the "ei_handshake" interface - with any version up to including the version provided in that event. - Once the request has been sent, the client must assume the negotiated - version number for the ei_handshake interface and the server - may send events and process requests matching that version. - - A client should not issue any requests until negotiating the version for - the "ei_handshake" interface. - - Note that the EIS implementation assumes that the supported - client version of the "ei_handshake" interface is 1 unless and - until the client announces a higher version of this interface in the - ei_handshake.interface_version request. - This event must be sent by the EIS implementation for any - interfaces that supports client-created objects (e.g. "ei_callback"). + interfaces that supports client-created objects (e.g. "ei_callback") + before the ei_handshake.connection event. The client must not assume those interfaces are supported unless and until those versions have been received. + This request must not be sent for the "ei_handshake" interface, use + the handshake_version event instead. + This event may be sent by the EIS implementation for any other supported interface (but not necessarily all supported interfaces) before the ei_handshake.connection event. diff --git a/src/libei-handshake.c b/src/libei-handshake.c index 46b6bff..833d7d1 100644 --- a/src/libei-handshake.c +++ b/src/libei-handshake.c @@ -72,6 +72,9 @@ static int ei_handshake_initialize(struct ei_handshake *setup, uint32_t version) { struct ei *ei = ei_handshake_get_context(setup); + struct ei_interface_versions *v = &ei->interface_versions; + + ei_handshake_request_handshake_version(setup, v->ei_handshake); if (version >= EI_HANDSHAKE_REQUEST_CONTEXT_TYPE_SINCE_VERSION) ei_handshake_request_context_type(setup, @@ -83,8 +86,6 @@ ei_handshake_initialize(struct ei_handshake *setup, uint32_t version) ei_handshake_request_name(setup, ei->name); if (version >= EI_HANDSHAKE_REQUEST_INTERFACE_VERSION_SINCE_VERSION) { - struct ei_interface_versions *v = &ei->interface_versions; - ei_handshake_request_interface_version(setup, "ei_handshake", v->ei_handshake); ei_handshake_request_interface_version(setup, "ei_connection", v->ei_connection); ei_handshake_request_interface_version(setup, "ei_callback", v->ei_callback); ei_handshake_request_interface_version(setup, "ei_pingpong", v->ei_pingpong); @@ -100,6 +101,24 @@ ei_handshake_initialize(struct ei_handshake *setup, uint32_t version) return 0; } +static struct brei_result * +handle_msg_handshake_version(struct ei_handshake *setup, uint32_t version) +{ + struct ei *ei = ei_handshake_get_context(setup); + struct ei_interface_versions *v = &ei->interface_versions; + + uint32_t min_version = min(version, ei->interface_versions.ei_handshake); + v->ei_handshake = min_version; + + /* Now upgrade our protocol object to the server version (if applicable) */ + setup->proto_object.version = min_version; + + /* Now send all the bits we need to send */ + ei_handshake_initialize(setup, min_version); + + return NULL; +} + static struct brei_result * handle_msg_interface_version(struct ei_handshake *setup, const char *name, uint32_t version) { @@ -107,14 +126,7 @@ handle_msg_interface_version(struct ei_handshake *setup, const char *name, uint3 struct ei_interface_versions *v = &ei->interface_versions; if (streq(name, "ei_handshake")) { - uint32_t min_version = min(version, ei->interface_versions.ei_handshake); - v->ei_handshake = min_version; - - /* Now upgrade our protocol object to the server version (if applicable) */ - setup->proto_object.version = min_version; - - /* Now send all the bits we need to send */ - ei_handshake_initialize(setup, min_version); + /* EIS shouldn't send this anyway, let's ignore this */ } #define VERSION_UPDATE(iface_) if (streq(name, #iface_)) v->iface_ = min(version, v->iface_); else VERSION_UPDATE(ei_connection) @@ -164,6 +176,7 @@ handle_msg_connection(struct ei_handshake *setup, uint32_t serial, uint32_t id, } static const struct ei_handshake_interface interface = { + .handshake_version = handle_msg_handshake_version, .interface_version = handle_msg_interface_version, .connection = handle_msg_connection, }; diff --git a/src/libeis-handshake.c b/src/libeis-handshake.c index 4021c2f..d6ee91a 100644 --- a/src/libeis-handshake.c +++ b/src/libeis-handshake.c @@ -82,13 +82,37 @@ pong(struct eis_connection *connection, void *user_data) eis_queue_connect_event(client); } +static struct brei_result* +client_msg_handshake_version(struct eis_handshake *setup, uint32_t version) +{ + struct eis_client *client = eis_handshake_get_client(setup); + struct eis *eis = eis_client_get_context(client); + + log_debug(eis, "client %#x supports handshake version %u", client->id, version); + + if (version == 0) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_VALUE, "Invalid handshake version %u", version); + + if (setup->client_versions.ei_handshake != 0) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "Duplicate handshake version"); + if (version > setup->server_versions.ei_handshake) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "Invalid handshake version %ud", version); + + setup->client_versions.ei_handshake = min(setup->server_versions.ei_handshake, version); + + return 0; +} + static struct brei_result * client_msg_finish(struct eis_handshake *setup) { struct eis_client *client = eis_handshake_get_client(setup); /* Required interfaces - immediate disconnection if missing */ - if (setup->client_versions.ei_connection == 0 || + if (setup->client_versions.ei_handshake == 0 || + setup->client_versions.ei_connection == 0 || setup->client_versions.ei_callback == 0 || setup->client_versions.ei_pingpong == 0) return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, @@ -126,6 +150,10 @@ client_msg_finish(struct eis_handshake *setup) static struct brei_result * client_msg_name(struct eis_handshake *setup, const char *name) { + if (setup->client_versions.ei_handshake == 0) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "Missing handshake versions"); + if (setup->name) return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, "Duplicate client name"); @@ -137,6 +165,10 @@ client_msg_name(struct eis_handshake *setup, const char *name) static struct brei_result * client_msg_context_type(struct eis_handshake *setup, uint32_t type) { + if (setup->client_versions.ei_handshake == 0) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "Missing handshake versions"); + switch(type) { case EIS_HANDSHAKE_CONTEXT_TYPE_SENDER: setup->is_sender = true; @@ -152,6 +184,14 @@ client_msg_context_type(struct eis_handshake *setup, uint32_t type) static struct brei_result * client_msg_interface_version(struct eis_handshake *setup, const char *name, uint32_t version) { + if (setup->client_versions.ei_handshake == 0) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "Missing handshake versions"); + + if (streq(name, "ei_handshake")) + return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "ei_handshake may not be used in interface_version"); + struct eis_client *client = eis_handshake_get_client(setup); struct eis *eis = eis_client_get_context(client); struct v { @@ -167,7 +207,7 @@ client_msg_interface_version(struct eis_handshake *setup, const char *name, uint VERSION_ENTRY(ei_callback), VERSION_ENTRY(ei_pingpong), VERSION_ENTRY(ei_connection), - VERSION_ENTRY(ei_handshake), + /* ei_handshake is not handled here */ VERSION_ENTRY(ei_seat), VERSION_ENTRY(ei_device), VERSION_ENTRY(ei_pointer), @@ -199,6 +239,7 @@ client_msg_interface_version(struct eis_handshake *setup, const char *name, uint } static const struct eis_handshake_interface interface = { + .handshake_version = client_msg_handshake_version, .finish = client_msg_finish, .context_type = client_msg_context_type, .name = client_msg_name, @@ -227,7 +268,7 @@ eis_handshake_new(struct eis_client *client, setup->server_versions = *versions; eis_client_register_object(client, &setup->proto_object); - eis_handshake_event_interface_version(setup, "ei_handshake", versions->ei_handshake); + eis_handshake_event_handshake_version(setup, versions->ei_handshake); return setup; /* ref owned by caller */ } diff --git a/test/test_protocol.py b/test/test_protocol.py index 6c39860..15e7979 100644 --- a/test/test_protocol.py +++ b/test/test_protocol.py @@ -158,6 +158,7 @@ class Ei: def init_default_sender_connection(self) -> None: setup = self.handshake + self.send(setup.HandshakeVersion(VERSION_V(1))) self.send(setup.ContextType(EiHandshake.EiContextType.SENDER)) self.send(setup.Name("test client")) self.send( @@ -359,8 +360,7 @@ class TestEiProtocol: ei.wait_for(lambda: bool(setup.calllog)) call = setup.calllog[0] - assert call.name == "InterfaceVersion" - assert call.args["name"] == "ei_handshake" + assert call.name == "HandshakeVersion" assert call.args["version"] == VERSION_V(1) eis.terminate() @@ -406,6 +406,7 @@ class TestEiProtocol: except ValueError: pass + ei.send(ei.handshake.HandshakeVersion(VERSION_V(1))) ei.send(ei.handshake.ContextType(invalid_type)) try: @@ -503,6 +504,7 @@ class TestEiProtocol: setup = ei.handshake # Establish our connection + ei.send(setup.HandshakeVersion(VERSION_V(1))) ei.send(setup.ContextType(EiHandshake.EiContextType.SENDER)) ei.send(setup.Name("test client")) for interface in ["ei_connection", "ei_callback", "ei_pingpong"]: @@ -563,6 +565,7 @@ class TestEiProtocol: setup = ei.handshake # Establish our connection + ei.send(setup.HandshakeVersion(VERSION_V(1))) ei.send(setup.ContextType(EiHandshake.EiContextType.SENDER)) ei.send(setup.Name("test client")) for interface in ["ei_connection", "ei_callback", "ei_pingpong"]: @@ -735,6 +738,7 @@ class TestEiProtocol: ei.dispatch() setup = ei.handshake + ei.send(setup.HandshakeVersion(VERSION_V(1))) ei.send(setup.ContextType(EiHandshake.EiContextType.SENDER)) ei.send(setup.Name("test client"))