fix(ipc): preserve legacy preedit and bound client waits

gtk/main.c

@@ -81,7 +81,7 @@ readresp(Im *im, int want, int update, char *commit, int ncommit,
 	   resp) < 0)
 		return -1;
 	if(update)
-		setpreedit(im, pre, resp->npreedit);
+		setpreedit(im, pre, resp->preeditlen);
 	return 0;
 }
 

ipc.c

@@ -1,12 +1,102 @@
+#define _POSIX_C_SOURCE 200809L
+
 #include <errno.h>
+#include <fcntl.h>
+#include <poll.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 #include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include "ipc.h"
 
+static int64_t
+nowms(void)
+{
+	struct timespec ts;
+
+	if(clock_gettime(CLOCK_MONOTONIC, &ts) < 0)
+		return -1;
+	return (int64_t)ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
+}
+
+static int64_t
+deadline(void)
+{
+	int64_t now;
+
+	now = nowms();
+	if(now < 0)
+		return -1;
+	return now + Ipcwaitms;
+}
+
+static int
+waitfd(int fd, short events, int64_t until)
+{
+	struct pollfd pfd;
+	int64_t now, left;
+	int n;
+
+	pfd.fd = fd;
+	pfd.events = events;
+	for(;;){
+		now = nowms();
+		if(now < 0)
+			return -1;
+		left = until - now;
+		if(left <= 0){
+			errno = ETIMEDOUT;
+			return -1;
+		}
+		pfd.revents = 0;
+		n = poll(&pfd, 1, left > INT32_MAX ? INT32_MAX : (int)left);
+		if(n < 0 && errno == EINTR)
+			continue;
+		if(n < 0)
+			return -1;
+		if(n == 0){
+			errno = ETIMEDOUT;
+			return -1;
+		}
+		if(pfd.revents & POLLNVAL){
+			errno = EBADF;
+			return -1;
+		}
+		if(pfd.revents & (events|POLLERR|POLLHUP))
+			return 0;
+	}
+}
+
+static int
+readwait(int fd, void *buf, size_t n, int64_t until)
+{
+	unsigned char *p;
+	ssize_t r;
+
+	p = buf;
+	while(n > 0){
+		if(waitfd(fd, POLLIN, until) < 0)
+			return -1;
+		r = recv(fd, p, n, MSG_DONTWAIT);
+		if(r < 0 && errno == EINTR)
+			continue;
+		if(r < 0 && (errno == EAGAIN || errno == EWOULDBLOCK))
+			continue;
+		if(r < 0)
+			return -1;
+		if(r == 0){
+			errno = ECONNRESET;
+			return -1;
+		}
+		p += r;
+		n -= r;
+	}
+	return 0;
+}
+
 static void
 putlen(unsigned char p[Ipclensz], size_t n)
 {
@@ -70,7 +160,9 @@ int
 ipcconnect(void)
 {
 	struct sockaddr_un addr;
-	int e, fd;
+	socklen_t nerr;
+	int e, err, fd, fdflags, flags;
+	int64_t until;
 
 	memset(&addr, 0, sizeof addr);
 	addr.sun_family = AF_UNIX;
@@ -81,8 +173,33 @@ ipcconnect(void)
 	fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if(fd < 0)
 		return -1;
-	if(connect(fd, (struct sockaddr*)&addr, sizeof addr) == 0)
+	fdflags = fcntl(fd, F_GETFD);
+	if(fdflags < 0 || fcntl(fd, F_SETFD, fdflags|FD_CLOEXEC) < 0)
+		goto Bad;
+	flags = fcntl(fd, F_GETFL);
+	if(flags < 0 || fcntl(fd, F_SETFL, flags|O_NONBLOCK) < 0)
+		goto Bad;
+	until = deadline();
+	if(until < 0)
+		goto Bad;
+	if(connect(fd, (struct sockaddr*)&addr, sizeof addr) < 0){
+		if(errno != EINPROGRESS && errno != EALREADY && errno != EINTR &&
+		   errno != EAGAIN && errno != EWOULDBLOCK)
+			goto Bad;
+		if(waitfd(fd, POLLOUT, until) < 0)
+			goto Bad;
+		nerr = sizeof err;
+		if(getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &nerr) < 0)
+			goto Bad;
+		if(err != 0){
+			errno = err;
+			goto Bad;
+		}
+	}
+	if(fcntl(fd, F_SETFL, flags) < 0)
+		goto Bad;
 	return fd;
+Bad:
 	e = errno;
 	close(fd);
 	errno = e;
@@ -255,14 +372,28 @@ ipcsend(int fd, const void *buf, size_t n)
 {
 	const unsigned char *p;
 	ssize_t r;
+	int64_t until;
 
 	p = buf;
+	if(n == 0)
+		return 0;
+	until = deadline();
+	if(until < 0)
+		return -1;
 	while(n > 0){
-		r = send(fd, p, n, MSG_NOSIGNAL);
+		if(waitfd(fd, POLLOUT, until) < 0)
+			return -1;
+		r = send(fd, p, n, MSG_NOSIGNAL|MSG_DONTWAIT);
 		if(r < 0 && errno == EINTR)
 			continue;
-		if(r <= 0)
+		if(r < 0 && (errno == EAGAIN || errno == EWOULDBLOCK))
+			continue;
+		if(r < 0)
 			return -1;
+		if(r == 0){
+			errno = EPIPE;
+			return -1;
+		}
 		p += r;
 		n -= r;
 	}
@@ -270,7 +401,8 @@ ipcsend(int fd, const void *buf, size_t n)
 }
 
 static int
-readfield(int fd, size_t n, char *dst, size_t cap)
+readfield(int fd, size_t n, char *dst, size_t cap, size_t *copied,
+	int64_t until)
 {
 	unsigned char discard[128];
 	size_t keep, part;
@@ -280,17 +412,18 @@ readfield(int fd, size_t n, char *dst, size_t cap)
 	keep = 0;
 	if(cap > 0)
 		keep = n >= cap ? cap - 1 : n;
-	if(keep > 0 && ipcreadn(fd, dst, keep) < 0)
+	if(keep > 0 && readwait(fd, dst, keep, until) < 0)
 		return -1;
 	if(cap > 0)
 		dst[keep] = '\0';
 	n -= keep;
 	while(n > 0){
 		part = n < sizeof discard ? n : sizeof discard;
-		if(ipcreadn(fd, discard, part) < 0)
+		if(readwait(fd, discard, part, until) < 0)
 			return -1;
 		n -= part;
 	}
+	*copied = keep;
 	return 0;
 }
 
@@ -299,6 +432,8 @@ ipcreadresp(int fd, int want, char *commit, size_t ccap,
 	char *preedit, size_t pcap, Ipcresp *resp)
 {
 	unsigned char hdr[Ipcresphdrsz], npreedit[Ipclensz];
+	size_t wirelen;
+	int64_t until;
 
 	if(resp == NULL || (ccap > 0 && commit == NULL) ||
 	   (pcap > 0 && preedit == NULL))
@@ -308,20 +443,30 @@ ipcreadresp(int fd, int want, char *commit, size_t ccap,
 	if(pcap > 0)
 		preedit[0] = '\0';
 	memset(resp, 0, sizeof *resp);
-	if(ipcreadn(fd, hdr, sizeof hdr) < 0)
+	until = deadline();
+	if(until < 0 || readwait(fd, hdr, sizeof hdr, until) < 0)
 		return -1;
-	resp->eaten = hdr[0] != 0;
-	resp->ncommit = getlen(hdr + 1);
-	if(resp->ncommit > Ipcfieldmax)
+	if(hdr[0] > 1){
+		errno = EPROTO;
 		return -1;
-	if(readfield(fd, resp->ncommit, commit, ccap) < 0)
+	}
+	resp->eaten = hdr[0];
+	wirelen = getlen(hdr + 1);
+	if(wirelen > Ipcfieldmax){
+		errno = EPROTO;
+		return -1;
+	}
+	if(readfield(fd, wirelen, commit, ccap, &resp->commitlen, until) < 0)
 		return -1;
 	if(!want)
 		return 0;
-	if(ipcreadn(fd, npreedit, sizeof npreedit) < 0)
+	if(readwait(fd, npreedit, sizeof npreedit, until) < 0)
 		return -1;
-	resp->npreedit = getlen(npreedit);
-	if(resp->npreedit > Ipcfieldmax)
+	wirelen = getlen(npreedit);
+	if(wirelen > Ipcfieldmax){
+		errno = EPROTO;
 		return -1;
-	return readfield(fd, resp->npreedit, preedit, pcap);
+	}
+	return readfield(fd, wirelen, preedit, pcap, &resp->preeditlen,
+		until);
 }

ipc.h

@@ -50,6 +50,7 @@ enum
 	Ipcresphdrsz = 1 + Ipclensz,
 	Ipcfieldmax = 256,
 	Ipcmaxresp = Ipcresphdrsz + Ipcfieldmax + Ipclensz + Ipcfieldmax,
+	Ipcwaitms = 250,
 };
 
 enum
@@ -64,8 +65,9 @@ typedef struct Ipcresp Ipcresp;
 struct Ipcresp
 {
 	int	eaten;
-	size_t	ncommit;
-	size_t	npreedit;
+	/* Bytes copied to each caller buffer, excluding its trailing NUL. */
+	size_t	commitlen;
+	size_t	preeditlen;
 };
 
 void	ipcpackreq(unsigned char[Ipcreqsz], int, uint32_t, uint32_t);

srv.c

@@ -60,7 +60,7 @@ clientthread(void *arg)
 	Caret caret;
 	uchar out[Ipcmaxresp];
 	char commit[Maxutf], preedit[Maxutf];
-	int cap, n, ncommit, npreedit, type, want;
+	int cap, n, ncommit, negotiated, npreedit, type, want;
 	uchar token;
 
 	fd = (int)(uintptr)arg;
@@ -69,11 +69,15 @@ clientthread(void *arg)
 	kr.reply = reply;
 	kr.owner = &fd;
 	cap = Cclientpreedit;
+	negotiated = 0;
 	memset(&caret, 0, sizeof caret);
 	kr.cap = cap;
 	kr.caret = caret;
 	while((type = srvreadreq(fd, &kr, &want)) >= 0){
-		if(type == Ipccap)
+		if(type == Ipccap){
+			cap = want ? Cclientpreedit : 0;
+			negotiated = 1;
+		}else if(type == Ipclegacy && !negotiated)
 			cap = want ? Cclientpreedit : 0;
 		else if(type == Ipccaret)
 			caret = kr.caret;

tests/ipc_test.c

@@ -1,10 +1,31 @@
 #define _POSIX_C_SOURCE 200809L
 
+#include <errno.h>
+#include <fcntl.h>
 #include <stdlib.h>
 #include <sys/socket.h>
+#include <sys/un.h>
+#include <time.h>
 #include <unistd.h>
 #include "test.h"
 
+#undef accept
+#undef listen
+#undef send
+
+static int64_t
+nowms(void)
+{
+	struct timespec ts;
+
+	if(clock_gettime(CLOCK_MONOTONIC, &ts) < 0)
+		return 0;
+	return (int64_t)ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
+}
+
+static void ipcclientdeadlines(struct ct*);
+static void ipcconnectcloexec(struct ct*);
+
 void
 ipc_masks_modifiers(struct ct *t)
 {
@@ -196,15 +217,15 @@ ipc_response_empty_and_preedit(struct ct *t)
 	   CT_EQ_INT(t, 0, ipcreadresp(fd[1], 0, commit, sizeof commit,
 		nil, 0, &resp))){
 		CT_EQ_INT(t, 0, resp.eaten);
-		CT_EQ_SIZE(t, 0, resp.ncommit);
-		CT_EQ_SIZE(t, 0, resp.npreedit);
+		CT_EQ_SIZE(t, 0, resp.commitlen);
+		CT_EQ_SIZE(t, 0, resp.preeditlen);
 		CT_EQ_STR(t, "", commit);
 	}
 	if(CT_EQ_INT(t, 0, ipcreadresp(fd[1], 1, commit, sizeof commit,
 		preedit, sizeof preedit, &resp))){
 		CT_EQ_INT(t, 1, resp.eaten);
-		CT_EQ_SIZE(t, 2, resp.ncommit);
-		CT_EQ_SIZE(t, 4, resp.npreedit);
+		CT_EQ_SIZE(t, 2, resp.commitlen);
+		CT_EQ_SIZE(t, 4, resp.preeditlen);
 		CT_EQ_STR(t, "go", commit);
 		CT_EQ_STR(t, "kana", preedit);
 	}
@@ -234,8 +255,8 @@ ipc_response_max_and_drain(struct ct *t)
 	   CT_EQ_INT(t, 0, ipcsend(fd[0], second, nsecond)) &&
 	   CT_EQ_INT(t, 0, ipcreadresp(fd[1], 1, commit, sizeof commit,
 		preedit, sizeof preedit, &resp))){
-		CT_EQ_SIZE(t, Ipcfieldmax, resp.ncommit);
-		CT_EQ_SIZE(t, Ipcfieldmax, resp.npreedit);
+		CT_EQ_SIZE(t, sizeof commit - 1, resp.commitlen);
+		CT_EQ_SIZE(t, sizeof preedit - 1, resp.preeditlen);
 		CT_EQ_STR(t, "xxxxxxx", commit);
 		CT_EQ_STR(t, "xxxxxxx", preedit);
 	}
@@ -284,6 +305,19 @@ ipc_response_fragmented_and_truncated(struct ct *t)
 		preedit, sizeof preedit, &resp));
 	close(fd[0]);
 	close(fd[1]);
+
+	if(socketpair(AF_UNIX, SOCK_STREAM, 0, fd) < 0){
+		CT_ERRORF(t, "socketpair failed");
+		return;
+	}
+	frame[0] = 2;
+	CT_EQ_INT(t, 0, ipcsend(fd[0], frame, n));
+	errno = 0;
+	CT_EQ_INT(t, -1, ipcreadresp(fd[1], 1, commit, sizeof commit,
+		preedit, sizeof preedit, &resp));
+	CT_EQ_INT(t, EPROTO, errno);
+	close(fd[0]);
+	close(fd[1]);
 }
 
 void
@@ -298,4 +332,99 @@ ipc_broken_peer_send(struct ct *t)
 	close(fd[1]);
 	CT_EQ_INT(t, -1, ipcsend(fd[0], "x", 1));
 	close(fd[0]);
+	ipcclientdeadlines(t);
+	ipcconnectcloexec(t);
+}
+
+static void
+ipcclientdeadlines(struct ct *t)
+{
+	char fill[4096], commit[8];
+	Ipcresp resp;
+	int fd[2];
+	int64_t start, elapsed;
+	ssize_t n;
+
+	if(socketpair(AF_UNIX, SOCK_STREAM, 0, fd) < 0){
+		CT_ERRORF(t, "socketpair failed");
+		return;
+	}
+	start = nowms();
+	errno = 0;
+	CT_EQ_INT(t, -1, ipcreadresp(fd[0], 0, commit, sizeof commit,
+		nil, 0, &resp));
+	elapsed = nowms() - start;
+	CT_EQ_INT(t, ETIMEDOUT, errno);
+	CT_CHECK(t, elapsed >= 0 && elapsed < 4*Ipcwaitms);
+
+	memset(fill, 'x', sizeof fill);
+	for(;;){
+		n = send(fd[0], fill, sizeof fill, MSG_DONTWAIT|MSG_NOSIGNAL);
+		if(n < 0 && (errno == EAGAIN || errno == EWOULDBLOCK))
+			break;
+		if(n <= 0){
+			CT_ERRORF(t, "fill send failed: %s", strerror(errno));
+			close(fd[0]);
+			close(fd[1]);
+			return;
+		}
+	}
+	start = nowms();
+	errno = 0;
+	CT_EQ_INT(t, -1, ipcsend(fd[0], "x", 1));
+	elapsed = nowms() - start;
+	CT_EQ_INT(t, ETIMEDOUT, errno);
+	CT_CHECK(t, elapsed >= 0 && elapsed < 4*Ipcwaitms);
+	close(fd[0]);
+	close(fd[1]);
+}
+
+static void
+ipcconnectcloexec(struct ct *t)
+{
+	struct sockaddr_un addr;
+	char root[] = "/tmp/strans-ipc.XXXXXX";
+	char *old, *saved;
+	int accepted, client, flags, listener;
+
+	accepted = client = listener = -1;
+	old = getenv("XDG_RUNTIME_DIR");
+	saved = old == nil ? nil : strdup(old);
+	if(mkdtemp(root) == nil){
+		CT_ERRORF(t, "mkdtemp failed: %s", strerror(errno));
+		free(saved);
+		return;
+	}
+	setenv("XDG_RUNTIME_DIR", root, 1);
+	memset(&addr, 0, sizeof addr);
+	addr.sun_family = AF_UNIX;
+	if(ipcpath(addr.sun_path, sizeof addr.sun_path) < 0 ||
+	   (listener = socket(AF_UNIX, SOCK_STREAM, 0)) < 0 ||
+	   bind(listener, (struct sockaddr*)&addr, sizeof addr) < 0 ||
+	   listen(listener, 1) < 0){
+		CT_ERRORF(t, "listen failed: %s", strerror(errno));
+		goto Out;
+	}
+	client = ipcconnect();
+	if(!CT_CHECK(t, client >= 0))
+		goto Out;
+	accepted = accept(listener, nil, nil);
+	if(!CT_CHECK(t, accepted >= 0))
+		goto Out;
+	flags = fcntl(client, F_GETFD);
+	CT_CHECK(t, flags >= 0 && (flags & FD_CLOEXEC) != 0);
+Out:
+	if(accepted >= 0)
+		close(accepted);
+	if(client >= 0)
+		close(client);
+	if(listener >= 0)
+		close(listener);
+	unlink(addr.sun_path);
+	rmdir(root);
+	if(saved != nil){
+		setenv("XDG_RUNTIME_DIR", saved, 1);
+		free(saved);
+	}else
+		unsetenv("XDG_RUNTIME_DIR");
 }

tests/server_test.c

@@ -340,7 +340,7 @@ server_connection_ownership(struct ct *t)
 
 	if(!sendkey(t, &b, 0, 0, 'k'))
 		goto cleanup;
-	req = nextrequest(t, &gate, Keypress);
+	req = nextrequestcap(t, &gate, Keypress, 0);
 	CT_EQ_PTR(t, bowner, req.owner);
 	allowrequest(&gate);
 	CT_CHECK(t, readreply(t, &b, 0, nil, 0));