commit da3b05c3a9d7dbe7e159d444994f89c5347f6bf6
parent ff091937d01c2f508c8c999d224a664585619743
Author: Jacob R. Edwards <jacobouno@protonmail.com>
Date: Mon, 2 Aug 2021 18:57:33 -0700
Revise and add logging and input buffer overflow check
The logging functions are now much simpler and they don't do anything
other than log. Additionally more logging is done.
Diffstat:
8 files changed, 54 insertions(+), 59 deletions(-)
diff --git a/aps/aps.c b/aps/aps.c
@@ -87,6 +87,7 @@ aps_close(struct aps *aps)
int
aps_drop(struct aps *aps, int fd)
{
+ aps_log(aps, INFO, fd, "dropping client");
--aps->nfds;
aps->pfds[fd].fd = -1;
respfree(aps, fd);
@@ -109,11 +110,12 @@ aps_accept(struct aps *aps)
return r;
while ((s = accept4(aps->con->sock, NULL, NULL, 0)) >= 0) {
+ aps_log(aps, INFO, s, "client accepted");
++aps->nfds;
aps->pfds[s].fd = s;
if ((aps->clients[s].request = bufnew()) == NULL ||
respnew(aps, s))
- aps_drop(aps, s);
+ aps_errordrop(aps, s, errstr);
}
return errno != EWOULDBLOCK;
@@ -165,9 +167,12 @@ aps_read(struct aps *aps, int s)
struct buf *buf;
buf = aps->clients[s].request;
- if (buf->len == buf->size && bufresize(buf, MAX(buf->size, 128) * 2)) {
- aps_log(aps, ERROR, strerror(errno), s);
- return NULL;
+ if (buf->len == buf->size) {
+ if (buf->size == UINT_MAX || bufresize(buf,
+ (buf->size > UINT_MAX / 4) ? UINT_MAX : (MAX(128, buf->size) * 4))) {
+ aps_errordrop(aps, s, errstr);
+ return NULL;
+ }
}
len = recv(s, buf->data + buf->len, buf->size - buf->len, 0);
@@ -198,6 +203,7 @@ aps_command(struct aps *aps, int s)
return 0;
buf = aps->clients[s].request;
+ aps_log(aps, COMMAND, s, buf->data);
argv = asplit(buf->data, AP_ARG_SEPS);
if (argv == NULL)
return 1;
@@ -222,3 +228,11 @@ aps_command(struct aps *aps, int s)
free(argv);
return respfinish(aps, s, status);
}
+
+void
+aps_errordrop(struct aps *aps, int fd, char *why)
+{
+ /* One could send(2) the error aswell. */
+ aps_log(aps, ERROR, fd, why);
+ aps_drop(aps, fd);
+}
diff --git a/aps/aps.h b/aps/aps.h
@@ -32,3 +32,4 @@ int aps_handle(struct aps *, int);
int aps_update(struct aps *);
char *aps_read(struct aps *, int);
int aps_command(struct aps *, int);
+void aps_errordrop(struct aps *, int, char *);
diff --git a/aps/command.c b/aps/command.c
@@ -42,7 +42,7 @@ findset(struct aps *aps, char **patterns, unsigned int len, unsigned int flags)
if (item == NULL)
return enotfound;
if (queue_set(aps, item))
- return strerror(errno);
+ return errstr;
return NULL;
}
@@ -64,7 +64,7 @@ aps_add(struct aps *aps, int s, int argc, char **argv)
for (i = 0; i < argc; ++i) {
if (queue_add(aps, argv[i]))
- return strerror(errno);
+ return errstr;
}
return NULL;
}
@@ -104,7 +104,7 @@ aps_list(struct aps *aps, int s, int argc, char **argv)
item = aps->queue;
while ((item = finddir(findnext, item, aps->queue, argv, argc, 0))) {
if (respadd(aps, s, item->path))
- return strerror(errno);
+ return errstr;
item = findnext(item);
if (item == aps->queue)
return NULL;
@@ -129,7 +129,7 @@ aps_play(struct aps *aps, int s, int argc, char **argv)
else if (aps->queue == NULL)
return eposition;
if (pplay(aps->player, aps->queue->path))
- return strerror(errno);
+ return errstr;
return NULL;
}
@@ -137,7 +137,7 @@ char *
aps_pause(struct aps *aps, int s, int argc, char **argv)
{
if (psuspend(aps->player))
- return strerror(errno);
+ return errstr;
return NULL;
}
@@ -145,7 +145,7 @@ char *
aps_stop(struct aps *aps, int s, int argc, char **argv)
{
if (pstop(aps->player))
- return strerror(errno);
+ return errstr;
return NULL;
}
@@ -153,7 +153,7 @@ char *
aps_toggle(struct aps *aps, int s, int argc, char **argv)
{
if (ptoggle(aps->player))
- return strerror(errno);
+ return errstr;
return NULL;
}
@@ -167,7 +167,7 @@ aps_name(struct aps *aps, int s, int argc, char **argv)
return enotfound;
if (respadd(aps, s, item->path))
- return strerror(errno);
+ return errstr;
return NULL;
}
@@ -176,7 +176,7 @@ aps_next(struct aps *aps, int s, int argc, char **argv)
{
if (argc) {
if (argsub(&aps->next, argv, argc) == NULL)
- return strerror(errno);
+ return errstr;
return NULL;
}
return findplay(aps, aps->next, arglen(aps->next), 0);
@@ -217,6 +217,6 @@ aps_status(struct aps *aps, int s, int argc, char **argv)
}
if (respadd(aps, s, status))
- return strerror(errno);
+ return errstr;
return NULL;
}
diff --git a/aps/log.c b/aps/log.c
@@ -16,24 +16,15 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
-#include <sys/uio.h>
#include <sys/socket.h>
-#include <errno.h>
#include <limits.h>
#include <poll.h>
-#include <string.h>
+#include <stdio.h>
#include "aps.h"
#include "log.h"
-void
-aploadiov(struct iovec *v, char *s)
-{
- v->iov_base = s;
- v->iov_len = strlen(s);
-}
-
char *
apstrll(int level)
{
@@ -46,41 +37,22 @@ apstrll(int level)
return "error";
case FATAL:
return "fatal";
+ case COMMAND:
+ return "command";
default:
return "undefined";
}
}
void
-aplog(int level, char *message, int error)
+aplog(int level, int fd, char *msg)
{
- struct iovec *v;
- struct iovec iov[7];
-
- v = iov;
- aploadiov(v++, "aplog.");
- aploadiov(v++, apstrll(level));
- if (message) {
- aploadiov(v++, ": ");
- aploadiov(v++, message);
- }
- if (error) {
- aploadiov(v++, ": ");
- aploadiov(v++, strerror(error));
- }
- aploadiov(v++, ".\n");
- writev(2, iov, v - iov);
+ fprintf(stderr, "ap.%s: [%d] %s\n", apstrll(level), fd, msg);
}
void
-aps_log(struct aps *aps, int level, char *message, int fd)
+aps_log(struct aps *aps, int level, int fd, char *msg)
{
if (aps == NULL || level & aps->logmask)
- aplog(level, message, errno);
- if (aps != NULL) {
- if (level == FATAL || (fd == 0 && level == ERROR))
- aps->close = 1;
- else if (level == ERROR && aps_drop(aps, fd))
- aps_log(aps, FATAL, "unable to drop client", 0);
- }
+ aplog(level, fd, msg);
}
diff --git a/aps/log.h b/aps/log.h
@@ -1,9 +1,10 @@
enum {
- INFO = 1,
- WARN = 2,
- ERROR = 3,
- FATAL = 4
+ INFO = 1 << 0,
+ WARN = 1 << 1,
+ ERROR = 1 << 2,
+ FATAL = 1 << 3,
+ COMMAND = 1 << 4
};
-void aplog(int, char *, int);
-void aps_log(struct aps *, int, char *, int);
+void aplog(int, int, char *);
+void aps_log(struct aps *, int, int, char *);
diff --git a/aps/main.c b/aps/main.c
@@ -24,6 +24,7 @@
#include <poll.h>
#include <signal.h>
#include <stdio.h>
+#include <string.h>
#ifdef __OpenBSD__
#include <unistd.h>
@@ -54,13 +55,14 @@ static struct command commands[] = {
void
slog(char *s)
{
- aplog(FATAL, s, errno);
+ aplog(FATAL, 0, errstr);
}
void
sigclose(int sig)
{
- aps_log(aps, FATAL, "signal", 0);
+ aps_log(aps, FATAL, 0, "close signal");
+ aps->close = 1;
}
void
@@ -118,7 +120,7 @@ main(int argc, char *argv[])
}
aps->player->state = STOPPED;
- aps->logmask = INFO | WARN | ERROR | FATAL;
+ aps->logmask = ~0;
aps->coms = commands;
aps->ncoms = LEN(commands);
diff --git a/aps/response.c b/aps/response.c
@@ -26,6 +26,8 @@
#include <string.h>
#include "aps.h"
+#include "log.h"
+#include "util.h"
#define RESP (aps->clients[fd].resp)
@@ -66,8 +68,9 @@ respfinish(struct aps *aps, int fd, char *error)
if (error == NULL) {
/* NOTE: Perhaps it should return an error? */
if (respadd(aps, fd, "ok"))
- respfinish(aps, fd, strerror(errno));
+ respfinish(aps, fd, errstr);
} else {
+ aps_log(aps, COMMAND, fd, error);
aps->clients[fd].resp->buf->len = 0;
if (bufappend(RESP->buf, "error: ", 7) ||
bufappend(RESP->buf, error, strlen(error)) ||
diff --git a/aps/util.h b/aps/util.h
@@ -7,3 +7,5 @@
#define ASSERT(EXP) ((EXP) ? 0 : BUG(#EXP))
#define BUG(WHY) (bug(__FILE__, __LINE__, WHY))
+
+#define errstr (strerror(errno))
