From 8d3c2b3d303adaf2c9caa91356183d9b40cdfeab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Feb 2015 11:47:53 -0800 Subject: [PATCH 1/7] add libzip fix --- platform/default/uv_zip.c | 41 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/platform/default/uv_zip.c b/platform/default/uv_zip.c index b6d2c2ebe..7cb3c516d 100644 --- a/platform/default/uv_zip.c +++ b/platform/default/uv_zip.c @@ -6,7 +6,8 @@ void uv__zip_open_error(uv_zip_t *zip, int error) { - zip->result = -error; + assert(zip); + zip->result = -error; if (zip->message) { free((char *)zip->message); zip->message = NULL; @@ -17,16 +18,18 @@ void uv__zip_open_error(uv_zip_t *zip, int error) { } void uv__zip_store_error(uv_zip_t *zip, const char *message) { + assert(zip); if (zip->message) { free((char *)zip->message); zip->message = NULL; } - const unsigned long length = strlen(message); + const unsigned long length = strlen(message) + 1; zip->message = malloc(length); strncpy((char *)zip->message, message, length); } void uv__zip_error(uv_zip_t *zip) { + assert(zip); int error; zip_error_get(zip->archive, &error, NULL); zip->result = -error; @@ -34,6 +37,7 @@ void uv__zip_error(uv_zip_t *zip) { } void uv__zip_file_error(uv_zip_t *zip) { + assert(zip); int error; zip_file_error_get(zip->file, &error, NULL); zip->result = -error; @@ -42,6 +46,7 @@ void uv__zip_file_error(uv_zip_t *zip) { void uv__zip_work_open(uv_work_t *req) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); assert(!zip->archive); int error; @@ -55,6 +60,7 @@ void uv__zip_work_open(uv_work_t *req) { void uv__zip_work_fdopen(uv_work_t *req) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); assert(!zip->archive); // extract the fd @@ -73,6 +79,7 @@ void uv__zip_work_fdopen(uv_work_t *req) { void uv__zip_work_stat(uv_work_t *req) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); assert(zip->archive); if (!zip->stat) { zip->stat = malloc(sizeof(struct zip_stat)); @@ -85,6 +92,7 @@ void uv__zip_work_stat(uv_work_t *req) { void uv__zip_work_fopen(uv_work_t *req) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); assert(zip->archive); zip->file = zip_fopen(zip->archive, zip->path, zip->flags); if (!zip->file) { @@ -94,6 +102,7 @@ void uv__zip_work_fopen(uv_work_t *req) { void uv__zip_work_fread(uv_work_t *req) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); assert(zip->file); assert(zip->buf); zip->result = zip_fread(zip->file, zip->buf->base, zip->buf->len); @@ -104,6 +113,7 @@ void uv__zip_work_fread(uv_work_t *req) { void uv__zip_work_fclose(uv_work_t *req) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); assert(zip->file); zip->result = zip_fclose(zip->file); if (zip->result != 0) { @@ -121,12 +131,14 @@ void uv__zip_work_discard(uv_work_t *req) { void uv__zip_after_work(uv_work_t *req, int status) { uv_zip_t *zip = (uv_zip_t *)req->data; + assert(zip); if (zip->cb) { zip->cb(zip); } } void uv_zip_init(uv_zip_t *zip) { + assert(zip); zip->work.data = zip; zip->message = NULL; zip->stat = NULL; @@ -134,6 +146,7 @@ void uv_zip_init(uv_zip_t *zip) { } void uv_zip_cleanup(uv_zip_t *zip) { + assert(zip); zip->data = NULL; zip->flags = 0; zip->result = 0; @@ -155,6 +168,10 @@ void uv_zip_cleanup(uv_zip_t *zip) { } int uv_zip_open(uv_loop_t* loop, uv_zip_t *zip, const char *path, zip_flags_t flags, uv_zip_cb cb) { + assert(loop); + assert(zip); + assert(path); + assert(strlen(path)); zip->path = path; zip->flags = flags; zip->cb = cb; @@ -162,6 +179,9 @@ int uv_zip_open(uv_loop_t* loop, uv_zip_t *zip, const char *path, zip_flags_t fl } int uv_zip_fdopen(uv_loop_t* loop, uv_zip_t *zip, uv_file fd, int flags, uv_zip_cb cb) { + assert(loop); + assert(zip); + assert(fd); zip->path = malloc(sizeof(uv_file)); *(uv_file *)zip->path = fd; zip->flags = flags; @@ -170,6 +190,10 @@ int uv_zip_fdopen(uv_loop_t* loop, uv_zip_t *zip, uv_file fd, int flags, uv_zip_ } int uv_zip_stat(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t flags, uv_zip_cb cb) { + assert(loop); + assert(zip); + assert(fname); + assert(strlen(fname)); zip->path = fname; zip->flags = flags; zip->cb = cb; @@ -177,6 +201,10 @@ int uv_zip_stat(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t f } int uv_zip_fopen(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t flags, uv_zip_cb cb) { + assert(loop); + assert(zip); + assert(fname); + assert(strlen(fname)); zip->path = fname; zip->flags = flags; zip->cb = cb; @@ -184,12 +212,19 @@ int uv_zip_fopen(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t } int uv_zip_fclose(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_zip_cb cb) { + assert(loop); + assert(zip); + assert(file); zip->file = file; zip->cb = cb; return uv_queue_work(loop, &zip->work, uv__zip_work_fclose, uv__zip_after_work); } int uv_zip_fread(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_buf_t *buf, uv_zip_cb cb) { + assert(loop); + assert(zip); + assert(file); + assert(buf); zip->file = file; zip->buf = buf; zip->cb = cb; @@ -197,6 +232,8 @@ int uv_zip_fread(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_buf_t } int uv_zip_discard(uv_loop_t* loop, uv_zip_t *zip, uv_zip_cb cb) { + assert(loop); + assert(zip); zip->cb = cb; return uv_queue_work(loop, &zip->work, uv__zip_work_discard, uv__zip_after_work); } From fe0baffe0dfbf369f422e175c0fcf3dda9055118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Feb 2015 18:30:19 -0800 Subject: [PATCH 2/7] update uv_zip and add tests --- gyp/asset-fs.gypi | 17 ++++++ gyp/asset-zip.gypi | 17 ++++++ .../storage/default/shared_request_base.hpp | 4 +- include/mbgl/storage/default_file_source.hpp | 7 ++- platform/default/asset_request_fs.cpp | 3 +- platform/default/asset_request_zip.cpp | 51 +++++++++++------- platform/default/uv_zip.c | 13 +++++ src/mbgl/storage/default_file_source.cpp | 11 ++-- test/fixtures/storage/assets.zip | Bin 0 -> 1136 bytes test/storage/directory_reading.cpp | 10 +++- test/storage/file_reading.cpp | 43 ++++++++++++++- 11 files changed, 144 insertions(+), 32 deletions(-) create mode 100644 test/fixtures/storage/assets.zip diff --git a/gyp/asset-fs.gypi b/gyp/asset-fs.gypi index 0606eaf7f..f8ec0e355 100644 --- a/gyp/asset-fs.gypi +++ b/gyp/asset-fs.gypi @@ -25,6 +25,9 @@ 'libraries': [ '<@(uv_static_libs)', ], + 'defines': [ + '-DMBGL_ASSET_FS' + ], }, 'conditions': [ @@ -37,6 +40,20 @@ }], ], + 'direct_dependent_settings': { + 'conditions': [ + ['OS == "mac"', { + 'xcode_settings': { + 'OTHER_CFLAGS': [ '<@(defines)' ], + 'OTHER_CPLUSPLUSFLAGS': [ '<@(defines)' ], + } + }, { + 'cflags': [ '<@(defines)' ], + 'cflags_cc': [ '<@(defines)' ], + }] + ], + }, + 'link_settings': { 'conditions': [ ['OS == "mac"', { diff --git a/gyp/asset-zip.gypi b/gyp/asset-zip.gypi index 25df2a3d8..5c57aa18b 100644 --- a/gyp/asset-zip.gypi +++ b/gyp/asset-zip.gypi @@ -33,6 +33,9 @@ '<@(uv_static_libs)', '<@(zip_static_libs)', ], + 'defines': [ + '-DMBGL_ASSET_ZIP' + ], }, 'conditions': [ @@ -47,6 +50,20 @@ }], ], + 'direct_dependent_settings': { + 'conditions': [ + ['OS == "mac"', { + 'xcode_settings': { + 'OTHER_CFLAGS': [ '<@(defines)' ], + 'OTHER_CPLUSPLUSFLAGS': [ '<@(defines)' ], + } + }, { + 'cflags': [ '<@(defines)' ], + 'cflags_cc': [ '<@(defines)' ], + }] + ], + }, + 'link_settings': { 'conditions': [ ['OS == "mac"', { diff --git a/include/mbgl/storage/default/shared_request_base.hpp b/include/mbgl/storage/default/shared_request_base.hpp index a0f3e8d4e..7745bef2b 100644 --- a/include/mbgl/storage/default/shared_request_base.hpp +++ b/include/mbgl/storage/default/shared_request_base.hpp @@ -78,8 +78,10 @@ protected: public: const Resource resource; -private: +protected: DefaultFileSource *source = nullptr; + +private: std::set observers; }; diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index 21048e99e..86e241404 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -18,8 +18,8 @@ class SharedRequestBase; class DefaultFileSource : public FileSource { public: - DefaultFileSource(FileCache *cache); - DefaultFileSource(FileCache *cache, uv_loop_t *loop); + DefaultFileSource(FileCache *cache, const std::string &root = ""); + DefaultFileSource(FileCache *cache, uv_loop_t *loop, const std::string &root = ""); ~DefaultFileSource(); Request *request(const Resource &resource, uv_loop_t *loop, Callback callback); @@ -30,6 +30,9 @@ public: void notify(SharedRequestBase *sharedRequest, const std::set &observers, std::shared_ptr response, FileCache::Hint hint); +public: + const std::string assetRoot; + private: struct ActionDispatcher; struct AddRequestAction; diff --git a/platform/default/asset_request_fs.cpp b/platform/default/asset_request_fs.cpp index 24856e520..1e32ee313 100644 --- a/platform/default/asset_request_fs.cpp +++ b/platform/default/asset_request_fs.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include @@ -63,7 +62,7 @@ AssetRequestImpl::AssetRequestImpl(AssetRequest *request_, uv_loop_t *loop) : re path = url.substr(8); } else { // This is a relative path. Prefix with the application root. - path = platform::assetRoot() + "/" + url.substr(8); + path = request->source->getAssetRoot() + "/" + url.substr(8); } uv_fs_open(loop, &req, path.c_str(), O_RDONLY, S_IRUSR, fileOpened); diff --git a/platform/default/asset_request_zip.cpp b/platform/default/asset_request_zip.cpp index 9a5f598bb..fd27c4b95 100644 --- a/platform/default/asset_request_zip.cpp +++ b/platform/default/asset_request_zip.cpp @@ -2,8 +2,8 @@ #include #include #include +#include #include -#include #include "uv_zip.h" @@ -26,12 +26,12 @@ public: AssetZipContext(uv_loop_t *loop); ~AssetZipContext(); - uv_zip_t *getHandle(); - void returnHandle(uv_zip_t *zip); + uv_zip_t *getHandle(const std::string &path); + void returnHandle(const std::string &path, uv_zip_t *zip); private: // A list of resuable uv_zip handles to avoid creating and destroying them all the time. - std::forward_list handles; + std::map> handles; }; // ------------------------------------------------------------------------------------------------- @@ -42,28 +42,30 @@ template<> pthread_once_t ThreadContext::once = PTHREAD_ONCE_IN AssetZipContext::AssetZipContext(uv_loop_t *loop_) : ThreadContext(loop_) { } -uv_zip_t *AssetZipContext::getHandle() { - if (!handles.empty()) { - auto zip = handles.front(); - handles.pop_front(); +uv_zip_t *AssetZipContext::getHandle(const std::string &path) { + auto &list = handles[path]; + if (!list.empty()) { + auto zip = list.front(); + list.pop_front(); return zip; } else { return nullptr; } } -void AssetZipContext::returnHandle(uv_zip_t *zip) { - uv_zip_cleanup(zip); - handles.push_front(zip); +void AssetZipContext::returnHandle(const std::string &path, uv_zip_t *zip) { + handles[path].push_front(zip); } AssetZipContext::~AssetZipContext() { // Close all zip handles - for (auto zip : handles) { - uv_zip_discard(loop, zip, [](uv_zip_t *zip_) { - uv_zip_cleanup(zip_); - delete zip_; - }); + for (auto &list : handles) { + for (auto zip : list.second) { + uv_zip_discard(loop, zip, [](uv_zip_t *zip_) { + uv_zip_cleanup(zip_); + delete zip_; + }); + } } handles.clear(); } @@ -82,6 +84,7 @@ public: private: AssetZipContext &context; AssetRequest *request = nullptr; + const std::string root; const std::string path; std::unique_ptr response; uv_buf_t buffer; @@ -111,8 +114,9 @@ AssetRequestImpl::~AssetRequestImpl() { AssetRequestImpl::AssetRequestImpl(AssetRequest *request_, uv_loop_t *loop) : context(*AssetZipContext::Get(loop)), request(request_), - path(request->resource.url.substr(8)) { - auto zip = context.getHandle(); + root(request->source->assetRoot), + path(std::string { "assets/" } + request->resource.url.substr(8)) { + auto zip = context.getHandle(root); if (zip) { archiveOpened(zip); } else { @@ -124,9 +128,12 @@ void AssetRequestImpl::openZipArchive() { uv_fs_t *req = new uv_fs_t(); req->data = this; + assert(request); + assert(request->source); + // We're using uv_fs_open first to obtain a file descriptor. Then, uv_zip_fdopen will operate // on a read-only file. - uv_fs_open(context.loop, req, platform::assetRoot().c_str(), O_RDONLY, S_IRUSR, [](uv_fs_t *fsReq) { + uv_fs_open(context.loop, req, root.c_str(), O_RDONLY, S_IRUSR, [](uv_fs_t *fsReq) { if (fsReq->result < 0) { auto impl = reinterpret_cast(fsReq->data); impl->notifyError(uv::getFileRequestError(fsReq)); @@ -191,6 +198,10 @@ void AssetRequestImpl::fileStated(uv_zip_t *zip) { response->modified = zip->stat->mtime; } + if (zip->stat->valid & ZIP_STAT_INDEX) { + response->etag = std::to_string(zip->stat->index); + } + uv_zip_fopen(context.loop, zip, path.c_str(), 0, INVOKE_MEMBER(fileOpened)); } } @@ -239,7 +250,7 @@ void AssetRequestImpl::fileClosed(uv_zip_t *zip) { void AssetRequestImpl::cleanup(uv_zip_t *zip) { MBGL_VERIFY_THREAD(tid); - context.returnHandle(zip); + context.returnHandle(root, zip); delete this; } diff --git a/platform/default/uv_zip.c b/platform/default/uv_zip.c index 7cb3c516d..51bd15e8e 100644 --- a/platform/default/uv_zip.c +++ b/platform/default/uv_zip.c @@ -172,6 +172,7 @@ int uv_zip_open(uv_loop_t* loop, uv_zip_t *zip, const char *path, zip_flags_t fl assert(zip); assert(path); assert(strlen(path)); + zip->result = 0; zip->path = path; zip->flags = flags; zip->cb = cb; @@ -182,6 +183,7 @@ int uv_zip_fdopen(uv_loop_t* loop, uv_zip_t *zip, uv_file fd, int flags, uv_zip_ assert(loop); assert(zip); assert(fd); + zip->result = 0; zip->path = malloc(sizeof(uv_file)); *(uv_file *)zip->path = fd; zip->flags = flags; @@ -194,6 +196,8 @@ int uv_zip_stat(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t f assert(zip); assert(fname); assert(strlen(fname)); + assert(zip->archive); + zip->result = 0; zip->path = fname; zip->flags = flags; zip->cb = cb; @@ -205,8 +209,11 @@ int uv_zip_fopen(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t assert(zip); assert(fname); assert(strlen(fname)); + assert(zip->archive); + zip->result = 0; zip->path = fname; zip->flags = flags; + zip->file = NULL; zip->cb = cb; return uv_queue_work(loop, &zip->work, uv__zip_work_fopen, uv__zip_after_work); } @@ -215,6 +222,8 @@ int uv_zip_fclose(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_zip_ assert(loop); assert(zip); assert(file); + assert(zip->archive); + zip->result = 0; zip->file = file; zip->cb = cb; return uv_queue_work(loop, &zip->work, uv__zip_work_fclose, uv__zip_after_work); @@ -225,6 +234,8 @@ int uv_zip_fread(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_buf_t assert(zip); assert(file); assert(buf); + assert(zip->archive); + zip->result = 0; zip->file = file; zip->buf = buf; zip->cb = cb; @@ -234,6 +245,8 @@ int uv_zip_fread(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_buf_t int uv_zip_discard(uv_loop_t* loop, uv_zip_t *zip, uv_zip_cb cb) { assert(loop); assert(zip); + assert(!zip->file); + zip->result = 0; zip->cb = cb; return uv_queue_work(loop, &zip->work, uv__zip_work_discard, uv__zip_after_work); } diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index e40aaff64..6d27adbe0 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -49,8 +50,9 @@ struct DefaultFileSource::StopAction { }; -DefaultFileSource::DefaultFileSource(FileCache *cache_) - : loop(uv_loop_new()), +DefaultFileSource::DefaultFileSource(FileCache *cache_, const std::string &root) + : assetRoot(root.empty() ? platform::assetRoot() : root), + loop(uv_loop_new()), cache(cache_), queue(new Queue(loop, [this](Action &action) { mapbox::util::apply_visitor(ActionDispatcher{*this}, action); @@ -63,8 +65,9 @@ DefaultFileSource::DefaultFileSource(FileCache *cache_) }) { } -DefaultFileSource::DefaultFileSource(FileCache *cache_, uv_loop_t *loop_) - : loop(loop_), +DefaultFileSource::DefaultFileSource(FileCache *cache_, uv_loop_t *loop_, const std::string &root) + : assetRoot(root.empty() ? platform::assetRoot() : root), + loop(loop_), cache(cache_), queue(new Queue(loop, [this](Action &action) { mapbox::util::apply_visitor(ActionDispatcher{*this}, action); diff --git a/test/fixtures/storage/assets.zip b/test/fixtures/storage/assets.zip new file mode 100644 index 0000000000000000000000000000000000000000..3127798145421c9caf6a97ee26ec2bd949e1213d GIT binary patch literal 1136 zcmWIWW@h1H00FC>05>oLO0YA?FeDZir?9|+Xl1gycuJXPV!i+C$n}CjgyJtGP05HTs zSc@nN^78W#c7V(Wc?2!~lJoOQQu9g_GK&>5Qj1c#0=yZSv;k%}jNpO$17;62vVoR?5)>X!VJ0VtWk6eimdz!~ dGDPA6+J`mqva$h-^6>xw literal 0 HcmV?d00001 diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp index bf472b5c7..3ee4dd172 100644 --- a/test/storage/directory_reading.cpp +++ b/test/storage/directory_reading.cpp @@ -4,12 +4,16 @@ #include -TEST_F(Storage, ReadDirectory) { +TEST_F(Storage, AssetReadDirectory) { SCOPED_TEST(ReadDirectory) using namespace mbgl; +#ifdef MBGL_ASSET_ZIP + DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip"); +#else DefaultFileSource fs(nullptr, uv_default_loop()); +#endif fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, uv_default_loop(), [&](const Response &res) { EXPECT_EQ(Response::Error, res.status); @@ -17,7 +21,11 @@ TEST_F(Storage, ReadDirectory) { EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); +#ifdef MBGL_ASSET_ZIP + EXPECT_EQ("No such file", res.message); +#elif MBGL_ASSET_FS EXPECT_EQ("illegal operation on a directory", res.message); +#endif ReadDirectory.finish(); }); diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp index 622eb2b8e..7e14fdcb1 100644 --- a/test/storage/file_reading.cpp +++ b/test/storage/file_reading.cpp @@ -5,12 +5,16 @@ #include #include -TEST_F(Storage, EmptyFile) { +TEST_F(Storage, AssetEmptyFile) { SCOPED_TEST(EmptyFile) using namespace mbgl; +#ifdef MBGL_ASSET_ZIP + DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip"); +#else DefaultFileSource fs(nullptr, uv_default_loop()); +#endif fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, uv_default_loop(), @@ -27,12 +31,43 @@ TEST_F(Storage, EmptyFile) { uv_run(uv_default_loop(), UV_RUN_DEFAULT); } -TEST_F(Storage, NonExistentFile) { +TEST_F(Storage, AssetNonEmptyFile) { + SCOPED_TEST(NonEmptyFile) + + using namespace mbgl; + +#ifdef MBGL_ASSET_ZIP + DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip"); +#else + DefaultFileSource fs(nullptr, uv_default_loop()); +#endif + + fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, + uv_default_loop(), + [&](const Response &res) { + EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(16ul, res.data.size()); + EXPECT_EQ(0, res.expires); + EXPECT_LT(1420000000, res.modified); + EXPECT_NE("", res.etag); + EXPECT_EQ("", res.message); + EXPECT_EQ("content is here\n", res.data); + NonEmptyFile.finish(); + }); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); +} + +TEST_F(Storage, AssetNonExistentFile) { SCOPED_TEST(NonExistentFile) using namespace mbgl; +#ifdef MBGL_ASSET_ZIP + DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip"); +#else DefaultFileSource fs(nullptr, uv_default_loop()); +#endif fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, uv_default_loop(), @@ -42,7 +77,11 @@ TEST_F(Storage, NonExistentFile) { EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); +#ifdef MBGL_ASSET_ZIP + EXPECT_EQ("No such file", res.message); +#elif MBGL_ASSET_FS EXPECT_EQ("no such file or directory", res.message); +#endif NonExistentFile.finish(); }); From b29bce0936ac711b7fa6c2a43684696021dfb668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Fri, 6 Feb 2015 16:13:25 +0100 Subject: [PATCH 3/7] add ca bundle loading back to CURL --- Makefile | 3 + .../com/mapbox/mapboxgl/app/MainActivity.java | 12 +- gyp/http-curl.gypi | 28 ++-- platform/default/http_request_curl.cpp | 122 +++++++++++++++++- src/mbgl/map/source.cpp | 4 +- src/mbgl/map/sprite.cpp | 2 +- src/mbgl/storage/default_file_source.cpp | 1 + test/storage/http_error.cpp | 2 +- 8 files changed, 150 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index a409f4bd7..da9b2aae1 100644 --- a/Makefile +++ b/Makefile @@ -165,6 +165,9 @@ Makefile/android-%: config/android-%.gypi android-lib-%: Makefile/android-% @$(ENV_android-$*) ./scripts/flock.py -v build/Android.lock $(MAKE) -C build/android-$* BUILDTYPE=$(BUILDTYPE) androidapp +# Builds the selected/default Android library +android-lib: android-lib-$(ANDROID_ABI) + # Builds the selected/default Android library android: android-lib-$(ANDROID_ABI) cd android/java && ./gradlew --parallel-threads=$(JOBS) build diff --git a/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java b/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java index e8b1d578b..a10c0206b 100644 --- a/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java +++ b/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java @@ -214,7 +214,7 @@ public class MainActivity extends ActionBarActivity { switch(position) { // Bright case 0: - mMapFragment.getMap().setStyleUrl("asset://styles/styles/bright-v7.json"); + mMapFragment.getMap().setStyleUrl("asset://styles/bright-v7.json"); mMapFragment.getMap().removeAllClasses(); mClassSpinner.setVisibility(View.INVISIBLE); mClassSpinner.setAdapter(null); @@ -223,7 +223,7 @@ public class MainActivity extends ActionBarActivity { // Basic case 1: - mMapFragment.getMap().setStyleUrl("asset://styles/styles/basic-v7.json"); + mMapFragment.getMap().setStyleUrl("asset://styles/basic-v7.json"); mMapFragment.getMap().removeAllClasses(); mClassSpinner.setVisibility(View.INVISIBLE); mClassSpinner.setAdapter(null); @@ -232,7 +232,7 @@ public class MainActivity extends ActionBarActivity { // Outdoors case 2: - mMapFragment.getMap().setStyleUrl("asset://styles/styles/outdoors-v7.json"); + mMapFragment.getMap().setStyleUrl("asset://styles/outdoors-v7.json"); mMapFragment.getMap().removeAllClasses(); mClassSpinner.setVisibility(View.VISIBLE); mClassSpinner.setAdapter(mOutdoorsClassAdapter); @@ -241,7 +241,7 @@ public class MainActivity extends ActionBarActivity { // Satellite case 3: - mMapFragment.getMap().setStyleUrl("asset://styles/styles/satellite-v7.json"); + mMapFragment.getMap().setStyleUrl("asset://styles/satellite-v7.json"); mMapFragment.getMap().removeAllClasses(); mClassSpinner.setVisibility(View.VISIBLE); mClassSpinner.setAdapter(mSatelliteClassAdapter); @@ -250,7 +250,7 @@ public class MainActivity extends ActionBarActivity { // Pencil case 4: - mMapFragment.getMap().setStyleUrl("asset://styles/styles/pencil-v7.json"); + mMapFragment.getMap().setStyleUrl("asset://styles/pencil-v7.json"); mMapFragment.getMap().removeAllClasses(); mClassSpinner.setVisibility(View.INVISIBLE); mClassSpinner.setAdapter(null); @@ -259,7 +259,7 @@ public class MainActivity extends ActionBarActivity { // Empty case 5: - mMapFragment.getMap().setStyleUrl("asset://styles/styles/empty-v7.json"); + mMapFragment.getMap().setStyleUrl("asset://styles/empty-v7.json"); mMapFragment.getMap().removeAllClasses(); mClassSpinner.setVisibility(View.INVISIBLE); mClassSpinner.setAdapter(null); diff --git a/gyp/http-curl.gypi b/gyp/http-curl.gypi index 87edff249..bfb2054d5 100644 --- a/gyp/http-curl.gypi +++ b/gyp/http-curl.gypi @@ -14,6 +14,24 @@ '../include', ], + 'conditions': [ + ['host == "android"', { + 'variables': { + # Android uses libzip and openssl to set CURL's CA bundle. + 'cflags_cc': [ '<@(zip_cflags)', '<@(openssl_cflags)' ], + 'ldflags': [ '<@(zip_ldflags)', ], + 'libraries': [ '<@(zip_static_libs)', ], + }, + }], + ['OS == "mac"', { + 'xcode_settings': { + 'OTHER_CPLUSPLUSFLAGS': [ '<@(cflags_cc)' ], + }, + }, { + 'cflags_cc': [ '<@(cflags_cc)' ], + }], + ], + 'variables': { 'cflags_cc': [ '<@(uv_cflags)', @@ -30,16 +48,6 @@ ], }, - 'conditions': [ - ['OS == "mac"', { - 'xcode_settings': { - 'OTHER_CPLUSPLUSFLAGS': [ '<@(cflags_cc)' ], - }, - }, { - 'cflags_cc': [ '<@(cflags_cc)' ], - }], - ], - 'link_settings': { 'conditions': [ ['OS == "mac"', { diff --git a/platform/default/http_request_curl.cpp b/platform/default/http_request_curl.cpp index 411c5ee47..8772377a9 100644 --- a/platform/default/http_request_curl.cpp +++ b/platform/default/http_request_curl.cpp @@ -1,11 +1,18 @@ #include #include #include +#include #include #include +#ifdef __ANDROID__ +#include +#include +#include +#endif + #include #include #include @@ -125,6 +132,8 @@ private: int attempts = 0; static const int maxAttempts = 4; + + char error[CURL_ERROR_SIZE]; }; @@ -317,6 +326,104 @@ int HTTPCURLContext::startTimeout(CURLM * /* multi */, long timeout_ms, void *us // ------------------------------------------------------------------------------------------------- +#ifdef __ANDROID__ + +// This function is called to load the CA bundle +// from http://curl.haxx.se/libcurl/c/cacertinmem.html¯ +static CURLcode sslctx_function(CURL * /* curl */, void *sslctx, void * /* parm */) { + + int error = 0; + struct zip *apk = zip_open(mbgl::android::apkPath.c_str(), 0, &error); + if (apk == nullptr) { + return CURLE_SSL_CACERT_BADFILE; + } + + struct zip_file *apkFile = zip_fopen(apk, "assets/ca-bundle.crt", ZIP_FL_NOCASE); + if (apkFile == nullptr) { + zip_close(apk); + apk = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + struct zip_stat stat; + if (zip_stat(apk, "assets/ca-bundle.crt", ZIP_FL_NOCASE, &stat) != 0) { + zip_fclose(apkFile); + apkFile = nullptr; + zip_close(apk); + apk = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + if (stat.size > std::numeric_limits::max()) { + zip_fclose(apkFile); + apkFile = nullptr; + zip_close(apk); + apk = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + const auto pem = util::make_unique(stat.size); + + if (static_cast(zip_fread(apkFile, reinterpret_cast(pem.get()), stat.size)) != stat.size) { + zip_fclose(apkFile); + apkFile = nullptr; + zip_close(apk); + apk = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + // get a pointer to the X509 certificate store (which may be empty!) + X509_STORE *store = SSL_CTX_get_cert_store((SSL_CTX *)sslctx); + if (store == nullptr) { + return CURLE_SSL_CACERT_BADFILE; + } + + // get a BIO + BIO *bio = BIO_new_mem_buf(pem.get(), static_cast(stat.size)); + if (bio == nullptr) { + store = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + // use it to read the PEM formatted certificate from memory into an X509 + // structure that SSL can use + X509 *cert = nullptr; + while (PEM_read_bio_X509(bio, &cert, 0, nullptr) != nullptr) { + if (cert == nullptr) { + BIO_free(bio); + bio = nullptr; + store = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + // add our certificate to this store + if (X509_STORE_add_cert(store, cert) == 0) { + X509_free(cert); + cert = nullptr; + BIO_free(bio); + bio = nullptr; + store = nullptr; + return CURLE_SSL_CACERT_BADFILE; + } + + X509_free(cert); + cert = nullptr; + } + + // decrease reference counts + BIO_free(bio); + bio = nullptr; + + zip_fclose(apkFile); + apkFile = nullptr; + zip_close(apk); + apk = nullptr; + + // all set to go + return CURLE_OK; +} +#endif + HTTPRequestImpl::HTTPRequestImpl(HTTPRequest *request_, uv_loop_t *loop, std::unique_ptr response_) : context(HTTPCURLContext::Get(loop)), request(request_), @@ -325,6 +432,9 @@ HTTPRequestImpl::HTTPRequestImpl(HTTPRequest *request_, uv_loop_t *loop, std::un assert(request); context->addRequest(request); + // Zero out the error buffer. + memset(error, 0, sizeof(error)); + // If there's already a response, set the correct etags/modified headers to make sure we are // getting a 304 response if possible. This avoids redownloading unchanged data. if (existingResponse) { @@ -343,7 +453,13 @@ HTTPRequestImpl::HTTPRequestImpl(HTTPRequest *request_, uv_loop_t *loop, std::un } handleError(curl_easy_setopt(handle, CURLOPT_PRIVATE, this)); + handleError(curl_easy_setopt(handle, CURLOPT_ERRORBUFFER, error)); +#ifdef __ANDROID__ + handleError(curl_easy_setopt(handle, CURLOPT_SSLCERTTYPE, "PEM")); + handleError(curl_easy_setopt(handle, CURLOPT_SSL_CTX_FUNCTION, sslctx_function)); +#else handleError(curl_easy_setopt(handle, CURLOPT_CAINFO, "ca-bundle.crt")); +#endif handleError(curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 1)); handleError(curl_easy_setopt(handle, CURLOPT_URL, request->resource.url.c_str())); handleError(curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, writeCallback)); @@ -550,21 +666,19 @@ void HTTPRequestImpl::handleResult(CURLcode code) { // Add human-readable error code if (code != CURLE_OK) { - response->message = curl_easy_strerror(code); + response->status = Response::Error; + response->message = std::string { curl_easy_strerror(code) } + ": " + error; switch (code) { case CURLE_COULDNT_RESOLVE_PROXY: case CURLE_COULDNT_RESOLVE_HOST: case CURLE_COULDNT_CONNECT: - response->status = Response::Error; return finish(ResponseStatus::ConnectionError); case CURLE_OPERATION_TIMEDOUT: - response->status = Response::Error; return finish(ResponseStatus::TemporaryError); default: - response->status = Response::Error; return finish(ResponseStatus::PermanentError); } } else { diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index f23bcaa14..cdf9bdd37 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -43,7 +43,7 @@ void Source::load(Map& map, FileSource& fileSource) { const std::string url = util::mapbox::normalizeSourceURL(info.url, map.getAccessToken()); fileSource.request({ Resource::Kind::JSON, url }, **map.loop, [source, &map](const Response &res) { if (res.status != Response::Successful) { - Log::Warning(Event::General, "failed to load source TileJSON"); + Log::Warning(Event::General, "Failed to load source TileJSON: %s", res.message.c_str()); return; } @@ -51,7 +51,7 @@ void Source::load(Map& map, FileSource& fileSource) { d.Parse<0>(res.data.c_str()); if (d.HasParseError()) { - Log::Warning(Event::General, "invalid source TileJSON"); + Log::Warning(Event::General, "Invalid source TileJSON; Parse Error at %d: %s", d.GetErrorOffset(), d.GetParseError()); return; } diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index 19f7b7c8c..9543fe083 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -82,7 +82,7 @@ void Sprite::load(FileSource& fileSource) { sprite->parseImage(); sprite->complete(); } else { - Log::Warning(Event::Sprite, "Failed to load sprite image: Error %s", res.message.c_str()); + Log::Warning(Event::Sprite, "Failed to load sprite image: %s", res.message.c_str()); if (!sprite->future.valid()) { sprite->promise.set_exception(std::make_exception_ptr(std::runtime_error(res.message))); } diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index 6d27adbe0..c6b201b55 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -10,6 +10,7 @@ #include #include +#include #pragma GCC diagnostic push #ifndef __clang__ diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index 45c23b94b..498f1eae6 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -44,7 +44,7 @@ TEST_F(Storage, HTTPError) { EXPECT_LT(1.5, duration) << "Resource wasn't retried the correct number of times"; EXPECT_GT(1.7, duration) << "Resource wasn't retried the correct number of times"; EXPECT_EQ(Response::Error, res.status); - EXPECT_TRUE(res.message == "Couldn't connect to server" || res.message == "The operation couldn’t be completed. (NSURLErrorDomain error -1004.)"); + EXPECT_TRUE(res.message == "Couldn't connect to server: couldn't connect to host" || res.message == "The operation couldn’t be completed. (NSURLErrorDomain error -1004.)") << res.message; EXPECT_EQ("", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); From 8f32c8ae7ad09f13229eed0ec75753f93c443adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Feb 2015 18:46:09 -0800 Subject: [PATCH 4/7] wrong symbol name --- platform/default/asset_request_fs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/default/asset_request_fs.cpp b/platform/default/asset_request_fs.cpp index 1e32ee313..5ebde2d88 100644 --- a/platform/default/asset_request_fs.cpp +++ b/platform/default/asset_request_fs.cpp @@ -62,7 +62,7 @@ AssetRequestImpl::AssetRequestImpl(AssetRequest *request_, uv_loop_t *loop) : re path = url.substr(8); } else { // This is a relative path. Prefix with the application root. - path = request->source->getAssetRoot() + "/" + url.substr(8); + path = request->source->assetRoot + "/" + url.substr(8); } uv_fs_open(loop, &req, path.c_str(), O_RDONLY, S_IRUSR, fileOpened); From d16974264cff51f2c143ba646b2b50bf7a1b0ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Feb 2015 18:49:48 -0800 Subject: [PATCH 5/7] update uv_zip --- platform/default/uv_zip.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/platform/default/uv_zip.c b/platform/default/uv_zip.c index 51bd15e8e..cb1b07898 100644 --- a/platform/default/uv_zip.c +++ b/platform/default/uv_zip.c @@ -118,6 +118,8 @@ void uv__zip_work_fclose(uv_work_t *req) { zip->result = zip_fclose(zip->file); if (zip->result != 0) { uv__zip_file_error(zip); + } else { + zip->file = NULL; } } From ef57f6b5227eb83c3707b663d581d9cfcce84432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Feb 2015 19:03:04 -0800 Subject: [PATCH 6/7] wrong parameter order --- .../src/main/java/com/mapbox/mapboxgl/app/MainActivity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java b/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java index a10c0206b..831dd99ae 100644 --- a/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java +++ b/android/java/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxgl/app/MainActivity.java @@ -201,7 +201,7 @@ public class MainActivity extends ActionBarActivity { // Handles location updates from GPS private void updateLocation(Location location) { if (location != null) { - LatLngZoom coordinate = new LatLngZoom(location.getLongitude(), location.getLatitude(), 16); + LatLngZoom coordinate = new LatLngZoom(location.getLatitude(), location.getLongitude(), 16); mMapFragment.getMap().setCenterCoordinate(coordinate, true); } } From 18263436e94313020e2297edb83e70ec308ef5ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Feb 2015 22:08:03 -0800 Subject: [PATCH 7/7] add missing test fixture file --- test/fixtures/storage/nonempty | 1 + 1 file changed, 1 insertion(+) create mode 100644 test/fixtures/storage/nonempty diff --git a/test/fixtures/storage/nonempty b/test/fixtures/storage/nonempty new file mode 100644 index 000000000..929bb2313 --- /dev/null +++ b/test/fixtures/storage/nonempty @@ -0,0 +1 @@ +content is here