[core] align implementations of local and asset file sourceupstream/local-asset-filesource

6 files changed, 84 insertions, 26 deletions

diff --git a/platform/default/asset_file_source.cpp b/platform/default/asset_file_source.cpp
index 54dbb8d0f6..3063bf88a0 100644
--- a/platform/default/asset_file_source.cpp
+++ b/platform/default/asset_file_source.cpp
@@ -10,6 +10,12 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+namespace {
+
+const std::string assetProtocol = "asset://";
+
+} // namespace
+
 namespace mbgl {
 
 class AssetFileSource::Impl {
@@ -19,18 +25,17 @@ public:
     }
 
     void request(const std::string& url, ActorRef<FileSourceRequest> req) {
-        std::string path;
+        Response response;
 
-        if (url.size() <= 8 || url[8] == '/') {
-            // This is an empty or absolute path.
-            path = mbgl::util::percentDecode(url.substr(8));
-        } else {
-            // This is a relative path. Prefix with the application root.
-            path = root + "/" + mbgl::util::percentDecode(url.substr(8));
+        if (!acceptsURL(url)) {
+            response.error = std::make_unique<Response::Error>(Response::Error::Reason::Other,
+                                                               "Invalid asset URL");
+            req.invoke(&FileSourceRequest::setResponse, response);
+            return;
         }
 
-        Response response;
-
+        // Cut off the protocol and prefix with path.
+        const auto path = root + "/" + mbgl::util::percentDecode(url.substr(assetProtocol.size()));
         struct stat buf;
         int result = stat(path.c_str(), &buf);
 
@@ -69,4 +74,8 @@ std::unique_ptr<AsyncRequest> AssetFileSource::request(const Resource& resource,
     return std::move(req);
 }
 
+bool AssetFileSource::acceptsURL(const std::string& url) {
+    return std::equal(assetProtocol.begin(), assetProtocol.end(), url.begin());
+}
+
 } // namespace mbgl
diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp
index 608b782ab9..cb602995a4 100644
--- a/platform/default/default_file_source.cpp
+++ b/platform/default/default_file_source.cpp
@@ -14,16 +14,6 @@
 
 #include <cassert>
 
-namespace {
-
-const std::string assetProtocol = "asset://";
-
-bool isAssetURL(const std::string& url) {
-    return std::equal(assetProtocol.begin(), assetProtocol.end(), url.begin());
-}
-
-} // namespace
-
 namespace mbgl {
 
 class DefaultFileSource::Impl {
@@ -118,7 +108,7 @@ public:
             ref.invoke(&FileSourceRequest::setResponse, res);
         };
 
-        if (isAssetURL(resource.url)) {
+        if (AssetFileSource::acceptsURL(resource.url)) {
             //Asset request
             tasks[req] = assetFileSource->request(resource, callback);
         } else if (LocalFileSource::acceptsURL(resource.url)) {
diff --git a/platform/default/local_file_source.cpp b/platform/default/local_file_source.cpp
index 21803d0935..0635e86d80 100644
--- a/platform/default/local_file_source.cpp
+++ b/platform/default/local_file_source.cpp
@@ -16,8 +16,7 @@
 
 namespace {
 
-const char* protocol = "file://";
-const std::size_t protocolLength = 7;
+const std::string fileProtocol = "file://";
 
 } // namespace
 
@@ -28,11 +27,17 @@ public:
     Impl(ActorRef<Impl>) {}
 
     void request(const std::string& url, ActorRef<FileSourceRequest> req) {
-        // Cut off the protocol
-        std::string path = mbgl::util::percentDecode(url.substr(protocolLength));
-
         Response response;
 
+        if (!acceptsURL(url)) {
+            response.error = std::make_unique<Response::Error>(Response::Error::Reason::Other,
+                                                               "Invalid file URL");
+            req.invoke(&FileSourceRequest::setResponse, response);
+            return;
+        }
+
+        // Cut off the protocol and prefix with path.
+        const auto path = mbgl::util::percentDecode(url.substr(fileProtocol.size()));
         struct stat buf;
         int result = stat(path.c_str(), &buf);
 
@@ -70,7 +75,7 @@ std::unique_ptr<AsyncRequest> LocalFileSource::request(const Resource& resource,
 }
 
 bool LocalFileSource::acceptsURL(const std::string& url) {
-    return url.compare(0, protocolLength, protocol) == 0;
+    return std::equal(fileProtocol.begin(), fileProtocol.end(), url.begin());
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/storage/asset_file_source.hpp b/src/mbgl/storage/asset_file_source.hpp
index 6ed7af8aaf..5d98b4e69e 100644
--- a/src/mbgl/storage/asset_file_source.hpp
+++ b/src/mbgl/storage/asset_file_source.hpp
@@ -15,6 +15,8 @@ public:
 
     std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
 
+    static bool acceptsURL(const std::string& url);
+
 private:
     class Impl;
 
diff --git a/test/storage/asset_file_source.test.cpp b/test/storage/asset_file_source.test.cpp
index a39d2963d2..978a41a306 100644
--- a/test/storage/asset_file_source.test.cpp
+++ b/test/storage/asset_file_source.test.cpp
@@ -69,6 +69,15 @@ TEST(AssetFileSource, Load) {
     loop.run();
 }
 
+TEST(AssetFileSource, AcceptsURL) {
+    EXPECT_TRUE(AssetFileSource::acceptsURL("asset://empty"));
+    EXPECT_TRUE(AssetFileSource::acceptsURL("asset:///test"));
+    EXPECT_FALSE(AssetFileSource::acceptsURL("assds://foo"));
+    EXPECT_FALSE(AssetFileSource::acceptsURL("asset:"));
+    EXPECT_FALSE(AssetFileSource::acceptsURL("style.json"));
+    EXPECT_FALSE(AssetFileSource::acceptsURL(""));
+}
+
 TEST(AssetFileSource, EmptyFile) {
     util::RunLoop loop;
 
@@ -118,6 +127,23 @@ TEST(AssetFileSource, NonExistentFile) {
     loop.run();
 }
 
+TEST(AssetFileSource, InvalidURL) {
+    util::RunLoop loop;
+
+    AssetFileSource fs("test/fixtures/storage/assets");
+
+    std::unique_ptr<AsyncRequest> req = fs.request({ Resource::Unknown, "test://wrong-scheme" }, [&](Response res) {
+        req.reset();
+        ASSERT_NE(nullptr, res.error);
+        EXPECT_EQ(Response::Error::Reason::Other, res.error->reason);
+        EXPECT_EQ("Invalid asset URL", res.error->message);
+        ASSERT_FALSE(res.data.get());
+        loop.stop();
+    });
+
+    loop.run();
+}
+
 TEST(AssetFileSource, ReadDirectory) {
     util::RunLoop loop;
 
diff --git a/test/storage/local_file_source.test.cpp b/test/storage/local_file_source.test.cpp
index 4d509e6c7d..e1756f8e7d 100644
--- a/test/storage/local_file_source.test.cpp
+++ b/test/storage/local_file_source.test.cpp
@@ -20,6 +20,15 @@ std::string toAbsoluteURL(const std::string& fileName) {
 
 using namespace mbgl;
 
+TEST(LocalFileSource, AcceptsURL) {
+    EXPECT_TRUE(LocalFileSource::acceptsURL("file://empty"));
+    EXPECT_TRUE(LocalFileSource::acceptsURL("file:///test"));
+    EXPECT_FALSE(LocalFileSource::acceptsURL("flie://foo"));
+    EXPECT_FALSE(LocalFileSource::acceptsURL("file:"));
+    EXPECT_FALSE(LocalFileSource::acceptsURL("style.json"));
+    EXPECT_FALSE(LocalFileSource::acceptsURL(""));
+}
+
 TEST(LocalFileSource, EmptyFile) {
     util::RunLoop loop;
 
@@ -69,6 +78,23 @@ TEST(LocalFileSource, NonExistentFile) {
     loop.run();
 }
 
+TEST(LocalFileSource, InvalidURL) {
+    util::RunLoop loop;
+
+    LocalFileSource fs;
+
+    std::unique_ptr<AsyncRequest> req = fs.request({ Resource::Unknown, "test://wrong-scheme" }, [&](Response res) {
+        req.reset();
+        ASSERT_NE(nullptr, res.error);
+        EXPECT_EQ(Response::Error::Reason::Other, res.error->reason);
+        EXPECT_EQ("Invalid file URL", res.error->message);
+        ASSERT_FALSE(res.data.get());
+        loop.stop();
+    });
+
+    loop.run();
+}
+
 TEST(LocalFileSource, ReadDirectory) {
     util::RunLoop loop;