Fix the payload type returned by IPC

If a client is subscribed and sends a subsequent ipc command which
causes event updates, then those event updates override the
`client->current_command` and send the incorrect type for the payload
associated with the command.

Example:

SUBSCRIBE {window}
RUN_COMMAND focus -> PAYLOAD_TYPE is 0x80000002 for window events

Therefore, we decouple the `client->current_command` by passing it as an
argument to the ipc_send_reply function, avoiding a data race. The same
is done for the `client->payload_length` as a precautionary measure for
the same reason.

1 files changed, 72 insertions, 64 deletions

diff --git a/sway/ipc-server.c b/sway/ipc-server.c
index e133a5bf..ca1c1b12 100644
--- a/sway/ipc-server.c
+++ b/sway/ipc-server.c
@@ -47,13 +47,14 @@ struct ipc_client {
         struct wl_event_source *writable_event_source;
         struct sway_server *server;
         int fd;
-        uint32_t payload_length;
         uint32_t security_policy;
-        enum ipc_command_type current_command;
         enum ipc_command_type subscribed_events;
         size_t write_buffer_len;
         size_t write_buffer_size;
         char *write_buffer;
+        // The following are for storing data between event_loop calls
+        uint32_t pending_length;
+        enum ipc_command_type pending_type;
 };
 
 struct sockaddr_un *ipc_user_sockaddr(void);
@@ -61,8 +62,10 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data);
 int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data);
 int ipc_client_handle_writable(int client_fd, uint32_t mask, void *data);
 void ipc_client_disconnect(struct ipc_client *client);
-void ipc_client_handle_command(struct ipc_client *client);
-bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t payload_length);
+void ipc_client_handle_command(struct ipc_client *client, uint32_t payload_length,
+        enum ipc_command_type payload_type);
+bool ipc_send_reply(struct ipc_client *client, enum ipc_command_type payload_type,
+        const char *payload, uint32_t payload_length);
 
 static void handle_display_destroy(struct wl_listener *listener, void *data) {
         if (ipc_event_source) {
@@ -178,7 +181,7 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data) {
                 return 0;
         }
         client->server = server;
-        client->payload_length = 0;
+        client->pending_length = 0;
         client->fd = client_fd;
         client->subscribed_events = 0;
         client->event_source = wl_event_loop_add_fd(server->wl_event_loop,
@@ -224,9 +227,13 @@ int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data) {
         }
 
         // Wait for the rest of the command payload in case the header has already been read
-        if (client->payload_length > 0) {
-                if ((uint32_t)read_available >= client->payload_length) {
-                        ipc_client_handle_command(client);
+        if (client->pending_length > 0) {
+                if ((uint32_t)read_available >= client->pending_length) {
+                        // Reset pending values.
+                        uint32_t pending_length = client->pending_length;
+                        enum ipc_command_type pending_type = client->pending_type;
+                        client->pending_length = 0;
+                        ipc_client_handle_command(client, pending_length, pending_type);
                 }
                 return 0;
         }
@@ -251,11 +258,15 @@ int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data) {
                 return 0;
         }
 
-        memcpy(&client->payload_length, &buf32[0], sizeof(buf32[0]));
-        memcpy(&client->current_command, &buf32[1], sizeof(buf32[1]));
+        memcpy(&client->pending_length, &buf32[0], sizeof(buf32[0]));
+        memcpy(&client->pending_type, &buf32[1], sizeof(buf32[1]));
 
-        if (read_available - received >= (long)client->payload_length) {
-                ipc_client_handle_command(client);
+        if (read_available - received >= (long)client->pending_length) {
+                // Reset pending values.
+                uint32_t pending_length = client->pending_length;
+                enum ipc_command_type pending_type = client->pending_type;
+                client->pending_length = 0;
+                ipc_client_handle_command(client, pending_length, pending_type);
         }
 
         return 0;
@@ -278,8 +289,8 @@ static void ipc_send_event(const char *json_string, enum ipc_command_type event)
                 if ((client->subscribed_events & event_mask(event)) == 0) {
                         continue;
                 }
-                client->current_command = event;
-                if (!ipc_send_reply(client, json_string, (uint32_t) strlen(json_string))) {
+                if (!ipc_send_reply(client, event, json_string,
+                                (uint32_t)strlen(json_string))) {
                         sway_log_errno(SWAY_INFO, "Unable to send reply to IPC client");
                         /* ipc_send_reply destroys client on error, which also
                          * removes it from the list, so we need to process
@@ -567,20 +578,21 @@ static void ipc_get_marks_callback(struct sway_container *con, void *data) {
         }
 }
 
-void ipc_client_handle_command(struct ipc_client *client) {
+void ipc_client_handle_command(struct ipc_client *client, uint32_t payload_length,
+                enum ipc_command_type payload_type) {
         if (!sway_assert(client != NULL, "client != NULL")) {
                 return;
         }
 
-        char *buf = malloc(client->payload_length + 1);
+        char *buf = malloc(payload_length + 1);
         if (!buf) {
                 sway_log_errno(SWAY_INFO, "Unable to allocate IPC payload");
                 ipc_client_disconnect(client);
                 return;
         }
-        if (client->payload_length > 0) {
+        if (payload_length > 0) {
                 // Payload should be fully available
-                ssize_t received = recv(client->fd, buf, client->payload_length, 0);
+                ssize_t received = recv(client->fd, buf, payload_length, 0);
                 if (received == -1)
                 {
                         sway_log_errno(SWAY_INFO, "Unable to receive payload from IPC client");
@@ -589,16 +601,15 @@ void ipc_client_handle_command(struct ipc_client *client) {
                         return;
                 }
         }
-        buf[client->payload_length] = '\0';
+        buf[payload_length] = '\0';
 
-        bool client_valid = true;
-        switch (client->current_command) {
+        switch (payload_type) {
         case IPC_COMMAND:
         {
                 char *line = strtok(buf, "\n");
                 while (line) {
                         size_t line_length = strlen(line);
-                        if (line + line_length >= buf + client->payload_length) {
+                        if (line + line_length >= buf + payload_length) {
                                 break;
                         }
                         line[line_length] = ';';
@@ -609,7 +620,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
                 transaction_commit_dirty();
                 char *json = cmd_results_to_json(res_list);
                 int length = strlen(json);
-                client_valid = ipc_send_reply(client, json, (uint32_t)length);
+                ipc_send_reply(client, payload_type, json, (uint32_t)length);
                 free(json);
                 while (res_list->length) {
                         struct cmd_results *results = res_list->items[0];
@@ -623,7 +634,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
         case IPC_SEND_TICK:
         {
                 ipc_event_tick(buf);
-                ipc_send_reply(client, "{\"success\": true}", 17);
+                ipc_send_reply(client, payload_type, "{\"success\": true}", 17);
                 goto exit_cleanup;
         }
 
@@ -656,8 +667,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                         }
                 }
                 const char *json_string = json_object_to_json_string(outputs);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(outputs); // free
                 goto exit_cleanup;
         }
@@ -667,8 +678,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                 json_object *workspaces = json_object_new_array();
                 root_for_each_workspace(ipc_get_workspaces_callback, workspaces);
                 const char *json_string = json_object_to_json_string(workspaces);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(workspaces); // free
                 goto exit_cleanup;
         }
@@ -679,7 +690,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
                 struct json_object *request = json_tokener_parse(buf);
                 if (request == NULL || !json_object_is_type(request, json_type_array)) {
                         const char msg[] = "{\"success\": false}";
-                        client_valid = ipc_send_reply(client, msg, strlen(msg));
+                        ipc_send_reply(client, payload_type, msg, strlen(msg));
                         sway_log(SWAY_INFO, "Failed to parse subscribe request");
                         goto exit_cleanup;
                 }
@@ -707,7 +718,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
                                 is_tick = true;
                         } else {
                                 const char msg[] = "{\"success\": false}";
-                                client_valid = ipc_send_reply(client, msg, strlen(msg));
+                                ipc_send_reply(client, payload_type, msg, strlen(msg));
                                 json_object_put(request);
                                 sway_log(SWAY_INFO, "Unsupported event type in subscribe request");
                                 goto exit_cleanup;
@@ -716,11 +727,11 @@ void ipc_client_handle_command(struct ipc_client *client) {
 
                 json_object_put(request);
                 const char msg[] = "{\"success\": true}";
-                client_valid = ipc_send_reply(client, msg, strlen(msg));
+                ipc_send_reply(client, payload_type, msg, strlen(msg));
                 if (is_tick) {
-                        client->current_command = IPC_EVENT_TICK;
                         const char tickmsg[] = "{\"first\": true, \"payload\": \"\"}";
-                        ipc_send_reply(client, tickmsg, strlen(tickmsg));
+                        ipc_send_reply(client, IPC_EVENT_TICK, tickmsg,
+                                strlen(tickmsg));
                 }
                 goto exit_cleanup;
         }
@@ -733,8 +744,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                         json_object_array_add(inputs, ipc_json_describe_input(device));
                 }
                 const char *json_string = json_object_to_json_string(inputs);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(inputs); // free
                 goto exit_cleanup;
         }
@@ -747,8 +758,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                         json_object_array_add(seats, ipc_json_describe_seat(seat));
                 }
                 const char *json_string = json_object_to_json_string(seats);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(seats); // free
                 goto exit_cleanup;
         }
@@ -757,8 +768,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
         {
                 json_object *tree = ipc_json_describe_node_recursive(&root->node);
                 const char *json_string = json_object_to_json_string(tree);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t) strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(tree);
                 goto exit_cleanup;
         }
@@ -768,8 +779,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                 json_object *marks = json_object_new_array();
                 root_for_each_container(ipc_get_marks_callback, marks);
                 const char *json_string = json_object_to_json_string(marks);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(marks);
                 goto exit_cleanup;
         }
@@ -778,8 +789,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
         {
                 json_object *version = ipc_json_get_version();
                 const char *json_string = json_object_to_json_string(version);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(version); // free
                 goto exit_cleanup;
         }
@@ -794,9 +805,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                                 json_object_array_add(bars, json_object_new_string(bar->id));
                         }
                         const char *json_string = json_object_to_json_string(bars);
-                        client_valid =
-                                ipc_send_reply(client, json_string,
-                                        (uint32_t)strlen(json_string));
+                        ipc_send_reply(client, payload_type, json_string,
+                                (uint32_t)strlen(json_string));
                         json_object_put(bars); // free
                 } else {
                         // Send particular bar's details
@@ -810,15 +820,14 @@ void ipc_client_handle_command(struct ipc_client *client) {
                         }
                         if (!bar) {
                                 const char *error = "{ \"success\": false, \"error\": \"No bar with that ID\" }";
-                                client_valid =
-                                        ipc_send_reply(client, error, (uint32_t)strlen(error));
+                                ipc_send_reply(client, payload_type, error,
+                                        (uint32_t)strlen(error));
                                 goto exit_cleanup;
                         }
                         json_object *json = ipc_json_describe_bar_config(bar);
                         const char *json_string = json_object_to_json_string(json);
-                        client_valid =
-                                ipc_send_reply(client, json_string,
-                                        (uint32_t)strlen(json_string));
+                        ipc_send_reply(client, payload_type, json_string,
+                                (uint32_t)strlen(json_string));
                         json_object_put(json); // free
                 }
                 goto exit_cleanup;
@@ -832,8 +841,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
                         json_object_array_add(modes, json_object_new_string(mode->name));
                 }
                 const char *json_string = json_object_to_json_string(modes);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(modes); // free
                 goto exit_cleanup;
         }
@@ -843,34 +852,32 @@ void ipc_client_handle_command(struct ipc_client *client) {
                 json_object *json = json_object_new_object();
                 json_object_object_add(json, "config", json_object_new_string(config->current_config));
                 const char *json_string = json_object_to_json_string(json);
-                client_valid =
-                        ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
+                ipc_send_reply(client, payload_type, json_string,
+                        (uint32_t)strlen(json_string));
                 json_object_put(json); // free
                 goto exit_cleanup;
-    }
+        }
 
         case IPC_SYNC:
         {
                 // It was decided sway will not support this, just return success:false
                 const char msg[] = "{\"success\": false}";
-                ipc_send_reply(client, msg, strlen(msg));
+                ipc_send_reply(client, payload_type, msg, strlen(msg));
                 goto exit_cleanup;
         }
 
         default:
-                sway_log(SWAY_INFO, "Unknown IPC command type %i", client->current_command);
+                sway_log(SWAY_INFO, "Unknown IPC command type %x", payload_type);
                 goto exit_cleanup;
         }
 
 exit_cleanup:
-        if (client_valid) {
-                client->payload_length = 0;
-        }
         free(buf);
         return;
 }
 
-bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t payload_length) {
+bool ipc_send_reply(struct ipc_client *client, enum ipc_command_type payload_type,
+                const char *payload, uint32_t payload_length) {
         assert(payload);
 
         char data[IPC_HEADER_SIZE];
@@ -878,7 +885,7 @@ bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t pay
 
         memcpy(data, ipc_magic, sizeof(ipc_magic));
         memcpy(&data32[0], &payload_length, sizeof(payload_length));
-        memcpy(&data32[1], &client->current_command, sizeof(client->current_command));
+        memcpy(&data32[1], &payload_type, sizeof(payload_type));
 
         while (client->write_buffer_len + IPC_HEADER_SIZE + payload_length >=
                                  client->write_buffer_size) {
@@ -910,6 +917,7 @@ bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t pay
                                 ipc_client_handle_writable, client);
         }
 
-        sway_log(SWAY_DEBUG, "Added IPC reply to client %d queue: %s", client->fd, payload);
+        sway_log(SWAY_DEBUG, "Added IPC reply of type 0x%x to client %d queue: %s",
+                payload_type, client->fd, payload);
         return true;
 }