diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp index a44902e1f..278fce1df 100644 --- a/include/mbgl/storage/response.hpp +++ b/include/mbgl/storage/response.hpp @@ -23,7 +23,7 @@ public: // This is set to true for 304 Not Modified responses. bool notModified = false; - // The actual data of the response. This is null if notModified is true. + // The actual data of the response. Present only for non-error, non-notModified responses. std::shared_ptr data; optional modified; diff --git a/platform/android/src/http_request_android.cpp b/platform/android/src/http_request_android.cpp index 390334627..105ba5860 100644 --- a/platform/android/src/http_request_android.cpp +++ b/platform/android/src/http_request_android.cpp @@ -199,17 +199,14 @@ void HTTPAndroidRequest::onResponse(JNIEnv* env, int code, jstring /* message */ response->expires = util::parseTimePoint(mbgl::android::std_string_from_jstring(env, expires).c_str()); } - if (body != nullptr) { - jbyte* bodyData = env->GetByteArrayElements(body, nullptr); - response->data = std::make_shared(reinterpret_cast(bodyData), env->GetArrayLength(body)); - env->ReleaseByteArrayElements(body, bodyData, JNI_ABORT); - } - if (code == 200) { - // Nothing to do; this is what we want + if (body != nullptr) { + jbyte* bodyData = env->GetByteArrayElements(body, nullptr); + response->data = std::make_shared(reinterpret_cast(bodyData), env->GetArrayLength(body)); + env->ReleaseByteArrayElements(body, bodyData, JNI_ABORT); + } } else if (code == 304) { response->notModified = true; - response->data.reset(); } else if (code == 404) { response->error = std::make_unique(Error::Reason::NotFound, "HTTP status code 404"); } else if (code >= 500 && code < 600) { diff --git a/platform/darwin/http_request_nsurl.mm b/platform/darwin/http_request_nsurl.mm index fa705638b..890e347af 100644 --- a/platform/darwin/http_request_nsurl.mm +++ b/platform/darwin/http_request_nsurl.mm @@ -194,8 +194,6 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e } else if ([res isKindOfClass:[NSHTTPURLResponse class]]) { const long responseCode = [(NSHTTPURLResponse *)res statusCode]; - response->data = std::make_shared((const char *)[data bytes], [data length]); - NSDictionary *headers = [(NSHTTPURLResponse *)res allHeaderFields]; NSString *cache_control = [headers objectForKey:@"Cache-Control"]; if (cache_control) { @@ -218,10 +216,9 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e } if (responseCode == 200) { - // Nothing to do; this is what we want. + response->data = std::make_shared((const char *)[data bytes], [data length]); } else if (responseCode == 304) { response->notModified = true; - response->data.reset(); } else if (responseCode == 404) { response->error = std::make_unique(Error::Reason::NotFound, "HTTP status code 404"); diff --git a/platform/default/http_request_curl.cpp b/platform/default/http_request_curl.cpp index f7fb27afc..660da4c43 100644 --- a/platform/default/http_request_curl.cpp +++ b/platform/default/http_request_curl.cpp @@ -509,19 +509,14 @@ void HTTPCURLRequest::handleResult(CURLcode code) { long responseCode = 0; curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &responseCode); - // Move over any data we got. We're storing this in a separate object because the Response - // object defines it as const. - if (data) { - response->data = std::move(data); - } else { - response->data = std::make_shared(); - } - if (responseCode == 200) { - // Nothing to do; this is what we want. + if (data) { + response->data = std::move(data); + } else { + response->data = std::make_shared(); + } } else if (responseCode == 304) { response->notModified = true; - response->data.reset(); } else if (responseCode == 404) { response->error = std::make_unique(Error::Reason::NotFound, "HTTP status code 404"); diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index 3f29e5c74..259ea825e 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -26,8 +26,7 @@ TEST_F(Storage, HTTPTemporaryError) { ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); EXPECT_EQ("HTTP status code 500", res.error->message); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ("", *res.data); + ASSERT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); diff --git a/test/storage/http_reading.cpp b/test/storage/http_reading.cpp index 36fbf43a1..d360ec003 100644 --- a/test/storage/http_reading.cpp +++ b/test/storage/http_reading.cpp @@ -47,9 +47,8 @@ TEST_F(Storage, HTTP404) { EXPECT_TRUE(util::ThreadContext::currentlyOn(util::ThreadType::Main)); ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ("Cannot GET /doesnotexist\n", *res.data); EXPECT_EQ("HTTP status code 404", res.error->message); + EXPECT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); @@ -74,9 +73,8 @@ TEST_F(Storage, HTTP500) { EXPECT_TRUE(util::ThreadContext::currentlyOn(util::ThreadType::Main)); ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ("Server Error!", *res.data); EXPECT_EQ("HTTP status code 500", res.error->message); + EXPECT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag));