diff --git a/src/store_memcached.c b/src/store_memcached.c index 981b8b10..07546f03 100644 --- a/src/store_memcached.c +++ b/src/store_memcached.c @@ -80,6 +80,11 @@ static int memcached_tile_read(struct storage_backend * store, const char *xmlco memcached_return_t rc; char * buf_raw; + if (m == NULL) { + snprintf(log_msg, 1024, "Failed to allocate memory for metatile header\n"); + return -1; + } + mask = METATILE - 1; meta_offset = (x & mask) * METATILE + (y & mask); @@ -235,7 +240,6 @@ static int memcached_metatile_expire(struct storage_backend * store, const char char * buf; size_t len; uint32_t flags; - uint64_t cas; memcached_return_t rc; //TODO: deal with options @@ -246,11 +250,11 @@ static int memcached_metatile_expire(struct storage_backend * store, const char return -1; } - //cas = memcached_result_cas(&rc); - ((struct stat_info *)buf)->expired = 1; - rc = memcached_cas(store->storage_ctx, meta_path, strlen(meta_path), buf, len, 0, flags, cas); + // memcached_get() does not return a CAS value, so a compare-and-swap cannot be used here + // (the previous code passed an uninitialised CAS token). Replace the existing item instead. + rc = memcached_replace(store->storage_ctx, meta_path, strlen(meta_path), buf, len, 0, flags); if (rc != MEMCACHED_SUCCESS) { free(buf); diff --git a/src/store_rados.c b/src/store_rados.c index c057a804..4b528114 100644 --- a/src/store_rados.c +++ b/src/store_rados.c @@ -134,6 +134,11 @@ static int rados_tile_read(struct storage_backend * store, const char *xmlconfig int err; char * buf_raw; + if (m == NULL) { + snprintf(log_msg, 1024, "Failed to allocate memory for metatile header\n"); + return -1; + } + mask = METATILE - 1; meta_offset = (x & mask) * METATILE + (y & mask); @@ -234,6 +239,11 @@ static int rados_metatile_write(struct storage_backend * store, const char *xmlc char * buf2 = malloc(sz2); int err; + if (buf2 == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "rados_metatile_write: failed to allocate memory for write buffer"); + return -1; + } + tile_stat.expired = 0; tile_stat.size = sz; tile_stat.mtime = time(NULL); @@ -352,7 +362,7 @@ struct storage_backend * init_storage_rados(const char * connection_string) int err; int i; - if (ctx == NULL) { + if (ctx == NULL || store == NULL) { free(ctx); free(store); return NULL; diff --git a/src/store_ro_composite.c b/src/store_ro_composite.c index 8faf4b4d..959cf056 100644 --- a/src/store_ro_composite.c +++ b/src/store_ro_composite.c @@ -202,8 +202,12 @@ static int ro_composite_metatile_expire(struct storage_backend * store, const ch static int ro_composite_close_storage(struct storage_backend * store) { struct ro_composite_ctx * ctx = (struct ro_composite_ctx *)(store->storage_ctx); + // close_storage() no longer frees the backend itself (#524), so free the + // two inner backends here, as the callers do for the outer one ctx->store_primary->close_storage(ctx->store_primary); + free(ctx->store_primary); ctx->store_secondary->close_storage(ctx->store_secondary); + free(ctx->store_secondary); if (ctx->cache.tile) { free(ctx->cache.tile); @@ -226,7 +230,7 @@ struct storage_backend * init_storage_ro_composite(const char * connection_strin return NULL; #else struct storage_backend * store = malloc(sizeof(struct storage_backend)); - struct ro_composite_ctx * ctx = malloc(sizeof(struct ro_composite_ctx)); + struct ro_composite_ctx * ctx = calloc(1, sizeof(struct ro_composite_ctx)); char * connection_string_primary; char * connection_string_secondary; char * tmp; @@ -248,7 +252,23 @@ struct storage_backend * init_storage_ro_composite(const char * connection_strin } connection_string_secondary = strstr(connection_string, "}{"); + + if (connection_string_secondary == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: malformed connection string, missing '}{' separator: %s", connection_string); + free(ctx); + free(store); + return NULL; + } + connection_string_primary = malloc(strlen(connection_string) - strlen("composite:{") - strlen(connection_string_secondary) + 1); + + if (connection_string_primary == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: failed to allocate memory for primary connection string"); + free(ctx); + free(store); + return NULL; + } + memcpy(connection_string_primary, connection_string + strlen("composite:{"), strlen(connection_string) - strlen("composite:{") - strlen(connection_string_secondary)); connection_string_primary[strlen(connection_string) - strlen("composite:{") - strlen(connection_string_secondary)] = 0; connection_string_secondary = strdup(connection_string_secondary + 2); @@ -258,30 +278,61 @@ struct storage_backend * init_storage_ro_composite(const char * connection_strin g_logger(G_LOG_LEVEL_DEBUG, "init_storage_ro_composite: Secondary storage backend: %s", connection_string_secondary); tmp = strstr(connection_string_primary, ","); + + if (tmp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: malformed primary connection string, missing ',' separator: %s", connection_string_primary); + free(connection_string_primary); + free(connection_string_secondary); + free(ctx); + free(store); + return NULL; + } + memcpy(ctx->xmlconfig_primary, connection_string_primary, tmp - connection_string_primary); ctx->xmlconfig_primary[tmp - connection_string_primary] = 0; ctx->store_primary = init_storage_backend(tmp + 1); if (ctx->store_primary == NULL) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: failed to initialise primary storage backend"); + free(connection_string_primary); + free(connection_string_secondary); free(ctx); free(store); return NULL; } tmp = strstr(connection_string_secondary, ","); + + if (tmp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: malformed secondary connection string, missing ',' separator: %s", connection_string_secondary); + free(connection_string_primary); + free(connection_string_secondary); + ctx->store_primary->close_storage(ctx->store_primary); + free(ctx->store_primary); + free(ctx); + free(store); + return NULL; + } + memcpy(ctx->xmlconfig_secondary, connection_string_secondary, tmp - connection_string_secondary); ctx->xmlconfig_secondary[tmp - connection_string_secondary] = 0; ctx->store_secondary = init_storage_backend(tmp + 1); if (ctx->store_secondary == NULL) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: failed to initialise secondary storage backend"); + free(connection_string_primary); + free(connection_string_secondary); ctx->store_primary->close_storage(ctx->store_primary); + free(ctx->store_primary); free(ctx); free(store); return NULL; } + // The backends keep their own copies of what they need from these + free(connection_string_primary); + free(connection_string_secondary); + ctx->render_size = 256; store->storage_ctx = ctx; diff --git a/src/store_ro_http_proxy.c b/src/store_ro_http_proxy.c index 69925fb7..61b0e098 100644 --- a/src/store_ro_http_proxy.c +++ b/src/store_ro_http_proxy.c @@ -116,6 +116,7 @@ static int ro_http_proxy_tile_retrieve(struct storage_backend * store, const cha if (res != CURLE_OK) { g_logger(G_LOG_LEVEL_ERROR, "ro_http_proxy_tile_fetch: failed to retrieve file: %s", curl_easy_strerror(res)); + free(chunk.memory); ctx->cache.x = -1; ctx->cache.y = -1; ctx->cache.z = -1; @@ -126,6 +127,7 @@ static int ro_http_proxy_tile_retrieve(struct storage_backend * store, const cha if (res != CURLE_OK) { g_logger(G_LOG_LEVEL_ERROR, "ro_http_proxy_tile_fetch: failed to retrieve HTTP code: %s", curl_easy_strerror(res)); + free(chunk.memory); ctx->cache.x = -1; ctx->cache.y = -1; ctx->cache.z = -1; @@ -150,19 +152,31 @@ static int ro_http_proxy_tile_retrieve(struct storage_backend * store, const cha case 404: { if (ctx->cache.tile != NULL) { free(ctx->cache.tile); + ctx->cache.tile = NULL; } + free(chunk.memory); ctx->cache.st_stat.size = -1; ctx->cache.st_stat.expired = 0; break; } - } + default: { + // Do not cache anything for unexpected responses, otherwise the previously + // cached tile would be served for these coordinates + g_logger(G_LOG_LEVEL_ERROR, "ro_http_proxy_tile_fetch: unexpected HTTP response code %li", httpCode); + free(chunk.memory); + ctx->cache.x = -1; + ctx->cache.y = -1; + ctx->cache.z = -1; + return -1; + } + } ctx->cache.x = x; ctx->cache.y = y; ctx->cache.z = z; - strcpy(ctx->cache.xmlname, xmlconfig); + snprintf(ctx->cache.xmlname, sizeof(ctx->cache.xmlname), "%s", xmlconfig); return 1; } } @@ -172,7 +186,12 @@ static int ro_http_proxy_tile_read(struct storage_backend * store, const char *x struct ro_http_proxy_ctx * ctx = (struct ro_http_proxy_ctx *)(store->storage_ctx); if (ro_http_proxy_tile_retrieve(store, xmlconfig, options, x, y, z) > 0) { - if (ctx->cache.st_stat.size > sz) { + if (ctx->cache.st_stat.size < 0) { + // Cached 404 + return -1; + } + + if ((size_t)ctx->cache.st_stat.size > sz) { g_logger(G_LOG_LEVEL_ERROR, "ro_http_proxy_tile_read: size was too big, overrun %lu %li", sz, ctx->cache.st_stat.size); return -1; } @@ -299,6 +318,7 @@ struct storage_backend * init_storage_ro_http_proxy(const char * connection_stri if (res != CURLE_OK) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_http_proxy: failed to initialise global curl: %s", curl_easy_strerror(res)); + free(ctx->baseurl); free(ctx); free(store); return NULL; @@ -308,6 +328,7 @@ struct storage_backend * init_storage_ro_http_proxy(const char * connection_stri if (!ctx->ctx) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_http_proxy: failed to initialise curl"); + free(ctx->baseurl); free(ctx); free(store); return NULL; diff --git a/tests/gen_tile_test.cpp b/tests/gen_tile_test.cpp index cff851b4..d5c612eb 100644 --- a/tests/gen_tile_test.cpp +++ b/tests/gen_tile_test.cpp @@ -1188,20 +1188,35 @@ TEST_CASE("rados storage-backend", "RADOS Tile storage backend") TEST_CASE("ro_composite storage-backend", "RO Composite Tile storage backend") { -#ifndef HAVE_CAIRO - SECTION("storage/initialise", "should return NULL") { - int found; - std::string err_log_lines, out_log_lines; + int found; + std::string err_log_lines, out_log_lines; + std::string tile_dir = P_tmpdir; + std::string valid = "composite:{default," + tile_dir + "}{default," + tile_dir + "}"; - start_capture(); - REQUIRE(init_storage_backend("composite:{") == NULL); - std::tie(err_log_lines, out_log_lines) = end_capture(); + start_capture(); + struct storage_backend *store = init_storage_backend(valid.c_str()); + std::tie(err_log_lines, out_log_lines) = end_capture(); + if (store == NULL) { + // Composite support depends on HAVE_CAIRO as seen by the C sources, which can differ + // from what this C++ file sees (e.g. Autotools builds), so check at run time found = err_log_lines.find("init_storage_ro_coposite: Support for compositing storage has not been compiled into this program"); REQUIRE(found > -1); + return; } -#endif + // Closing must free the inner backends and not touch the (never used) tile cache + store->close_storage(store); + free(store); + + start_capture(); + // Missing "}{" separator + REQUIRE(init_storage_backend(("composite:{default," + tile_dir + "}").c_str()) == NULL); + // Primary backend fails to initialise + REQUIRE(init_storage_backend(("composite:{default,/does/not/exist}{default," + tile_dir + "}").c_str()) == NULL); + // Secondary backend fails to initialise + REQUIRE(init_storage_backend(("composite:{default," + tile_dir + "}{default,/does/not/exist}").c_str()) == NULL); + end_capture(); } TEST_CASE("ro_http_proxy storage-backend", "RO HTTP Proxy Tile storage backend")