From f169153956d9aa578b96d50cd574c47ee8fd7511 Mon Sep 17 00:00:00 2001 From: Niels Provos Date: Thu, 19 Nov 2009 23:08:50 +0000 Subject: [PATCH] Remove most calls to event_err() in http and deal with memory errors instead svn:r1555 --- ChangeLog | 1 + buffer.c | 2 ++ http.c | 80 ++++++++++++++++++++++++++++++++----------- include/event2/http.h | 11 +++--- 4 files changed, 69 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7b2f34ea..c83101c8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -53,6 +53,7 @@ Changes in 2.0.3-alpha: o Numerous other bugfixes. o On FreeBSD and other OSes, connect can return ECONREFUSED immediately; instead of failing the function call, pretend with faileld in the callback. o Fix a race condition in the pthreads test case; found by Nick Mathewson + o Remove most calls to event_err() in http and deal with memory errors instead Changes in 2.0.2-alpha: o Add a new flag to bufferevents to make all callbacks automatically deferred. diff --git a/buffer.c b/buffer.c index 69810350..633f2d21 100644 --- a/buffer.c +++ b/buffer.c @@ -265,6 +265,8 @@ evbuffer_new(void) struct evbuffer *buffer; buffer = mm_calloc(1, sizeof(struct evbuffer)); + if (buffer == NULL) + return (NULL); TAILQ_INIT(&buffer->callbacks); buffer->refcnt = 1; diff --git a/http.c b/http.c index acbd0b57..26e81ca2 100644 --- a/http.c +++ b/http.c @@ -244,8 +244,10 @@ evhttp_htmlescape(const char *html) new_size += strlen(html_replace(html[i], scratch_space)); p = escaped_html = mm_malloc(new_size + 1); - if (escaped_html == NULL) - event_err(1, "%s: malloc(%d)", __func__, new_size + 1); + if (escaped_html == NULL) { + event_warn("%s: malloc(%d)", __func__, new_size + 1); + return (NULL); + } for (i = 0; i < old_size; ++i) { const char *replaced = html_replace(html[i], scratch_space); /* this is length checked */ @@ -1254,8 +1256,10 @@ evhttp_parse_response_line(struct evhttp_request *req, char *line) return (-1); } - if ((req->response_code_line = mm_strdup(readable)) == NULL) - event_err(1, "%s: strdup", __func__); + if ((req->response_code_line = mm_strdup(readable)) == NULL) { + event_warn("%s: strdup", __func__); + return (-1); + } return (0); } @@ -1310,7 +1314,7 @@ evhttp_parse_request_line(struct evhttp_request *req, char *line) } if ((req->uri = mm_strdup(uri)) == NULL) { - event_debug(("%s: evhttp_decode_uri", __func__)); + event_debug(("%s: mm_strdup", __func__)); return (-1); } @@ -1889,8 +1893,11 @@ evhttp_make_request(struct evhttp_connection *evcon, req->type = type; if (req->uri != NULL) mm_free(req->uri); - if ((req->uri = mm_strdup(uri)) == NULL) - event_err(1, "%s: strdup", __func__); + if ((req->uri = mm_strdup(uri)) == NULL) { + event_warn("%s: strdup", __func__); + evhttp_request_free(req); + return (-1); + } /* Set the protocol version if it is not supplied */ if (!req->major && !req->minor) { @@ -2001,6 +2008,11 @@ evhttp_send_error(struct evhttp_request *req, int error, const char *reason) "\n" struct evbuffer *buf = evbuffer_new(); + if (buf == NULL) { + /* if we cannot allocate memory; we just drop the connection */ + evhttp_connection_free(req->evcon); + return; + } /* close the connection on error */ evhttp_add_header(req->output_headers, "Connection", "close"); @@ -2163,6 +2175,9 @@ evhttp_encode_uri(const char *uri) struct evbuffer *buf = evbuffer_new(); char *p; + if (buf == NULL) + return (NULL); + for (p = (char *)uri; *p != '\0'; p++) { if (uri_chars[(unsigned char)(*p)]) { evbuffer_add(buf, p, 1); @@ -2216,9 +2231,11 @@ evhttp_decode_uri(const char *uri) { char *ret; - if ((ret = mm_malloc(strlen(uri) + 1)) == NULL) - event_err(1, "%s: malloc(%lu)", __func__, + if ((ret = mm_malloc(strlen(uri) + 1)) == NULL) { + event_warn("%s: malloc(%lu)", __func__, (unsigned long)(strlen(uri) + 1)); + return (NULL); + } evhttp_decode_uri_internal(uri, strlen(uri), ret, 0 /*always_decode_plus*/); @@ -2244,8 +2261,11 @@ evhttp_parse_query(const char *uri, struct evkeyvalq *headers) if (strchr(uri, '?') == NULL) return; - if ((line = mm_strdup(uri)) == NULL) - event_err(1, "%s: strdup", __func__); + if ((line = mm_strdup(uri)) == NULL) { + /* TODO(niels): does this function need to return -1 */ + event_warn("%s: strdup", __func__); + return; + } argument = line; @@ -2263,8 +2283,11 @@ evhttp_parse_query(const char *uri, struct evkeyvalq *headers) if (value == NULL) goto error; - if ((decoded_value = mm_malloc(strlen(value) + 1)) == NULL) - event_err(1, "%s: mm_malloc", __func__); + if ((decoded_value = mm_malloc(strlen(value) + 1)) == NULL) { + /* TODO(niels): do we need to return -1 here? */ + event_warn("%s: mm_malloc", __func__); + break; + } evhttp_decode_uri_internal(value, strlen(value), decoded_value, 1 /*always_decode_plus*/); event_debug(("Query Param: %s -> %s\n", key, decoded_value)); @@ -2383,8 +2406,19 @@ evhttp_handle_request(struct evhttp_request *req, void *arg) "

The requested URL %s was not found on this server.

"\ "\n" - char *escaped_html = evhttp_htmlescape(req->uri); - struct evbuffer *buf = evbuffer_new(); + char *escaped_html; + struct evbuffer *buf; + + if ((escaped_html = evhttp_htmlescape(req->uri)) == NULL) { + evhttp_connection_free(req->evcon); + return; + } + + if ((buf = evbuffer_new()) == NULL) { + mm_free(escaped_html); + evhttp_connection_free(req->evcon); + return; + } evhttp_response_code(req, HTTP_NOTFOUND, "Not Found"); @@ -2667,8 +2701,10 @@ evhttp_set_cb(struct evhttp *http, const char *uri, return (-1); } - if ((http_cb = mm_calloc(1, sizeof(struct evhttp_cb))) == NULL) - event_err(1, "%s: calloc", __func__); + if ((http_cb = mm_calloc(1, sizeof(struct evhttp_cb))) == NULL) { + event_warn("%s: calloc", __func__); + return (-2); + } http_cb->what = mm_strdup(uri); http_cb->cb = cb; @@ -2897,6 +2933,13 @@ evhttp_associate_new_request_with_connection(struct evhttp_connection *evcon) if ((req = evhttp_request_new(evhttp_handle_request, http)) == NULL) return (-1); + if ((req->remote_host = mm_strdup(evcon->address)) == NULL) { + event_warn("%s: strdup", __func__); + evhttp_request_free(req); + return (-1); + } + req->remote_port = evcon->port; + req->evcon = evcon; /* the request ends up owning the connection */ req->flags |= EVHTTP_REQ_OWN_CONNECTION; @@ -2904,9 +2947,6 @@ evhttp_associate_new_request_with_connection(struct evhttp_connection *evcon) req->kind = EVHTTP_REQUEST; - if ((req->remote_host = mm_strdup(evcon->address)) == NULL) - event_err(1, "%s: strdup", __func__); - req->remote_port = evcon->port; evhttp_start_read(evcon); diff --git a/include/event2/http.h b/include/event2/http.h index ade9e20b..328eefae 100644 --- a/include/event2/http.h +++ b/include/event2/http.h @@ -184,7 +184,7 @@ void evhttp_set_max_body_size(struct evhttp* http, ev_ssize_t max_body_size); @param path the path for which to invoke the callback @param cb the callback function that gets invoked on requesting path @param cb_arg an additional context argument for the callback - @return 0 on success, -1 if the callback existed already + @return 0 on success, -1 if the callback existed already, -2 on failure */ int evhttp_set_cb(struct evhttp *http, const char *path, void (*cb)(struct evhttp_request *, void *), void *cb_arg); @@ -407,7 +407,8 @@ void evhttp_connection_get_peer(struct evhttp_connection *evcon, /** Make an HTTP request over the specified connection. - The connection gets ownership of the request. + The connection gets ownership of the request. On failure, the + request object is no longer valid as it has been freed. @param evcon the evhttp_connection object over which to send the request @param req the previously created and configured request object @@ -498,7 +499,7 @@ void evhttp_clear_headers(struct evkeyvalq *headers); The returned string must be freed by the caller. @param uri an unencoded URI - @return a newly allocated URI-encoded string + @return a newly allocated URI-encoded string or NULL on failure */ char *evhttp_encode_uri(const char *uri); @@ -509,7 +510,7 @@ char *evhttp_encode_uri(const char *uri); The returned string must be freed by the caller. @param uri an encoded URI - @return a newly allocated unencoded URI + @return a newly allocated unencoded URI or NULL on failure */ char *evhttp_decode_uri(const char *uri); @@ -541,7 +542,7 @@ void evhttp_parse_query(const char *uri, struct evkeyvalq *headers); * The returned string needs to be freed by the caller. * * @param html an unescaped HTML string - * @return an escaped HTML string + * @return an escaped HTML string or NULL on error */ char *evhttp_htmlescape(const char *html);