From 73e45ff0246c2624b890bbbec0367eb1c450eec3 Mon Sep 17 00:00:00 2001 From: Vincent Sanders Date: Wed, 24 Jun 2015 10:31:13 +0100 Subject: Fix error reporting from fetch_start Any fetch start error was being reported as "out of memory" which was clearly insufficient. Foe example bad urls (reported was file:// with a missing /) were causing a warn_user with out of memory. This change now at least causes a "bad url" message. --- content/fetch.c | 18 ++++++++++----- content/fetch.h | 25 +++++++++++++++------ content/hlcache.c | 17 +++++++------- content/llcache.c | 66 +++++++++++++++++++++++++++++++++---------------------- 4 files changed, 79 insertions(+), 47 deletions(-) diff --git a/content/fetch.c b/content/fetch.c index 9e0bb255e..2307676d3 100644 --- a/content/fetch.c +++ b/content/fetch.c @@ -437,7 +437,7 @@ nserror fetcher_fdset(fd_set *read_fd_set, } /* exported interface documented in content/fetch.h */ -struct fetch * +nserror fetch_start(nsurl *url, nsurl *referer, fetch_callback callback, @@ -447,7 +447,8 @@ fetch_start(nsurl *url, const struct fetch_multipart_data *post_multipart, bool verifiable, bool downgrade_tls, - const char *headers[]) + const char *headers[], + struct fetch **fetch_out) { struct fetch *fetch; lwc_string *scheme; @@ -455,7 +456,7 @@ fetch_start(nsurl *url, fetch = malloc(sizeof (*fetch)); if (fetch == NULL) { - return NULL; + return NSERROR_NOMEM; } /* The URL we're fetching must have a scheme */ @@ -467,7 +468,7 @@ fetch_start(nsurl *url, if (fetch->fetcherd == -1) { lwc_string_unref(scheme); free(fetch); - return NULL; + return NSERROR_NO_FETCH_HANDLER; } FETCH_LOG("fetch %p, url '%s'", fetch, nsurl_access(url)); @@ -547,7 +548,11 @@ fetch_start(nsurl *url, free(fetch); - return NULL; + + /** \todo The fetchers setup should return nserror and that be + * passed back rather than assuming a bad url + */ + return NSERROR_BAD_URL; } /* Rah, got it, so ref the fetcher. */ @@ -563,7 +568,8 @@ fetch_start(nsurl *url, guit->browser->schedule(10, fetcher_poll, NULL); } - return fetch; + *fetch_out = fetch; + return NSERROR_OK; } /* exported interface documented in content/fetch.h */ diff --git a/content/fetch.h b/content/fetch.h index 37539ef2b..529a800fa 100644 --- a/content/fetch.h +++ b/content/fetch.h @@ -113,13 +113,24 @@ typedef void (*fetch_callback)(const fetch_msg *msg, void *p); * data contains an error message. FETCH_REDIRECT may replace the FETCH_HEADER, * FETCH_DATA, FETCH_FINISHED sequence if the server sends a replacement URL. * - */ -struct fetch *fetch_start(nsurl *url, nsurl *referer, - fetch_callback callback, - void *p, bool only_2xx, const char *post_urlenc, - const struct fetch_multipart_data *post_multipart, - bool verifiable, bool downgrade_tls, - const char *headers[]); + * \param url URL to fetch + * \param referer + * \param callback + * \param p + * \param only_2xx + * \param post_urlenc + * \param post_multipart + * \param verifiable + * \param downgrade_tls + * \param headers + * \param fetch_out ponter to recive new fetch object. + * \return NSERROR_OK and fetch_out updated else appropriate error code + */ +nserror fetch_start(nsurl *url, nsurl *referer, fetch_callback callback, + void *p, bool only_2xx, const char *post_urlenc, + const struct fetch_multipart_data *post_multipart, + bool verifiable, bool downgrade_tls, + const char *headers[], struct fetch **fetch_out); /** * Abort a fetch. diff --git a/content/hlcache.c b/content/hlcache.c index 388c59a94..85567f5a4 100644 --- a/content/hlcache.c +++ b/content/hlcache.c @@ -631,8 +631,9 @@ nserror hlcache_handle_retrieve(nsurl *url, uint32_t flags, assert(cb != NULL); ctx = calloc(1, sizeof(hlcache_retrieval_ctx)); - if (ctx == NULL) + if (ctx == NULL) { return NSERROR_NOMEM; + } ctx->handle = calloc(1, sizeof(hlcache_handle)); if (ctx->handle == NULL) { @@ -662,17 +663,17 @@ nserror hlcache_handle_retrieve(nsurl *url, uint32_t flags, hlcache_llcache_callback, ctx, &ctx->llcache); if (error != NSERROR_OK) { + /* error retriving handle so free context and return error */ free((char *) ctx->child.charset); free(ctx->handle); free(ctx); - return error; - } - - RING_INSERT(hlcache->retrieval_ctx_ring, ctx); - - *result = ctx->handle; + } else { + /* successfuly started fetch so add new context to list */ + RING_INSERT(hlcache->retrieval_ctx_ring, ctx); - return NSERROR_OK; + *result = ctx->handle; + } + return error; } /* See hlcache.h for documentation */ diff --git a/content/llcache.c b/content/llcache.c index 3eb34ab5b..851dbbb5f 100644 --- a/content/llcache.c +++ b/content/llcache.c @@ -299,8 +299,9 @@ static nserror llcache_object_user_new(llcache_handle_callback cb, void *pw, llcache_object_user *u; h = calloc(1, sizeof(llcache_handle)); - if (h == NULL) + if (h == NULL) { return NSERROR_NOMEM; + } u = calloc(1, sizeof(llcache_object_user)); if (u == NULL) { @@ -772,9 +773,13 @@ static nserror llcache_fetch_process_header(llcache_object *object, /** * (Re)fetch an object * + * sets up headers and attempts to start an actual fetch from the + * fetchers system updating the llcache object with the new fetch on + * sucessful start. + * * \pre The fetch parameters in object->fetch must be populated * - * \param object Object to refetch + * \param object Object to refetch * \return NSERROR_OK on success, appropriate error otherwise */ static nserror llcache_object_refetch(llcache_object *object) @@ -783,6 +788,7 @@ static nserror llcache_object_refetch(llcache_object *object) struct fetch_multipart_data *multipart = NULL; char **headers = NULL; int header_idx = 0; + nserror res; if (object->fetch.post != NULL) { if (object->fetch.post->type == LLCACHE_POST_URL_ENCODED) { @@ -794,8 +800,9 @@ static nserror llcache_object_refetch(llcache_object *object) /* Generate cache-control headers */ headers = malloc(3 * sizeof(char *)); - if (headers == NULL) + if (headers == NULL) { return NSERROR_NOMEM; + } if (object->cache.etag != NULL) { const size_t len = SLEN("If-None-Match: ") + @@ -843,24 +850,25 @@ static nserror llcache_object_refetch(llcache_object *object) LLCACHE_LOG("Refetching %p", object); /* Kick off fetch */ - object->fetch.fetch = fetch_start(object->url, object->fetch.referer, - llcache_fetch_callback, object, - object->fetch.flags & LLCACHE_RETRIEVE_NO_ERROR_PAGES, - urlenc, multipart, - object->fetch.flags & LLCACHE_RETRIEVE_VERIFIABLE, - object->fetch.tried_with_tls_downgrade, - (const char **) headers); + res = fetch_start(object->url, + object->fetch.referer, + llcache_fetch_callback, + object, + object->fetch.flags & LLCACHE_RETRIEVE_NO_ERROR_PAGES, + urlenc, + multipart, + object->fetch.flags & LLCACHE_RETRIEVE_VERIFIABLE, + object->fetch.tried_with_tls_downgrade, + (const char **)headers, + &object->fetch.fetch); /* Clean up cache-control headers */ - while (--header_idx >= 0) + while (--header_idx >= 0) { free(headers[header_idx]); + } free(headers); - /* Did we succeed in creating a fetch? */ - if (object->fetch.fetch == NULL) - return NSERROR_NOMEM; - - return NSERROR_OK; + return res; } /** @@ -1708,9 +1716,13 @@ llcache_object_retrieve_from_cache(nsurl *url, * \param result Pointer to location to recieve retrieved object * \return NSERROR_OK on success, appropriate error otherwise */ -static nserror llcache_object_retrieve(nsurl *url, uint32_t flags, - nsurl *referer, const llcache_post_data *post, - uint32_t redirect_count, llcache_object **result) +static nserror +llcache_object_retrieve(nsurl *url, + uint32_t flags, + nsurl *referer, + const llcache_post_data *post, + uint32_t redirect_count, + llcache_object **result) { nserror error; llcache_object *obj; @@ -2491,7 +2503,7 @@ static void llcache_persist(void *p) total_elapsed += elapsed; total_bandwidth = (total_written * 1000) / total_elapsed; - LLCACHE_LOG("Wrote %d bytes in %dms bw:%d %s", + LLCACHE_LOG("Wrote %zd bytes in %lums bw:%lu %s", written, elapsed, (written * 1000) / elapsed, nsurl_access(lst[idx]->url) ); @@ -2557,7 +2569,7 @@ static void llcache_persist(void *p) llcache->total_written += total_written; llcache->total_elapsed += total_elapsed; - LLCACHE_LOG("writeout size:%d time:%d bandwidth:%dbytes/s", + LLCACHE_LOG("writeout size:%zd time:%lu bandwidth:%lubytes/s", total_written, total_elapsed, total_bandwidth); LLCACHE_LOG("Rescheduling writeout in %dms", next); @@ -3208,7 +3220,7 @@ void llcache_clean(bool purge) llcache_size -= object->source_len; - LLCACHE_LOG("Freeing source data for %p len:%d", + LLCACHE_LOG("Freeing source data for %p len:%zd", object, object->source_len); } @@ -3228,7 +3240,7 @@ void llcache_clean(bool purge) (object->fetch.outstanding_query == false) && (object->store_state == LLCACHE_STATE_DISC) && (object->source_data == NULL)) { - LLCACHE_LOG("discarding backed object len:%d age:%d (%p) %s", + LLCACHE_LOG("discarding backed object len:%zd age:%d (%p) %s", object->source_len, time(NULL) - object->last_used, object, @@ -3258,7 +3270,7 @@ void llcache_clean(bool purge) (object->fetch.fetch == NULL) && (object->fetch.outstanding_query == false) && (object->store_state == LLCACHE_STATE_RAM)) { - LLCACHE_LOG("discarding fresh object len:%d age:%d (%p) %s", + LLCACHE_LOG("discarding fresh object len:%zd age:%d (%p) %s", object->source_len, time(NULL) - object->last_used, object, @@ -3413,13 +3425,15 @@ nserror llcache_handle_retrieve(nsurl *url, uint32_t flags, llcache_object *object; /* Can we fetch this URL at all? */ - if (fetch_can_fetch(url) == false) + if (fetch_can_fetch(url) == false) { return NSERROR_NO_FETCH_HANDLER; + } /* Create a new object user */ error = llcache_object_user_new(cb, pw, &user); - if (error != NSERROR_OK) + if (error != NSERROR_OK) { return error; + } /* Retrieve a suitable object from the cache, * creating a new one if needed. */ -- cgit v1.2.3