commit ae671b8dd620635196d0145e38f2502f047ee1c0
parent 7c3700015d907a650b876d543fe28407f8b9b5ea
Author: Jacob R. Edwards <jacobouno@protonmail.com>
Date: Thu, 20 May 2021 18:53:11 -0700
Add client response buffers
This prevents clients from stalling the server by not reading data.
Diffstat:
M | TODO | | | 1 | - |
M | aps.c | | | 24 | +++++++++++++++--------- |
M | aps.h | | | 7 | +++++-- |
M | command.c | | | 74 | ++++++++++++++++++++++++++++++++++++++++++++------------------------------ |
M | mkfile | | | 4 | ++-- |
M | player.c | | | 2 | -- |
A | response.c | | | 98 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
A | response.h | | | 12 | ++++++++++++ |
M | util.h | | | 1 | + |
9 files changed, 177 insertions(+), 46 deletions(-)
diff --git a/TODO b/TODO
@@ -1,4 +1,3 @@
- Create a client library using functions from apc.c
-- Don't wait for clients to read data
- Organize where functions are defined
- Rename strutures where appropriate
diff --git a/aps.c b/aps.c
@@ -36,6 +36,7 @@
#include "config.h"
#include "player.h"
#include "queue.h"
+#include "response.h"
#include "sock.h"
#include "util.h"
@@ -103,9 +104,12 @@ int
aps_handle(struct aps *aps, int fd)
{
if (aps->pfds[fd].revents & (POLLERR | POLLHUP | POLLNVAL))
- return aps_drop(aps, fd);
- if (aps->pfds[fd].revents & POLLIN)
- return aps_command(aps, fd);
+ return 1;
+ if (aps->pfds[fd].revents & POLLIN && aps_command(aps, fd))
+ return 1;
+ if (aps->pfds[fd].revents & POLLOUT && aps->clients[fd].response.lock &&
+ resp_send(aps, fd))
+ return 1;
return 0;
}
@@ -119,10 +123,10 @@ aps_update(struct aps *aps)
return 1;
for (i = 0; re > 0 && i < LEN(aps->pfds); ++i) {
- if (!aps->pfds[i].revents)
- continue;
- aps_handle(aps, i);
- --re;
+ if (aps->pfds[i].revents && aps_handle(aps, i)) {
+ aps_drop(aps, i);
+ --re;
+ }
}
return 0;
@@ -151,7 +155,7 @@ aps_open(struct aps *aps)
for (i = 0; i < LEN(aps->pfds); ++i) {
aps->pfds[i].fd = -1;
- aps->pfds[i].events = POLLIN;
+ aps->pfds[i].events = POLLIN | POLLOUT;
}
return 0;
}
@@ -191,7 +195,9 @@ main(int argc, char *argv[])
aps.player.state = STOPPED;
strcpy(aps.next, "+1");
- if (aps_open(&aps) || run(&aps)) {
+ if (aps_open(&aps))
+ die("open connection");
+ if (run(&aps)) {
perror("error");
aps_close(&aps);
return 1;
diff --git a/aps.h b/aps.h
@@ -5,8 +5,11 @@ struct aps {
int playing;
struct apcon con;
struct client {
- char buf[AP_LINE_MAX];
- int len;
+ struct request {
+ char buf[AP_LINE_MAX];
+ int len;
+ } request;
+ struct response response;
} clients[OPEN_MAX];
struct player player;
struct pollfd pfds[OPEN_MAX];
diff --git a/command.c b/command.c
@@ -32,34 +32,47 @@
#include "apcon.h"
#include "player.h"
#include "queue.h"
-#include "aps.h"
+#include "response.h"
#include "util.h"
+#include "aps.h"
+
static char *earg = "Invalid number of arguments";
static char *enotfound = "Not found";
static char *eposition = "No position in queue";
static char *etoofew = "Too few arguments";
int
-resp(int s, char *error)
+resp_finish(struct aps *aps, int s, char *error)
{
- if (error)
- return dprintf(s, "error: %s\n", error) < 0;
- else
- return send(s, "ok\n", 3, 0) != 3;
+ char buf[AP_LINE_MAX];
+ int len;
+
+ if (error) {
+ len = snprintf(buf, sizeof(buf), "error: %s\n", error);
+ } else {
+ len = 3;
+ memcpy(buf, "ok\n", len);
+ }
+
+ if (len >= sizeof(buf))
+ ERET(EMSGSIZE);
+ if (resp_add(aps, s, buf, len))
+ return 1;
+ resp_end(aps, s);
+ return 0;
}
int
-send_item(int s, struct item *item)
+resp_additem(struct aps *aps, int s, char *item)
{
- char buf[PATH_MAX + 1];
- int len;
-
- len = strlen(item->path);
- memcpy(buf, item->path, len);
- buf[len++] = '\n';
+ char buf[AP_LINE_MAX];
+ int n;
- return send(s, buf, len, 0) != len;
+ n = snprintf(buf, sizeof(buf), "%s\n", item);
+ if (n >= sizeof(buf))
+ ERET(EMSGSIZE);
+ return resp_add(aps, s, buf, n);
}
int
@@ -124,7 +137,7 @@ aps_list(struct aps *aps, int s, int argc, char **argv)
item = aps->queue.head;
while ((item = nextmatch(item_next, item, argv + 1, argc - 1, 1))) {
- if (send_item(s, item))
+ if (resp_additem(aps, s, item->path))
return strerror(errno);
item = item->next;
}
@@ -253,7 +266,7 @@ aps_name(struct aps *aps, int s, int argc, char **argv)
if (item == NULL)
return err;
- if (send_item(s, item))
+ if (resp_additem(aps, s, item->path))
return strerror(errno);
return NULL;
}
@@ -309,36 +322,37 @@ aps_command(struct aps *aps, int s)
};
c = &aps->clients[s];
- if (c->len == sizeof(c->buf)) {
- resp(s, strerror(EMSGSIZE));
- c->len = 0;
+ if (c->request.len == sizeof(c->request.buf)) {
+ c->request.len = 0;
+ return resp_finish(aps, s, strerror(EMSGSIZE));
}
- len = recv(s, c->buf + c->len, LEN(c->buf) - c->len, 0);
+ len = recv(s, c->request.buf + c->request.len, LEN(c->request.buf) - c->request.len, 0);
if (len == -1)
return 1;
- end = memchr(c->buf + c->len, '\n', len);
- c->len += len;
+ end = memchr(c->request.buf + c->request.len, '\n', len);
+ c->request.len += len;
if (end == NULL)
return 0; /* not a full command yet */
*end = 0;
- len = end - c->buf + 1;
- memcpy(buf, c->buf, len);
- memmove(c->buf, c->buf + len, c->len - len);
- c->len -= len;
+ len = end - c->request.buf + 1;
+ memcpy(buf, c->request.buf, len);
+ memmove(c->request.buf, c->request.buf + len, c->request.len - len);
+ c->request.len -= len;
argc = split(argv, LEN(argv), buf, argseps);
if (!argc)
- return resp(s, "No command");
+ return resp_finish(aps, s, "No command");
if (argc < 0)
- return resp(s, strerror(E2BIG));
+ return resp_finish(aps, s, strerror(E2BIG));
for (i = 0; i < LEN(commands); ++i) {
if (strcmp(*argv, commands[i].name) == 0)
- return resp(s, commands[i].func(aps, s, argc, argv));
+ return resp_finish(aps, s,
+ commands[i].func(aps, s, argc, argv));
}
- return resp(s, "Command not found");
+ return resp_finish(aps, s, "Command not found");
}
diff --git a/mkfile b/mkfile
@@ -8,8 +8,8 @@ ldlibs = -lc
prefix = /usr/local
-names = aps apc
-src = sock.c apcon.c util.c command.c queue.c player.c
+names = aps apc badapc
+src = sock.c apcon.c util.c command.c queue.c player.c response.c
mainobj = ${names:%=%.o}
obj = ${src:%.c=%.o}
diff --git a/player.c b/player.c
@@ -29,8 +29,6 @@
#include "player.h"
#include "util.h"
-#define ERET(E) { errno = E; return 1; }
-
int
pstart(struct player *p)
{
diff --git a/response.c b/response.c
@@ -0,0 +1,98 @@
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <limits.h>
+#include <poll.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "ap.h"
+#include "apcon.h"
+#include "player.h"
+#include "queue.h"
+#include "response.h"
+#include "util.h"
+
+#include "aps.h"
+
+int
+resp_enlarge(struct aps *aps, int fd, unsigned int more)
+{
+ struct response *resp;
+ unsigned int newsiz;
+ void *tmp;
+
+ resp = &aps->clients[fd].response;
+ if (resp->len + more < resp->len)
+ ERET(EOVERFLOW);
+
+ newsiz = (resp->siz ? resp->siz * 2 : BUFSIZ);
+ while (newsiz < resp->len + more) {
+ if (newsiz *= 2 < resp->siz)
+ ERET(EOVERFLOW);
+ }
+
+ tmp = realloc(resp->buf, newsiz);
+ if (tmp == NULL) {
+ resp_free(aps, fd);
+ return 1;
+ }
+
+ resp->buf = tmp;
+ resp->siz = newsiz;
+ return 0;
+}
+
+int
+resp_add(struct aps *aps, int fd, void *data, unsigned int len)
+{
+ struct response *resp;
+
+ resp = &aps->clients[fd].response;
+ if (resp->len + len > resp->siz && resp_enlarge(aps, fd, len))
+ return 1;
+
+ memcpy(resp->buf + resp->len, data, len);
+ resp->len += len;
+ return 0;
+}
+
+int
+resp_send(struct aps *aps, int fd)
+{
+ ssize_t n;
+ struct response *resp;
+
+ resp = &aps->clients[fd].response;
+ if (!resp->lock || resp->len - resp->sent == 0)
+ return 1;
+
+ n = send(fd, resp->buf + resp->sent, resp->len - resp->sent, 0);
+ switch (n) {
+ case -1:
+ return 1;
+ case 0:
+ ERET(ECONNRESET);
+ default:
+ if (resp->sent += n == resp->len)
+ resp_free(aps, fd);
+ return 0;
+ }
+}
+
+void
+resp_end(struct aps *aps, int fd)
+{
+ aps->clients[fd].response.lock = 1;
+}
+
+void
+resp_free(struct aps *aps, int fd)
+{
+ struct response *resp;
+
+ resp = &aps->clients[fd].response;
+ free(resp->buf);
+ memset(resp, 0, sizeof(*resp));
+}
diff --git a/response.h b/response.h
@@ -0,0 +1,12 @@
+struct response {
+ int lock;
+ unsigned int len;
+ unsigned int sent;
+ unsigned int siz;
+ void *buf;
+};
+
+int resp_add(struct aps *, int, void *, unsigned int);
+int resp_send(struct aps *, int);
+void resp_end(struct aps *, int);
+void resp_free(struct aps *, int);
diff --git a/util.h b/util.h
@@ -1,3 +1,4 @@
+#define ERET(E) { errno = E; return 1; }
#define ERROR(S) (write(2, S, sizeof(S) - 1) != sizeof(S) - 1)
#define LEN(X) (sizeof(X) / sizeof(*X))