Commit 99468b85 authored by Eric Wong's avatar Eric Wong

networking: more assertions and cleanups to size_t/unsigned changes

Basically, I don't trust myself nor Max to not have bugs in our code when switching over to unsigned types, so I've added more assertions which will hopefully trip and force us to fix these bugs before somebody can exploit them :) Some cleanups for parameter parsing using strtol and error reporting to the user. Also, fix some completely garbled indentation in inputStream_http.c git-svn-id: https://svn.musicpd.org/mpd/trunk@7209 09075e82-0dd4-0310-85a5-a0d7c8717e4f
parent 9ba72643
...@@ -57,8 +57,8 @@ typedef struct _InputStreemHTTPData { ...@@ -57,8 +57,8 @@ typedef struct _InputStreemHTTPData {
size_t icyOffset; size_t icyOffset;
char *proxyAuth; char *proxyAuth;
char *httpAuth; char *httpAuth;
/* Number of times mpd tried to get data */ /* Number of times mpd tried to get data */
int tries; int tries;
} InputStreamHTTPData; } InputStreamHTTPData;
void inputStream_initHttp(void) void inputStream_initHttp(void)
...@@ -113,37 +113,34 @@ void inputStream_initHttp(void) ...@@ -113,37 +113,34 @@ void inputStream_initHttp(void)
param = getConfigParam(CONF_HTTP_BUFFER_SIZE); param = getConfigParam(CONF_HTTP_BUFFER_SIZE);
if (param) { if (param) {
bufferSize = strtoul(param->value, &test, 10); long tmp = strtol(param->value, &test, 10);
if (*test != '\0' || tmp <= 0) {
if (*test != '\0') {
FATAL("\"%s\" specified for %s at line %i is not a " FATAL("\"%s\" specified for %s at line %i is not a "
"positive integer\n", "positive integer\n",
param->value, CONF_HTTP_BUFFER_SIZE, param->line); param->value, CONF_HTTP_BUFFER_SIZE, param->line);
} }
bufferSize *= 1024; bufferSize = tmp * 1024;
if (prebufferSize > bufferSize)
prebufferSize = bufferSize;
} }
param = getConfigParam(CONF_HTTP_PREBUFFER_SIZE); param = getConfigParam(CONF_HTTP_PREBUFFER_SIZE);
if (param) { if (param) {
prebufferSize = strtoul(param->value, &test, 10); long tmp = strtol(param->value, &test, 10);
if (*test != '\0' || tmp <= 0) {
if (prebufferSize <= 0 || *test != '\0') {
FATAL("\"%s\" specified for %s at line %i is not a " FATAL("\"%s\" specified for %s at line %i is not a "
"positive integer\n", "positive integer\n",
param->value, CONF_HTTP_PREBUFFER_SIZE, param->value, CONF_HTTP_PREBUFFER_SIZE,
param->line); param->line);
} }
prebufferSize *= 1024; prebufferSize = tmp * 1024;
} }
if (prebufferSize > bufferSize) if (prebufferSize > bufferSize)
prebufferSize = bufferSize; prebufferSize = bufferSize;
assert(bufferSize > 0 && "http bufferSize too small");
assert(prebufferSize > 0 && "http prebufferSize too small");
} }
/* base64 code taken from xmms */ /* base64 code taken from xmms */
...@@ -157,7 +154,7 @@ static char *base64Dup(char *s) ...@@ -157,7 +154,7 @@ static char *base64Dup(char *s)
char *ret = xcalloc(BASE64_LENGTH(len) + 1, 1); char *ret = xcalloc(BASE64_LENGTH(len) + 1, 1);
unsigned char *p = (unsigned char *)ret; unsigned char *p = (unsigned char *)ret;
char tbl[64] = { static const char tbl[64] = {
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H',
'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X',
...@@ -240,7 +237,7 @@ static InputStreamHTTPData *newInputStreamHTTPData(void) ...@@ -240,7 +237,7 @@ static InputStreamHTTPData *newInputStreamHTTPData(void)
ret->prebuffer = 0; ret->prebuffer = 0;
ret->icyOffset = 0; ret->icyOffset = 0;
ret->buffer = xmalloc(bufferSize); ret->buffer = xmalloc(bufferSize);
ret->tries = 0; ret->tries = 0;
return ret; return ret;
} }
...@@ -776,6 +773,9 @@ size_t inputStream_httpRead(InputStream * inStream, void *ptr, size_t size, ...@@ -776,6 +773,9 @@ size_t inputStream_httpRead(InputStream * inStream, void *ptr, size_t size,
if (data->icyMetaint > 0) { if (data->icyMetaint > 0) {
if (data->icyOffset >= data->icyMetaint) { if (data->icyOffset >= data->icyMetaint) {
size_t metalen = *(data->buffer); size_t metalen = *(data->buffer);
/* maybe we're in some strange universe where a byte
* can hold more than 255 ... */
assert(metalen <= 255 && "metalen greater than 255");
metalen <<= 4; metalen <<= 4;
if (metalen + 1 > data->buflen) { if (metalen + 1 > data->buflen) {
/* damn that's some fucking big metadata! */ /* damn that's some fucking big metadata! */
...@@ -796,6 +796,8 @@ size_t inputStream_httpRead(InputStream * inStream, void *ptr, size_t size, ...@@ -796,6 +796,8 @@ size_t inputStream_httpRead(InputStream * inStream, void *ptr, size_t size,
data->buflen); data->buflen);
data->icyOffset = 0; data->icyOffset = 0;
} }
assert(data->icyOffset <= data->icyMetaint &&
"icyOffset bigger than icyMetaint!");
maxToSend = data->icyMetaint - data->icyOffset; maxToSend = data->icyMetaint - data->icyOffset;
maxToSend = maxToSend > data->buflen ? data->buflen : maxToSend; maxToSend = maxToSend > data->buflen ? data->buflen : maxToSend;
} }
...@@ -879,25 +881,25 @@ int inputStream_httpBuffer(InputStream * inStream) ...@@ -879,25 +881,25 @@ int inputStream_httpBuffer(InputStream * inStream)
data->buflen < bufferSize - 1) { data->buflen < bufferSize - 1) {
readed = read(data->sock, data->buffer + data->buflen, readed = read(data->sock, data->buffer + data->buflen,
bufferSize - 1 - data->buflen); bufferSize - 1 - data->buflen);
/* If the connection is currently unavailable, or interrupted (EINTR) /*
* Don't give an error, so it's retried later. * If the connection is currently unavailable, or
* Max times in a row to re-try this is HTTP_MAX_TRIES * interrupted (EINTR)
*/ * Don't give an error, so it's retried later.
if (readed < 0 && (errno == EAGAIN || errno == EINTR) && data->tries < HTTP_MAX_TRIES) { * Max times in a row to retry this is HTTP_MAX_TRIES
data->tries++; */
DEBUG(__FILE__": Resource unavailable, trying %i times again\n", HTTP_MAX_TRIES-data->tries); if (readed < 0 &&
readed = 0; (errno == EAGAIN || errno == EINTR) &&
data->tries < HTTP_MAX_TRIES) {
data->tries++;
DEBUG(__FILE__": Resource unavailable, trying %i "
"times again\n", HTTP_MAX_TRIES - data->tries);
readed = 0;
} else if (readed <= 0) { } else if (readed <= 0) {
close(data->sock); while (close(data->sock) && errno == EINTR);
if(errno)
DEBUG(__FILE__": Closing connection with error: '%s'\n", strerror(errno));
data->connState = HTTP_CONN_STATE_CLOSED; data->connState = HTTP_CONN_STATE_CLOSED;
readed = 0; readed = 0;
} } else /* readed > 0, reset */
else{ data->tries = 0;
/* Reset the tries back to 0, because we managed to read data */
data->tries = 0;
}
data->buflen += readed; data->buflen += readed;
} }
......
...@@ -318,7 +318,8 @@ static int processLineOfInput(Interface * interface) ...@@ -318,7 +318,8 @@ static int processLineOfInput(Interface * interface)
"(%lu)\n", "(%lu)\n",
interface->num, interface->num,
(unsigned long)interface->cmd_list_size, (unsigned long)interface->cmd_list_size,
(unsigned long)interface_max_command_list_size); (unsigned long)
interface_max_command_list_size);
closeInterface(interface); closeInterface(interface);
ret = COMMAND_RETURN_CLOSE; ret = COMMAND_RETURN_CLOSE;
} else } else
...@@ -362,7 +363,7 @@ static int processBytesRead(Interface * interface, int bytesRead) ...@@ -362,7 +363,7 @@ static int processBytesRead(Interface * interface, int bytesRead)
buf_tail++; buf_tail++;
if (*buf_tail == '\n') { if (*buf_tail == '\n') {
*buf_tail = '\0'; *buf_tail = '\0';
if (interface->bufferLength - interface->bufferPos > 1) { if (interface->bufferLength > interface->bufferPos) {
if (*(buf_tail - 1) == '\r') if (*(buf_tail - 1) == '\r')
*(buf_tail - 1) = '\0'; *(buf_tail - 1) = '\0';
} }
...@@ -382,6 +383,8 @@ static int processBytesRead(Interface * interface, int bytesRead) ...@@ -382,6 +383,8 @@ static int processBytesRead(Interface * interface, int bytesRead)
interface->cmd_list && interface->cmd_list &&
!interface->cmd_list_dup) !interface->cmd_list_dup)
cmd_list_clone(interface); cmd_list_clone(interface);
assert(interface->bufferLength >= interface->bufferPos
&& "bufferLength >= bufferPos");
interface->bufferLength -= interface->bufferPos; interface->bufferLength -= interface->bufferPos;
memmove(interface->buffer, memmove(interface->buffer,
interface->buffer + interface->bufferPos, interface->buffer + interface->bufferPos,
...@@ -563,25 +566,23 @@ void initInterfaces(void) ...@@ -563,25 +566,23 @@ void initInterfaces(void)
param = getConfigParam(CONF_MAX_COMMAND_LIST_SIZE); param = getConfigParam(CONF_MAX_COMMAND_LIST_SIZE);
if (param) { if (param) {
interface_max_command_list_size = strtol(param->value, long tmp = strtol(param->value, &test, 10);
&test, 10); if (*test != '\0' || tmp <= 0) {
if (*test != '\0' || interface_max_command_list_size <= 0) {
FATAL("max command list size \"%s\" is not a positive " FATAL("max command list size \"%s\" is not a positive "
"integer, line %i\n", param->value, param->line); "integer, line %i\n", param->value, param->line);
} }
interface_max_command_list_size *= 1024; interface_max_command_list_size = tmp * 1024;
} }
param = getConfigParam(CONF_MAX_OUTPUT_BUFFER_SIZE); param = getConfigParam(CONF_MAX_OUTPUT_BUFFER_SIZE);
if (param) { if (param) {
interface_max_output_buffer_size = strtol(param->value, long tmp = strtol(param->value, &test, 10);
&test, 10); if (*test != '\0' || tmp <= 0) {
if (*test != '\0' || interface_max_output_buffer_size <= 0) {
FATAL("max output buffer size \"%s\" is not a positive " FATAL("max output buffer size \"%s\" is not a positive "
"integer, line %i\n", param->value, param->line); "integer, line %i\n", param->value, param->line);
} }
interface_max_output_buffer_size *= 1024; interface_max_output_buffer_size = tmp * 1024;
} }
interfaces = xmalloc(sizeof(Interface) * interface_max_connections); interfaces = xmalloc(sizeof(Interface) * interface_max_connections);
...@@ -650,13 +651,16 @@ static void flushInterfaceBuffer(Interface * interface) ...@@ -650,13 +651,16 @@ static void flushInterfaceBuffer(Interface * interface)
if (ret < 0) if (ret < 0)
break; break;
else if ((size_t)ret < buf->size) { else if ((size_t)ret < buf->size) {
assert(interface->deferred_bytes >= ret);
interface->deferred_bytes -= ret; interface->deferred_bytes -= ret;
buf->data = (char *)buf->data + ret; buf->data = (char *)buf->data + ret;
buf->size -= ret; buf->size -= ret;
} else { } else {
struct sllnode *tmp = buf; struct sllnode *tmp = buf;
interface->deferred_bytes -= (buf->size + size_t decr = (buf->size + sizeof(struct sllnode));
sizeof(struct sllnode));
assert(interface->deferred_bytes >= decr);
interface->deferred_bytes -= decr;
buf = buf->next; buf = buf->next;
free(tmp); free(tmp);
interface->deferred_send = buf; interface->deferred_send = buf;
...@@ -709,7 +713,11 @@ int interfacePrintWithFD(int fd, char *buffer, size_t buflen) ...@@ -709,7 +713,11 @@ int interfacePrintWithFD(int fd, char *buffer, size_t buflen)
interface = interfaces + i; interface = interfaces + i;
while (buflen > 0 && !interface->expired) { while (buflen > 0 && !interface->expired) {
size_t left = interface->send_buf_size - interface->send_buf_used; size_t left;
assert(interface->send_buf_size >= interface->send_buf_used);
left = interface->send_buf_size - interface->send_buf_used;
copylen = buflen > left ? left : buflen; copylen = buflen > left ? left : buflen;
memcpy(interface->send_buf + interface->send_buf_used, buffer, memcpy(interface->send_buf + interface->send_buf_used, buffer,
copylen); copylen);
...@@ -737,11 +745,11 @@ static void printInterfaceOutBuffer(Interface * interface) ...@@ -737,11 +745,11 @@ static void printInterfaceOutBuffer(Interface * interface)
+ interface->send_buf_used; + interface->send_buf_used;
if (interface->deferred_bytes > if (interface->deferred_bytes >
interface_max_output_buffer_size) { interface_max_output_buffer_size) {
ERROR("interface %i: output buffer size (%li) is " ERROR("interface %i: output buffer size (%lu) is "
"larger than the max (%li)\n", "larger than the max (%lu)\n",
interface->num, interface->num,
(long)interface->deferred_bytes, (unsigned long)interface->deferred_bytes,
(long)interface_max_output_buffer_size); (unsigned long)interface_max_output_buffer_size);
/* cause interface to close */ /* cause interface to close */
interface->expired = 1; interface->expired = 1;
do { do {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment