Permalink
Browse files

Handle memory allocation issues better.

Return 503 for memory issues during HTTP parsing.
  • Loading branch information...
1 parent 7069389 commit 9b62593f53af8a73132d864f98d148ffe5b5554a @nicolasff committed Feb 29, 2012
Showing with 39 additions and 17 deletions.
  1. +14 −9 client.c
  2. +9 −4 client.h
  3. +16 −4 worker.c
View
@@ -14,12 +14,14 @@
#include <hiredis/hiredis.h>
#include <hiredis/async.h>
+#define CHECK_ALLOC(c, ptr) if(!(ptr)) { c->failed_alloc = 1; return -1;}
+
static int
http_client_on_url(struct http_parser *p, const char *at, size_t sz) {
struct http_client *c = p->data;
- c->path = realloc(c->path, c->path_sz + sz + 1);
+ CHECK_ALLOC(c, c->path = realloc(c->path, c->path_sz + sz + 1));
memcpy(c->path + c->path_sz, at, sz);
c->path_sz += sz;
c->path[c->path_sz] = 0;
@@ -40,7 +42,7 @@ http_client_on_body(struct http_parser *p, const char *at, size_t sz) {
int
http_client_add_to_body(struct http_client *c, const char *at, size_t sz) {
- c->body = realloc(c->body, c->body_sz + sz + 1);
+ CHECK_ALLOC(c, c->body = realloc(c->body, c->body_sz + sz + 1));
memcpy(c->body + c->body_sz, at, sz);
c->body_sz += sz;
c->body[c->body_sz] = 0;
@@ -57,13 +59,13 @@ http_client_on_header_name(struct http_parser *p, const char *at, size_t sz) {
/* if we're not adding to the same header name as last time, realloc to add one field. */
if(c->last_cb != LAST_CB_KEY) {
n = ++c->header_count;
- c->headers = realloc(c->headers, n * sizeof(struct http_header));
+ CHECK_ALLOC(c, c->headers = realloc(c->headers, n * sizeof(struct http_header)));
memset(&c->headers[n-1], 0, sizeof(struct http_header));
}
/* Add data to the current header name. */
- c->headers[n-1].key = realloc(c->headers[n-1].key,
- c->headers[n-1].key_sz + sz + 1);
+ CHECK_ALLOC(c, c->headers[n-1].key = realloc(c->headers[n-1].key,
+ c->headers[n-1].key_sz + sz + 1));
memcpy(c->headers[n-1].key + c->headers[n-1].key_sz, at, sz);
c->headers[n-1].key_sz += sz;
c->headers[n-1].key[c->headers[n-1].key_sz] = 0;
@@ -146,8 +148,8 @@ http_client_on_header_value(struct http_parser *p, const char *at, size_t sz) {
size_t n = c->header_count;
/* Add data to the current header value. */
- c->headers[n-1].val = realloc(c->headers[n-1].val,
- c->headers[n-1].val_sz + sz + 1);
+ CHECK_ALLOC(c, c->headers[n-1].val = realloc(c->headers[n-1].val,
+ c->headers[n-1].val_sz + sz + 1));
memcpy(c->headers[n-1].val + c->headers[n-1].val_sz, at, sz);
c->headers[n-1].val_sz += sz;
c->headers[n-1].val[c->headers[n-1].val_sz] = 0;
@@ -297,11 +299,14 @@ http_client_read(struct http_client *c) {
close(c->fd);
http_client_free(c);
- return -1;
+ return (int)CLIENT_DISCONNECTED;
}
/* save what we've just read */
c->buffer = realloc(c->buffer, c->sz + ret);
+ if(!c->buffer) {
+ return (int)CLIENT_OOM;
+ }
memcpy(c->buffer + c->sz, buffer, ret);
c->sz += ret;
@@ -319,7 +324,7 @@ http_client_remove_data(struct http_client *c, size_t sz) {
return -1;
/* replace buffer */
- buffer = malloc(c->sz - sz);
+ CHECK_ALLOC(c, buffer = malloc(c->sz - sz));
memcpy(buffer, c->buffer + sz, c->sz - sz);
free(c->buffer);
c->buffer = buffer;
View
@@ -15,6 +15,10 @@ typedef enum {
LAST_CB_KEY = 1,
LAST_CB_VAL = 2} last_cb_t;
+typedef enum {
+ CLIENT_DISCONNECTED = -1,
+ CLIENT_OOM = -2} client_error_t;
+
struct http_client {
int fd;
@@ -34,10 +38,11 @@ struct http_client {
last_cb_t last_cb;
/* various flags. */
- int keep_alive;
- int broken;
- int is_websocket;
- int http_version;
+ char keep_alive;
+ char broken;
+ char is_websocket;
+ char http_version;
+ char failed_alloc;
/* HTTP data */
char *path;
View
@@ -44,8 +44,13 @@ worker_can_read(int fd, short event, void *p) {
ret = http_client_read(c);
if(ret <= 0) {
- /* client disconnected */
- return;
+ if((client_error_t)ret == CLIENT_DISCONNECTED) {
+ return;
+ } else if (c->failed_alloc || (client_error_t)ret == CLIENT_OOM) {
+ slog(c->w->s, WEBDIS_DEBUG, "503", 3);
+ http_send_error(c, 503, "Service Unavailable");
+ return;
+ }
}
if(c->is_websocket) {
@@ -55,7 +60,10 @@ worker_can_read(int fd, short event, void *p) {
/* run parser */
nparsed = http_client_execute(c);
- if(c->is_websocket) {
+ if(c->failed_alloc) {
+ slog(c->w->s, WEBDIS_DEBUG, "503", 3);
+ http_send_error(c, 503, "Service Unavailable");
+ } else if(c->is_websocket) {
/* we need to use the remaining (unparsed) data as the body. */
if(nparsed < ret) {
http_client_add_to_body(c, c->buffer + nparsed + 1, c->sz - nparsed - 1);
@@ -66,8 +74,12 @@ worker_can_read(int fd, short event, void *p) {
free(c->buffer);
c->buffer = NULL;
c->sz = 0;
- } else if(nparsed != ret || c->request_sz > c->s->cfg->http_max_request_size) {
+ } else if(nparsed != ret) {
+ slog(c->w->s, WEBDIS_DEBUG, "400", 3);
http_send_error(c, 400, "Bad Request");
+ } else if(c->request_sz > c->s->cfg->http_max_request_size) {
+ slog(c->w->s, WEBDIS_DEBUG, "413", 3);
+ http_send_error(c, 413, "Request Entity Too Large");
}
}

0 comments on commit 9b62593

Please sign in to comment.