From a6aae6c63175f7bb90d5a88f205d90ba6c74e485 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Thu, 29 Sep 2016 12:59:56 +0300 Subject: [PATCH] review fixes --- android/jni/Android.mk | 1 + android/jni/com/mapswithme/core/ScopedEnv.hpp | 40 +++ .../jni/com/mapswithme/util/HttpClient.cpp | 291 +++++++----------- .../src/com/mapswithme/util/HttpClient.java | 55 ++-- editor/config_loader.cpp | 4 +- editor/osm_auth.cpp | 132 ++++---- editor/user_stats.cpp | 10 +- platform/http_client.cpp | 165 ++++++++++ platform/http_client.hpp | 236 ++++---------- platform/http_client_apple.mm | 187 ++++++----- platform/http_client_curl.cpp | 57 ++-- platform/platform.pro | 1 + platform/platform_mac.mm | 3 - routing/online_cross_fetcher.cpp | 6 +- .../platform.xcodeproj/project.pbxproj | 4 + 15 files changed, 609 insertions(+), 583 deletions(-) create mode 100644 android/jni/com/mapswithme/core/ScopedEnv.hpp create mode 100644 platform/http_client.cpp diff --git a/android/jni/Android.mk b/android/jni/Android.mk index 4c31824c50..aed7643bfd 100644 --- a/android/jni/Android.mk +++ b/android/jni/Android.mk @@ -62,6 +62,7 @@ LOCAL_HEADER_FILES := \ ../../private.h \ com/mapswithme/core/jni_helper.hpp \ com/mapswithme/core/logging.hpp \ + com/mapswithme/core/ScopedEnv.hpp \ com/mapswithme/core/ScopedLocalRef.hpp \ com/mapswithme/maps/Framework.hpp \ com/mapswithme/opengl/android_gl_utils.hpp \ diff --git a/android/jni/com/mapswithme/core/ScopedEnv.hpp b/android/jni/com/mapswithme/core/ScopedEnv.hpp new file mode 100644 index 0000000000..d87b51419f --- /dev/null +++ b/android/jni/com/mapswithme/core/ScopedEnv.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include + +// Scoped environment which can attach to any thread and automatically detach +class ScopedEnv final +{ +public: + ScopedEnv(JavaVM * vm) + { + JNIEnv * env; + auto result = vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6); + if (result == JNI_EDETACHED) + { + result = vm->AttachCurrentThread(&env, nullptr); + m_needToDetach = (result == JNI_OK); + } + + if (result == JNI_OK) + { + m_env = env; + m_vm = vm; + } + } + + ~ScopedEnv() + { + if (m_vm != nullptr && m_needToDetach) + m_vm->DetachCurrentThread(); + } + + JNIEnv * operator->() { return m_env; } + operator bool() const { return m_env != nullptr; } + JNIEnv * get() { return m_env; } + +private: + bool m_needToDetach = false; + JNIEnv * m_env = nullptr; + JavaVM * m_vm = nullptr; +}; diff --git a/android/jni/com/mapswithme/util/HttpClient.cpp b/android/jni/com/mapswithme/util/HttpClient.cpp index 42b90697de..ccb2c74977 100644 --- a/android/jni/com/mapswithme/util/HttpClient.cpp +++ b/android/jni/com/mapswithme/util/HttpClient.cpp @@ -24,67 +24,97 @@ SOFTWARE. #include #include "../core/jni_helper.hpp" +#include "../core/ScopedEnv.hpp" +#include "../core/ScopedLocalRef.hpp" #include "platform/http_client.hpp" -#include "base/logging.hpp" #include "base/assert.hpp" +#include "base/exception.hpp" +#include "base/logging.hpp" #include "std/string.hpp" +#include "std/unordered_map.hpp" -namespace -{ -template -unique_ptr MakeScopedPointer(T * ptr, D deleter) -{ - return unique_ptr(ptr, deleter); -} - -// Scoped environment which can attach to any thread and automatically detach -class ScopedEnv final -{ -public: - ScopedEnv(JavaVM * vm) - { - JNIEnv * env; - auto result = vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6); - if (result == JNI_EDETACHED) - { - result = vm->AttachCurrentThread(&env, nullptr); - m_needToDetach = (result == JNI_OK); - } - - if (result == JNI_OK) - { - m_env = env; - m_vm = vm; - } - } - - ~ScopedEnv() - { - if (m_vm != nullptr && m_needToDetach) - m_vm->DetachCurrentThread(); - } - - JNIEnv * operator->() { return m_env; } - operator bool() const { return m_env != nullptr; } - JNIEnv * get() { return m_env; } - -private: - bool m_needToDetach = false; - JNIEnv * m_env = nullptr; - JavaVM * m_vm = nullptr; -}; -} // namespace +DECLARE_EXCEPTION(JniException, RootException); #define CLEAR_AND_RETURN_FALSE_ON_EXCEPTION \ - if (env->ExceptionCheck()) { \ + if (env->ExceptionCheck()) \ + { \ env->ExceptionDescribe(); \ env->ExceptionClear(); \ return false; \ } +namespace +{ +void RethrowOnJniException(ScopedEnv & env) +{ + if (!env->ExceptionCheck()) + return; + + env->ExceptionDescribe(); + env->ExceptionClear(); + MYTHROW(JniException, ()); +} + +jfieldID GetHttpParamsFieldId(ScopedEnv & env, const char * name) +{ + return env->GetFieldID(g_httpParamsClazz, name, "Ljava/lang/String;"); +} + +// Set string value to HttpClient.Params object, throws JniException and +void SetString(ScopedEnv & env, jobject params, jfieldID const fieldId, string const & value) +{ + if (value.empty()) + return; + + jni::ScopedLocalRef const wrappedValue(env.get(), jni::ToJavaString(env.get(), value)); + RethrowOnJniException(env); + + env->SetObjectField(params, fieldId, wrappedValue.get()); + RethrowOnJniException(env); +} + +// Get string value from HttpClient.Params object, throws JniException. +void GetString(ScopedEnv & env, jobject const params, jfieldID const fieldId, string & result) +{ + jni::ScopedLocalRef const wrappedValue( + env.get(), static_cast(env->GetObjectField(params, fieldId))); + RethrowOnJniException(env); + if (wrappedValue) + result = jni::ToNativeString(env.get(), wrappedValue.get()); +} + +class Ids +{ +public: + explicit Ids(ScopedEnv & env) + { + m_fieldIds = + {{"httpMethod", GetHttpParamsFieldId(env, "httpMethod")}, + {"contentType", GetHttpParamsFieldId(env, "contentType")}, + {"contentEncoding", GetHttpParamsFieldId(env, "contentEncoding")}, + {"userAgent", GetHttpParamsFieldId(env, "userAgent")}, + {"inputFilePath", GetHttpParamsFieldId(env, "inputFilePath")}, + {"basicAuthUser", GetHttpParamsFieldId(env, "basicAuthUser")}, + {"basicAuthPassword", GetHttpParamsFieldId(env, "basicAuthPassword")}, + {"cookies", GetHttpParamsFieldId(env, "cookies")}, + {"receivedUrl", GetHttpParamsFieldId(env, "receivedUrl")}}; + } + + jfieldID GetId(string const & fieldName) const + { + auto const it = m_fieldIds.find(fieldName); + CHECK(it != m_fieldIds.end(), ("Incorrect field name:", fieldName)); + return it->second; + } + +private: + unordered_map m_fieldIds; +}; +} // namespace + //*********************************************************************** // Exported functions implementation //*********************************************************************** @@ -97,24 +127,26 @@ bool HttpClient::RunHttpRequest() if (!env) return false; - auto const deleter = [&env](jobject o) { env->DeleteLocalRef(o); }; + static Ids ids(env); // Create and fill request params. - auto const jniUrl = MakeScopedPointer(jni::ToJavaString(env.get(), m_urlRequested), deleter); + jni::ScopedLocalRef const jniUrl(env.get(), + jni::ToJavaString(env.get(), m_urlRequested)); CLEAR_AND_RETURN_FALSE_ON_EXCEPTION static jmethodID const httpParamsConstructor = env->GetMethodID(g_httpParamsClazz, "", "(Ljava/lang/String;)V"); - auto const httpParamsObject = - MakeScopedPointer(env->NewObject(g_httpParamsClazz, httpParamsConstructor, jniUrl.get()), deleter); + jni::ScopedLocalRef const httpParamsObject( + env.get(), env->NewObject(g_httpParamsClazz, httpParamsConstructor, jniUrl.get())); CLEAR_AND_RETURN_FALSE_ON_EXCEPTION // Cache it on the first call. static jfieldID const dataField = env->GetFieldID(g_httpParamsClazz, "data", "[B"); if (!m_bodyData.empty()) { - auto const jniPostData = MakeScopedPointer(env->NewByteArray(m_bodyData.size()), deleter); + jni::ScopedLocalRef const jniPostData(env.get(), + env->NewByteArray(m_bodyData.size())); CLEAR_AND_RETURN_FALSE_ON_EXCEPTION env->SetByteArrayRegion(jniPostData.get(), 0, m_bodyData.size(), @@ -126,108 +158,22 @@ bool HttpClient::RunHttpRequest() } ASSERT(!m_httpMethod.empty(), ("Http method type can not be empty.")); - static jfieldID const httpMethodField = - env->GetFieldID(g_httpParamsClazz, "httpMethod", "Ljava/lang/String;"); - { - const auto jniHttpMethod = MakeScopedPointer(jni::ToJavaString(env.get(), m_httpMethod), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - env->SetObjectField(httpParamsObject.get(), httpMethodField, jniHttpMethod.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION + try + { + SetString(env, httpParamsObject.get(), ids.GetId("httpMethod"), m_httpMethod); + SetString(env, httpParamsObject.get(), ids.GetId("contentType"), m_contentType); + SetString(env, httpParamsObject.get(), ids.GetId("contentEncoding"), m_contentEncoding); + SetString(env, httpParamsObject.get(), ids.GetId("userAgent"), m_userAgent); + SetString(env, httpParamsObject.get(), ids.GetId("inputFilePath"), m_inputFile); + SetString(env, httpParamsObject.get(), ids.GetId("outputFilePath"), m_outputFile); + SetString(env, httpParamsObject.get(), ids.GetId("basicAuthUser"), m_basicAuthUser); + SetString(env, httpParamsObject.get(), ids.GetId("basicAuthPassword"), m_basicAuthPassword); + SetString(env, httpParamsObject.get(), ids.GetId("cookies"), m_cookies); } - - static jfieldID const contentTypeField = - env->GetFieldID(g_httpParamsClazz, "contentType", "Ljava/lang/String;"); - if (!m_contentType.empty()) + catch (JniException const & ex) { - auto const jniContentType = MakeScopedPointer(jni::ToJavaString(env.get(), m_contentType), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), contentTypeField, jniContentType.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - static jfieldID const contentEncodingField = - env->GetFieldID(g_httpParamsClazz, "contentEncoding", "Ljava/lang/String;"); - if (!m_contentEncoding.empty()) - { - auto const jniContentEncoding = MakeScopedPointer(jni::ToJavaString(env.get(), m_contentEncoding), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), contentEncodingField, jniContentEncoding.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - if (!m_userAgent.empty()) - { - static jfieldID const userAgentField = - env->GetFieldID(g_httpParamsClazz, "userAgent", "Ljava/lang/String;"); - - auto const jniUserAgent = MakeScopedPointer(jni::ToJavaString(env.get(), m_userAgent), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), userAgentField, jniUserAgent.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - if (!m_bodyFile.empty()) - { - static jfieldID const inputFilePathField = - env->GetFieldID(g_httpParamsClazz, "inputFilePath", "Ljava/lang/String;"); - - auto const jniInputFilePath = MakeScopedPointer(jni::ToJavaString(env.get(), m_bodyFile), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), inputFilePathField, jniInputFilePath.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - if (!m_receivedFile.empty()) - { - static jfieldID const outputFilePathField = - env->GetFieldID(g_httpParamsClazz, "outputFilePath", "Ljava/lang/String;"); - - auto const jniOutputFilePath = MakeScopedPointer(jni::ToJavaString(env.get(), m_receivedFile), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), outputFilePathField, jniOutputFilePath.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - if (!m_basicAuthUser.empty()) - { - static jfieldID const basicAuthUserField = - env->GetFieldID(g_httpParamsClazz, "basicAuthUser", "Ljava/lang/String;"); - - auto const jniBasicAuthUser = MakeScopedPointer(jni::ToJavaString(env.get(), m_basicAuthUser), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), basicAuthUserField, jniBasicAuthUser.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - if (!m_basicAuthPassword.empty()) - { - static jfieldID const basicAuthPasswordField = - env->GetFieldID(g_httpParamsClazz, "basicAuthPassword", "Ljava/lang/String;"); - - auto const jniBasicAuthPassword = - MakeScopedPointer(jni::ToJavaString(env.get(), m_basicAuthPassword), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), basicAuthPasswordField, jniBasicAuthPassword.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - } - - static jfieldID const cookiesField = - env->GetFieldID(g_httpParamsClazz, "cookies", "Ljava/lang/String;"); - if (!m_cookies.empty()) - { - const auto jniCookies = MakeScopedPointer(jni::ToJavaString(env.get(), m_cookies), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - - env->SetObjectField(httpParamsObject.get(), cookiesField, jniCookies.get()); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION + return false; } static jfieldID const debugModeField = env->GetFieldID(g_httpParamsClazz, "debugMode", "Z"); @@ -254,40 +200,31 @@ bool HttpClient::RunHttpRequest() m_errorCode = env->GetIntField(response, httpResponseCodeField); CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - static jfieldID const receivedUrlField = - env->GetFieldID(g_httpParamsClazz, "receivedUrl", "Ljava/lang/String;"); - auto const jniReceivedUrl = - MakeScopedPointer(static_cast(env->GetObjectField(response, receivedUrlField)), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - if (jniReceivedUrl) - m_urlReceived = jni::ToNativeString(env.get(), jniReceivedUrl.get()); - - // contentTypeField is already cached above. - auto const jniContentType = - MakeScopedPointer(static_cast(env->GetObjectField(response, contentTypeField)), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - if (jniContentType) - m_contentTypeReceived = jni::ToNativeString(env.get(), jniContentType.get()); - - // contentEncodingField is already cached above. - auto const jniContentEncoding = - MakeScopedPointer(static_cast(env->GetObjectField(response, contentEncodingField)), deleter); - CLEAR_AND_RETURN_FALSE_ON_EXCEPTION - if (jniContentEncoding) - m_contentEncodingReceived = jni::ToNativeString(env.get(), jniContentEncoding.get()); + try + { + GetString(env, response, ids.GetId("receivedUrl"), m_urlReceived); + GetString(env, response, ids.GetId("contentType"), m_contentTypeReceived); + GetString(env, response, ids.GetId("contentEncoding"), m_contentEncodingReceived); + } + catch (JniException const & ex) + { + return false; + } // Note: cookies field is used not only to send Cookie header, but also to receive back // Server-Cookie header. CookiesField is already cached above. - auto const jniServerCookies = - MakeScopedPointer(static_cast(env->GetObjectField(response, cookiesField)), deleter); + jni::ScopedLocalRef const jniServerCookies( + env.get(), static_cast(env->GetObjectField(response, ids.GetId("cookies")))); CLEAR_AND_RETURN_FALSE_ON_EXCEPTION if (jniServerCookies) + { m_serverCookies = - normalize_server_cookies(std::move(jni::ToNativeString(env.get(), jniServerCookies.get()))); + NormalizeServerCookies(jni::ToNativeString(env.get(), jniServerCookies.get())); + } // dataField is already cached above. - auto const jniData = - MakeScopedPointer(static_cast(env->GetObjectField(response, dataField)), deleter); + jni::ScopedLocalRef const jniData( + env.get(), static_cast(env->GetObjectField(response, dataField))); CLEAR_AND_RETURN_FALSE_ON_EXCEPTION if (jniData) { diff --git a/android/src/com/mapswithme/util/HttpClient.java b/android/src/com/mapswithme/util/HttpClient.java index ed7d5833a4..3f3510ad97 100644 --- a/android/src/com/mapswithme/util/HttpClient.java +++ b/android/src/com/mapswithme/util/HttpClient.java @@ -24,6 +24,8 @@ package com.mapswithme.util; +import android.support.annotation.NonNull; +import android.text.TextUtils; import android.util.Base64; import android.util.Log; @@ -48,10 +50,10 @@ public final class HttpClient // Globally accessible for faster unit-testing public static int TIMEOUT_IN_MILLISECONDS = 30000; - public static Params run(final Params p) throws IOException, NullPointerException + public static Params run(@NonNull final Params p) throws IOException, NullPointerException { - if (p.httpMethod == null) - throw new NullPointerException("Please set valid HTTP method for request at Params.httpMethod field."); + if (TextUtils.isEmpty(p.httpMethod)) + throw new IllegalArgumentException("Please set valid HTTP method for request at Params.httpMethod field."); HttpURLConnection connection = null; if (p.debugMode) @@ -79,21 +81,21 @@ public final class HttpClient connection.setReadTimeout(TIMEOUT_IN_MILLISECONDS); connection.setUseCaches(false); connection.setRequestMethod(p.httpMethod); - if (p.basicAuthUser != null) + if (!TextUtils.isEmpty(p.basicAuthUser)) { final String encoded = Base64.encodeToString((p.basicAuthUser + ":" + p.basicAuthPassword).getBytes(), Base64.NO_WRAP); connection.setRequestProperty("Authorization", "Basic " + encoded); } - if (p.userAgent != null) + if (!TextUtils.isEmpty(p.userAgent)) connection.setRequestProperty("User-Agent", p.userAgent); - if (p.cookies != null) + if (!TextUtils.isEmpty(p.cookies)) connection.setRequestProperty("Cookie", p.cookies); - if (p.inputFilePath != null || p.data != null) + if (!TextUtils.isEmpty(p.inputFilePath) || p.data != null) { // Send (POST, PUT...) data to the server. - if (p.contentType == null) + if (TextUtils.isEmpty(p.contentType)) throw new NullPointerException("Please set Content-Type for request."); // Work-around for situation when more than one consequent POST requests can lead to stable @@ -101,7 +103,7 @@ public final class HttpClient // The only found reference to this bug is http://stackoverflow.com/a/24303115/1209392 connection.setRequestProperty("Connection", "close"); connection.setRequestProperty("Content-Type", p.contentType); - if (p.contentEncoding != null) + if (!TextUtils.isEmpty(p.contentEncoding)) connection.setRequestProperty("Content-Encoding", p.contentEncoding); connection.setDoOutput(true); @@ -153,19 +155,14 @@ public final class HttpClient final Map> headers = connection.getHeaderFields(); if (headers != null && headers.containsKey("Set-Cookie")) { - p.cookies = ""; - for (final String value : headers.get("Set-Cookie")) - { - // Multiple Set-Cookie headers are normalized in C++ code. - if (value != null) - p.cookies += value + ", "; - } + // Multiple Set-Cookie headers are normalized in C++ code. + android.text.TextUtils.join(", ", headers.get("Set-Cookie")); } // This implementation receives any data only if we have HTTP::OK (200). if (p.httpResponseCode == HttpURLConnection.HTTP_OK) { OutputStream ostream; - if (p.outputFilePath != null) + if (!TextUtils.isEmpty(p.outputFilePath)) ostream = new BufferedOutputStream(new FileOutputStream(p.outputFilePath), STREAM_BUFFER_SIZE); else ostream = new ByteArrayOutputStream(STREAM_BUFFER_SIZE); @@ -196,26 +193,26 @@ public final class HttpClient public static class Params { - public String url = null; + public String url; // Can be different from url in case of redirects. - public String receivedUrl = null; - public String httpMethod = null; + public String receivedUrl; + public String httpMethod; // Should be specified for any request whose method allows non-empty body. // On return, contains received Content-Type or null. - public String contentType = null; + public String contentType; // Can be specified for any request whose method allows non-empty body. // On return, contains received Content-Encoding or null. - public String contentEncoding = null; - public byte[] data = null; + public String contentEncoding; + public byte[] data; // Send from input file if specified instead of data. - public String inputFilePath = null; + public String inputFilePath; // Received data is stored here if not null or in data otherwise. - public String outputFilePath = null; + public String outputFilePath; // Optionally client can override default HTTP User-Agent. - public String userAgent = null; - public String basicAuthUser = null; - public String basicAuthPassword = null; - public String cookies = null; + public String userAgent; + public String basicAuthUser; + public String basicAuthPassword; + public String cookies; public int httpResponseCode = -1; public boolean debugMode = false; public boolean followRedirects = true; diff --git a/editor/config_loader.cpp b/editor/config_loader.cpp index 6bd94f3e58..b5ef7241a4 100644 --- a/editor/config_loader.cpp +++ b/editor/config_loader.cpp @@ -39,9 +39,9 @@ string RunSimpleHttpRequest(string const & url) LOG(LWARNING, ("Exception from HttpClient::RunHttpRequest, message: ", ex.what())); } - if (result && !request.was_redirected() && request.error_code() == 200) // 200 - http status OK + if (result && !request.WasRedirected() && request.ErrorCode() == 200) // 200 - http status OK { - return request.server_response(); + return request.ServerResponse(); } return {}; diff --git a/editor/osm_auth.cpp b/editor/osm_auth.cpp index b85ee31929..b6a27d10b0 100644 --- a/editor/osm_auth.cpp +++ b/editor/osm_auth.cpp @@ -53,18 +53,6 @@ string BuildPostRequest(map const & params) } return result; } - -// TODO(AlexZ): DebugPrint doesn't detect this overload. Fix it. -string DP(HttpClient const & request) -{ - string str = "HTTP " + strings::to_string(request.error_code()) + " url [" + request.url_requested() + "]"; - if (request.was_redirected()) - str += " was redirected to [" + request.url_received() + "]"; - if (!request.server_response().empty()) - str += " response: " + request.server_response(); - return str; -} - } // namespace // static @@ -136,25 +124,25 @@ OsmOAuth::SessionID OsmOAuth::FetchSessionId(string const & subUrl) const HttpClient request(url); if (!request.RunHttpRequest()) MYTHROW(NetworkError, ("FetchSessionId Network error while connecting to", url)); - if (request.was_redirected()) - MYTHROW(UnexpectedRedirect, ("Redirected to", request.url_received(), "from", url)); - if (request.error_code() != HTTP::OK) - MYTHROW(FetchSessionIdError, (DP(request))); + if (request.WasRedirected()) + MYTHROW(UnexpectedRedirect, ("Redirected to", request.UrlReceived(), "from", url)); + if (request.ErrorCode() != HTTP::OK) + MYTHROW(FetchSessionIdError, (DebugPrint(request))); - SessionID const sid = { request.combined_cookies(), FindAuthenticityToken(request.server_response()) }; + SessionID const sid = { request.CombinedCookies(), FindAuthenticityToken(request.ServerResponse()) }; if (sid.m_cookies.empty() || sid.m_token.empty()) - MYTHROW(FetchSessionIdError, ("Cookies and/or token are empty for request", DP(request))); + MYTHROW(FetchSessionIdError, ("Cookies and/or token are empty for request", DebugPrint(request))); return sid; } void OsmOAuth::LogoutUser(SessionID const & sid) const { HttpClient request(m_baseUrl + "/logout"); - request.set_cookies(sid.m_cookies); + request.SetCookies(sid.m_cookies); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("LogoutUser Network error while connecting to", request.url_requested())); - if (request.error_code() != HTTP::OK) - MYTHROW(LogoutUserError, (DP(request))); + MYTHROW(NetworkError, ("LogoutUser Network error while connecting to", request.UrlRequested())); + if (request.ErrorCode() != HTTP::OK) + MYTHROW(LogoutUserError, (DebugPrint(request))); } bool OsmOAuth::LoginUserPassword(string const & login, string const & password, SessionID const & sid) const @@ -168,50 +156,50 @@ bool OsmOAuth::LoginUserPassword(string const & login, string const & password, {"authenticity_token", sid.m_token} }; HttpClient request(m_baseUrl + "/login"); - request.set_body_data(BuildPostRequest(params), "application/x-www-form-urlencoded") - .set_cookies(sid.m_cookies) - .set_handle_redirects(false); + request.SetBodyData(BuildPostRequest(params), "application/x-www-form-urlencoded") + .SetCookies(sid.m_cookies) + .SetHandleRedirects(false); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("LoginUserPassword Network error while connecting to", request.url_requested())); + MYTHROW(NetworkError, ("LoginUserPassword Network error while connecting to", request.UrlRequested())); // At the moment, automatic redirects handling is buggy on Androids < 4.4. // set_handle_redirects(false) works only for Android code, iOS one (and curl) still automatically follow all redirects. - if (request.error_code() != HTTP::OK && request.error_code() != HTTP::Found) - MYTHROW(LoginUserPasswordServerError, (DP(request))); + if (request.ErrorCode() != HTTP::OK && request.ErrorCode() != HTTP::Found) + MYTHROW(LoginUserPasswordServerError, (DebugPrint(request))); // Not redirected page is a 100% signal that login and/or password are invalid. - if (!request.was_redirected()) + if (!request.WasRedirected()) return false; // Check if we were redirected to some 3rd party site. - if (request.url_received().find(m_baseUrl) != 0) - MYTHROW(UnexpectedRedirect, (DP(request))); + if (request.UrlReceived().find(m_baseUrl) != 0) + MYTHROW(UnexpectedRedirect, (DebugPrint(request))); // m_baseUrl + "/login" means login and/or password are invalid. - return request.server_response().find("/login") == string::npos; + return request.ServerResponse().find("/login") == string::npos; } bool OsmOAuth::LoginSocial(string const & callbackPart, string const & socialToken, SessionID const & sid) const { string const url = m_baseUrl + callbackPart + socialToken; HttpClient request(url); - request.set_cookies(sid.m_cookies) - .set_handle_redirects(false); + request.SetCookies(sid.m_cookies) + .SetHandleRedirects(false); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("LoginSocial Network error while connecting to", request.url_requested())); - if (request.error_code() != HTTP::OK && request.error_code() != HTTP::Found) - MYTHROW(LoginSocialServerError, (DP(request))); + MYTHROW(NetworkError, ("LoginSocial Network error while connecting to", request.UrlRequested())); + if (request.ErrorCode() != HTTP::OK && request.ErrorCode() != HTTP::Found) + MYTHROW(LoginSocialServerError, (DebugPrint(request))); // Not redirected page is a 100% signal that social login has failed. - if (!request.was_redirected()) + if (!request.WasRedirected()) return false; // Check if we were redirected to some 3rd party site. - if (request.url_received().find(m_baseUrl) != 0) - MYTHROW(UnexpectedRedirect, (DP(request))); + if (request.UrlReceived().find(m_baseUrl) != 0) + MYTHROW(UnexpectedRedirect, (DebugPrint(request))); // m_baseUrl + "/login" means login and/or password are invalid. - return request.server_response().find("/login") == string::npos; + return request.ServerResponse().find("/login") == string::npos; } // Fakes a buttons press to automatically accept requested permissions. @@ -229,17 +217,17 @@ string OsmOAuth::SendAuthRequest(string const & requestTokenKey, SessionID const {"commit", "Save changes"} }; HttpClient request(m_baseUrl + "/oauth/authorize"); - request.set_body_data(BuildPostRequest(params), "application/x-www-form-urlencoded") - .set_cookies(sid.m_cookies) - .set_handle_redirects(false); + request.SetBodyData(BuildPostRequest(params), "application/x-www-form-urlencoded") + .SetCookies(sid.m_cookies) + .SetHandleRedirects(false); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("SendAuthRequest Network error while connecting to", request.url_requested())); + MYTHROW(NetworkError, ("SendAuthRequest Network error while connecting to", request.UrlRequested())); - string const callbackURL = request.url_received(); + string const callbackURL = request.UrlReceived(); string const vKey = "oauth_verifier="; auto const pos = callbackURL.find(vKey); if (pos == string::npos) - MYTHROW(SendAuthRequestError, ("oauth_verifier is not found", DP(request))); + MYTHROW(SendAuthRequestError, ("oauth_verifier is not found", DebugPrint(request))); auto const end = callbackURL.find("&", pos); return callbackURL.substr(pos + vKey.length(), end == string::npos ? end : end - pos - vKey.length()); @@ -253,14 +241,14 @@ TRequestToken OsmOAuth::FetchRequestToken() const string const requestTokenQuery = oauth.getURLQueryString(OAuth::Http::Get, requestTokenUrl + "?oauth_callback=oob"); HttpClient request(requestTokenUrl + "?" + requestTokenQuery); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("FetchRequestToken Network error while connecting to", request.url_requested())); - if (request.error_code() != HTTP::OK) - MYTHROW(FetchRequestTokenServerError, (DP(request))); - if (request.was_redirected()) - MYTHROW(UnexpectedRedirect, ("Redirected to", request.url_received(), "from", request.url_requested())); + MYTHROW(NetworkError, ("FetchRequestToken Network error while connecting to", request.UrlRequested())); + if (request.ErrorCode() != HTTP::OK) + MYTHROW(FetchRequestTokenServerError, (DebugPrint(request))); + if (request.WasRedirected()) + MYTHROW(UnexpectedRedirect, ("Redirected to", request.UrlReceived(), "from", request.UrlRequested())); // Throws std::runtime_error. - OAuth::Token const reqToken = OAuth::Token::extract(request.server_response()); + OAuth::Token const reqToken = OAuth::Token::extract(request.ServerResponse()); return { reqToken.key(), reqToken.secret() }; } @@ -273,13 +261,13 @@ TKeySecret OsmOAuth::FinishAuthorization(TRequestToken const & requestToken, str string const queryString = oauth.getURLQueryString(OAuth::Http::Get, accessTokenUrl, "", true); HttpClient request(accessTokenUrl + "?" + queryString); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("FinishAuthorization Network error while connecting to", request.url_requested())); - if (request.error_code() != HTTP::OK) - MYTHROW(FinishAuthorizationServerError, (DP(request))); - if (request.was_redirected()) - MYTHROW(UnexpectedRedirect, ("Redirected to", request.url_received(), "from", request.url_requested())); + MYTHROW(NetworkError, ("FinishAuthorization Network error while connecting to", request.UrlRequested())); + if (request.ErrorCode() != HTTP::OK) + MYTHROW(FinishAuthorizationServerError, (DebugPrint(request))); + if (request.WasRedirected()) + MYTHROW(UnexpectedRedirect, ("Redirected to", request.UrlReceived(), "from", request.UrlRequested())); - OAuth::KeyValuePairs const responseData = OAuth::ParseKeyValuePairs(request.server_response()); + OAuth::KeyValuePairs const responseData = OAuth::ParseKeyValuePairs(request.ServerResponse()); // Throws std::runtime_error. OAuth::Token const accessToken = OAuth::Token::extract(responseData); return { accessToken.key(), accessToken.secret() }; @@ -352,15 +340,15 @@ bool OsmOAuth::ResetPassword(string const & email) const {"commit", "Reset password"} }; HttpClient request(m_baseUrl + kForgotPasswordUrlPart); - request.set_body_data(BuildPostRequest(params), "application/x-www-form-urlencoded"); - request.set_cookies(sid.m_cookies); + request.SetBodyData(BuildPostRequest(params), "application/x-www-form-urlencoded"); + request.SetCookies(sid.m_cookies); if (!request.RunHttpRequest()) - MYTHROW(NetworkError, ("ResetPassword Network error while connecting to", request.url_requested())); - if (request.error_code() != HTTP::OK) - MYTHROW(ResetPasswordServerError, (DP(request))); + MYTHROW(NetworkError, ("ResetPassword Network error while connecting to", request.UrlRequested())); + if (request.ErrorCode() != HTTP::OK) + MYTHROW(ResetPasswordServerError, (DebugPrint(request))); - if (request.was_redirected() && request.url_received().find(m_baseUrl) != string::npos) + if (request.WasRedirected() && request.UrlReceived().find(m_baseUrl) != string::npos) return true; return false; } @@ -394,13 +382,13 @@ OsmOAuth::Response OsmOAuth::Request(string const & method, string const & httpM HttpClient request(url + "?" + query); if (httpMethod != "GET") - request.set_body_data(body, "application/xml", httpMethod); + request.SetBodyData(body, "application/xml", httpMethod); if (!request.RunHttpRequest()) MYTHROW(NetworkError, ("Request Network error while connecting to", url)); - if (request.was_redirected()) - MYTHROW(UnexpectedRedirect, ("Redirected to", request.url_received(), "from", url)); + if (request.WasRedirected()) + MYTHROW(UnexpectedRedirect, ("Redirected to", request.UrlReceived(), "from", url)); - return Response(request.error_code(), request.server_response()); + return Response(request.ErrorCode(), request.ServerResponse()); } OsmOAuth::Response OsmOAuth::DirectRequest(string const & method, bool api) const @@ -409,10 +397,10 @@ OsmOAuth::Response OsmOAuth::DirectRequest(string const & method, bool api) cons HttpClient request(url); if (!request.RunHttpRequest()) MYTHROW(NetworkError, ("DirectRequest Network error while connecting to", url)); - if (request.was_redirected()) - MYTHROW(UnexpectedRedirect, ("Redirected to", request.url_received(), "from", url)); + if (request.WasRedirected()) + MYTHROW(UnexpectedRedirect, ("Redirected to", request.UrlReceived(), "from", url)); - return Response(request.error_code(), request.server_response()); + return Response(request.ErrorCode(), request.ServerResponse()); } string DebugPrint(OsmOAuth::Response const & code) diff --git a/editor/user_stats.cpp b/editor/user_stats.cpp index 059a3b35df..3a71ea9884 100644 --- a/editor/user_stats.cpp +++ b/editor/user_stats.cpp @@ -12,8 +12,6 @@ #include "3party/pugixml/src/pugixml.hpp" -using TRequest = platform::HttpClient; - namespace { string const kUserStatsUrl = "https://editor-api.maps.me/user?format=xml"; @@ -91,7 +89,7 @@ bool UserStatsLoader::Update(string const & userName) } auto const url = kUserStatsUrl + "&name=" + UrlEncode(userName); - TRequest request(url); + platform::HttpClient request(url); if (!request.RunHttpRequest()) { @@ -99,13 +97,13 @@ bool UserStatsLoader::Update(string const & userName) return false; } - if (request.error_code() != 200) + if (request.ErrorCode() != 200) { - LOG(LWARNING, ("Server returned", request.error_code(), "for url", url)); + LOG(LWARNING, ("Server returned", request.ErrorCode(), "for url", url)); return false; } - auto const response = request.server_response(); + auto const response = request.ServerResponse(); pugi::xml_document document; if (!document.load_buffer(response.data(), response.size())) diff --git a/platform/http_client.cpp b/platform/http_client.cpp new file mode 100644 index 0000000000..dcd1727b69 --- /dev/null +++ b/platform/http_client.cpp @@ -0,0 +1,165 @@ +#include "platform/http_client.hpp" + +#include "base/string_utils.hpp" + +#include "std/sstream.hpp" + +namespace platform +{ +HttpClient & HttpClient::SetDebugMode(bool debug_mode) +{ + m_debugMode = debug_mode; + return *this; +} + +HttpClient & HttpClient::SetUrlRequested(string const & url) +{ + m_urlRequested = url; + return *this; +} + +HttpClient & HttpClient::SetHttpMethod(string const & method) +{ + m_httpMethod = method; + return *this; +} + +HttpClient & HttpClient::SetBodyFile(string const & body_file, string const & content_type, + string const & http_method /* = "POST" */, + string const & content_encoding /* = "" */) +{ + m_inputFile = body_file; + m_bodyData.clear(); + m_contentType = content_type; + m_httpMethod = http_method; + m_contentEncoding = content_encoding; + return *this; +} + +HttpClient & HttpClient::SetReceivedFile(string const & received_file) +{ + m_outputFile = received_file; + return *this; +} + +HttpClient & HttpClient::SetUserAgent(string const & user_agent) +{ + m_userAgent = user_agent; + return *this; +} + +HttpClient & HttpClient::SetUserAndPassword(string const & user, string const & password) +{ + m_basicAuthUser = user; + m_basicAuthPassword = password; + return *this; +} + +HttpClient & HttpClient::SetCookies(string const & cookies) +{ + m_cookies = cookies; + return *this; +} + +HttpClient & HttpClient::SetHandleRedirects(bool handle_redirects) +{ + m_handleRedirects = handle_redirects; + return *this; +} + +string const & HttpClient::UrlRequested() const +{ + return m_urlRequested; +} + +string const & HttpClient::UrlReceived() const +{ + return m_urlReceived; +} + +bool HttpClient::WasRedirected() const +{ + return m_urlRequested != m_urlReceived; +} + +int HttpClient::ErrorCode() const +{ + return m_errorCode; +} + +string const & HttpClient::ServerResponse() const +{ + return m_serverResponse; +} + +string const & HttpClient::HttpMethod() const +{ + return m_httpMethod; +} + +string HttpClient::CombinedCookies() const +{ + if (m_serverCookies.empty()) + return m_cookies; + + if (m_cookies.empty()) + return m_serverCookies; + + return m_serverCookies + "; " + m_cookies; +} + +string HttpClient::CookieByName(string name) const +{ + string const str = CombinedCookies(); + name += "="; + auto const cookie = str.find(name); + auto const eq = cookie + name.size(); + if (cookie != string::npos && str.size() > eq) + return str.substr(eq, str.find(';', eq) - eq); + + return {}; +} + +// static +string HttpClient::NormalizeServerCookies(string && cookies) +{ + istringstream is(cookies); + string str, result; + + // Split by ", ". Can have invalid tokens here, expires= can also contain a comma. + while (getline(is, str, ',')) + { + size_t const leading = str.find_first_not_of(" "); + if (leading != string::npos) + str.substr(leading).swap(str); + + // In the good case, we have '=' and it goes before any ' '. + auto const eq = str.find('='); + if (eq == string::npos) + continue; // It's not a cookie: no valid key value pair. + + auto const sp = str.find(' '); + if (sp != string::npos && eq > sp) + continue; // It's not a cookie: comma in expires date. + + // Insert delimiter. + if (!result.empty()) + result.append("; "); + + // Read cookie itself. + result.append(str, 0, str.find(";")); + } + return result; +} + +string DebugPrint(HttpClient const & request) +{ + ostringstream ostr; + ostr << "HTTP " << request.ErrorCode() << " url [" << request.UrlRequested() << "]"; + if (request.WasRedirected()) + ostr << " was redirected to [" << request.UrlReceived() << "]"; + if (!request.ServerResponse().empty()) + ostr << " response: " << request.ServerResponse(); + return ostr.str(); +} +} diff --git a/platform/http_client.hpp b/platform/http_client.hpp index ad21a0409e..11f77084d4 100644 --- a/platform/http_client.hpp +++ b/platform/http_client.hpp @@ -25,212 +25,106 @@ SOFTWARE. #include "base/macros.hpp" -#include "std/sstream.hpp" #include "std/string.hpp" namespace platform { class HttpClient { - public: - enum { kNotInitialized = -1 }; +public: + static auto constexpr kNoError = -1; HttpClient() = default; HttpClient(string const & url) : m_urlRequested(url) {} - HttpClient & set_debug_mode(bool debug_mode) - { - m_debugMode = debug_mode; - return *this; - } - HttpClient & set_url_requested(string const & url) - { - m_urlRequested = url; - return *this; - } - HttpClient & set_http_method(string const & method) - { - m_httpMethod = method; - return *this; - } + + // Synchronous (blocking) call, should be implemented for each platform + // @returns true if connection was made and server returned something (200, 404, etc.). + // @note Implementations should transparently support all needed HTTP redirects. + // Implemented for each platform. + bool RunHttpRequest(); + + // Shared methods for all platforms, implemented at http_client.cpp + HttpClient & SetDebugMode(bool debug_mode); + HttpClient & SetUrlRequested(string const & url); + HttpClient & SetHttpMethod(string const & method); // This method is mutually exclusive with set_body_data(). - HttpClient & set_body_file(string const & body_file, - string const & content_type, - string const & http_method = "POST", - string const & content_encoding = "") - { - m_bodyFile = body_file; - m_bodyData.clear(); - m_contentType = content_type; - m_httpMethod = http_method; - m_contentEncoding = content_encoding; - return *this; - } + HttpClient & SetBodyFile(string const & body_file, string const & content_type, + string const & http_method = "POST", + string const & content_encoding = ""); // If set, stores server reply in file specified. - HttpClient & set_received_file(string const & received_file) - { - m_receivedFile = received_file; - return *this; - } - HttpClient & set_user_agent(string const & user_agent) - { - m_userAgent = user_agent; - return *this; - } + HttpClient & SetReceivedFile(string const & received_file); + HttpClient & SetUserAgent(string const & user_agent); // This method is mutually exclusive with set_body_file(). - HttpClient & set_body_data(string const & body_data, - string const & content_type, - string const & http_method = "POST", - string const & content_encoding = "") + template + HttpClient & SetBodyData(StringT && body_data, string const & content_type, + string const & http_method = "POST", + string const & content_encoding = "") { - m_bodyData = body_data; - m_bodyFile.clear(); - m_contentType = content_type; - m_httpMethod = http_method; - m_contentEncoding = content_encoding; - return *this; - } - // Move version to avoid string copying. - // This method is mutually exclusive with set_body_file(). - HttpClient & set_body_data(string && body_data, - string const & content_type, - string const & http_method = "POST", - string const & content_encoding = "") - { - m_bodyData = move(body_data); - m_bodyFile.clear(); + m_bodyData = forward(body_data); + m_inputFile.clear(); m_contentType = content_type; m_httpMethod = http_method; m_contentEncoding = content_encoding; return *this; } // HTTP Basic Auth. - HttpClient & set_user_and_password(string const & user, string const & password) - { - m_basicAuthUser = user; - m_basicAuthPassword = password; - return *this; - } + HttpClient & SetUserAndPassword(string const & user, string const & password); // Set HTTP Cookie header. - HttpClient & set_cookies(string const & cookies) - { - m_cookies = cookies; - return *this; - } + HttpClient & SetCookies(string const & cookies); // When set to true (default), clients never get 3XX codes from servers, redirects are handled automatically. // TODO: "false" is now supported on Android only. - HttpClient & set_handle_redirects(bool handle_redirects) - { - m_handleRedirects = handle_redirects; - return *this; - } + HttpClient & SetHandleRedirects(bool handle_redirects); - // Synchronous (blocking) call, should be implemented for each platform - // @returns true if connection was made and server returned something (200, 404, etc.). - // @note Implementations should transparently support all needed HTTP redirects - bool RunHttpRequest(); - - string const & url_requested() const { return m_urlRequested; } + string const & UrlRequested() const; // @returns empty string in the case of error - string const & url_received() const { return m_urlReceived; } - bool was_redirected() const { return m_urlRequested != m_urlReceived; } + string const & UrlReceived() const; + bool WasRedirected() const; // Mix of HTTP errors (in case of successful connection) and system-dependent error codes, // in the simplest success case use 'if (200 == client.error_code())' // 200 means OK in HTTP - int error_code() const { return m_errorCode; } - string const & server_response() const { return m_serverResponse; } - string const & http_method() const { return m_httpMethod; } + int ErrorCode() const; + string const & ServerResponse() const; + string const & HttpMethod() const; // Pass this getter's value to the set_cookies() method for easier cookies support in the next request. - string combined_cookies() const - { - if (m_serverCookies.empty()) - { - return m_cookies; - } - if (m_cookies.empty()) - { - return m_serverCookies; - } - return m_serverCookies + "; " + m_cookies; - } + string CombinedCookies() const; // Returns cookie value or empty string if it's not present. - string cookie_by_name(string name) const - { - string const str = combined_cookies(); - name += "="; - auto const cookie = str.find(name); - auto const eq = cookie + name.size(); - if (cookie != string::npos && str.size() > eq) - { - return str.substr(eq, str.find(';', eq) - eq); - } - return string(); - } + string CookieByName(string name) const; private: // Internal helper to convert cookies like this: // "first=value1; expires=Mon, 26-Dec-2016 12:12:32 GMT; path=/, second=value2; path=/, third=value3; " // into this: // "first=value1; second=value2; third=value3" - static string normalize_server_cookies(string && cookies) - { - istringstream is(cookies); - string str, result; - // Split by ", ". Can have invalid tokens here, expires= can also contain a comma. - while (getline(is, str, ',')) - { - size_t const leading = str.find_first_not_of(" "); - if (leading != string::npos) - { - str.substr(leading).swap(str); - } - // In the good case, we have '=' and it goes before any ' '. - auto const eq = str.find('='); - if (eq == string::npos) - { - continue; // It's not a cookie: no valid key value pair. - } - auto const sp = str.find(' '); - if (sp != string::npos && eq > sp) - { - continue; // It's not a cookie: comma in expires date. - } - // Insert delimiter. - if (!result.empty()) - { - result.append("; "); - } - // Read cookie itself. - result.append(str, 0, str.find(";")); - } - return result; - } + static string NormalizeServerCookies(string && cookies); - string m_urlRequested; - // Contains final content's url taking redirects (if any) into an account. - string m_urlReceived; - int m_errorCode = kNotInitialized; - string m_bodyFile; - // Used instead of server_reply_ if set. - string m_receivedFile; - // Data we received from the server if output_file_ wasn't initialized. - string m_serverResponse; - string m_contentType; - string m_contentTypeReceived; - string m_contentEncoding; - string m_contentEncodingReceived; - string m_userAgent; - string m_bodyData; - string m_httpMethod = "GET"; - string m_basicAuthUser; - string m_basicAuthPassword; - // All Set-Cookie values from server response combined in a Cookie format: - // cookie1=value1; cookie2=value2 - // TODO(AlexZ): Support encoding and expiration/path/domains etc. - string m_serverCookies; - // Cookies set by the client before request is run. - string m_cookies; - bool m_debugMode = false; - bool m_handleRedirects = true; + string m_urlRequested; + // Contains final content's url taking redirects (if any) into an account. + string m_urlReceived; + int m_errorCode = kNoError; + string m_inputFile; + // Used instead of server_reply_ if set. + string m_outputFile; + // Data we received from the server if output_file_ wasn't initialized. + string m_serverResponse; + string m_contentType; + string m_contentTypeReceived; + string m_contentEncoding; + string m_contentEncodingReceived; + string m_userAgent; + string m_bodyData; + string m_httpMethod = "GET"; + string m_basicAuthUser; + string m_basicAuthPassword; + // All Set-Cookie values from server response combined in a Cookie format: + // cookie1=value1; cookie2=value2 + // TODO(AlexZ): Support encoding and expiration/path/domains etc. + string m_serverCookies; + // Cookies set by the client before request is run. + string m_cookies; + bool m_debugMode = false; + bool m_handleRedirects = true; - DISALLOW_COPY_AND_MOVE(HttpClient); + DISALLOW_COPY_AND_MOVE(HttpClient); }; + +string DebugPrint(HttpClient const & request); } // namespace platform diff --git a/platform/http_client_apple.mm b/platform/http_client_apple.mm index 9f06f6d5af..4d063c08c2 100644 --- a/platform/http_client_apple.mm +++ b/platform/http_client_apple.mm @@ -54,112 +54,105 @@ static const double kTimeoutInSeconds = 24.0; // TODO(AlexZ): Rewrite to use async implementation for better redirects handling and ability to cancel request from destructor. bool HttpClient::RunHttpRequest() { - @autoreleasepool - { - NSMutableURLRequest * request = [NSMutableURLRequest requestWithURL: - [NSURL URLWithString:[NSString stringWithUTF8String:m_urlRequested.c_str()]] - cachePolicy:NSURLRequestReloadIgnoringLocalCacheData timeoutInterval:kTimeoutInSeconds]; - // We handle cookies manually. - request.HTTPShouldHandleCookies = NO; + NSMutableURLRequest * request = [NSMutableURLRequest requestWithURL: + [NSURL URLWithString:[NSString stringWithUTF8String:m_urlRequested.c_str()]] + cachePolicy:NSURLRequestReloadIgnoringLocalCacheData timeoutInterval:kTimeoutInSeconds]; + // We handle cookies manually. + request.HTTPShouldHandleCookies = NO; - request.HTTPMethod = [NSString stringWithUTF8String:m_httpMethod.c_str()]; - if (!m_contentType.empty()) - [request setValue:[NSString stringWithUTF8String:m_contentType.c_str()] forHTTPHeaderField:@"Content-Type"]; + request.HTTPMethod = [NSString stringWithUTF8String:m_httpMethod.c_str()]; + if (!m_contentType.empty()) + [request setValue:[NSString stringWithUTF8String:m_contentType.c_str()] forHTTPHeaderField:@"Content-Type"]; - if (!m_contentEncoding.empty()) - [request setValue:[NSString stringWithUTF8String:m_contentEncoding.c_str()] forHTTPHeaderField:@"Content-Encoding"]; + if (!m_contentEncoding.empty()) + [request setValue:[NSString stringWithUTF8String:m_contentEncoding.c_str()] forHTTPHeaderField:@"Content-Encoding"]; - if (!m_userAgent.empty()) - [request setValue:[NSString stringWithUTF8String:m_userAgent.c_str()] forHTTPHeaderField:@"User-Agent"]; + if (!m_userAgent.empty()) + [request setValue:[NSString stringWithUTF8String:m_userAgent.c_str()] forHTTPHeaderField:@"User-Agent"]; - if (!m_cookies.empty()) - [request setValue:[NSString stringWithUTF8String:m_cookies.c_str()] forHTTPHeaderField:@"Cookie"]; + if (!m_cookies.empty()) + [request setValue:[NSString stringWithUTF8String:m_cookies.c_str()] forHTTPHeaderField:@"Cookie"]; #if (TARGET_OS_IPHONE > 0) - else if (gBrowserUserAgent) - [request setValue:gBrowserUserAgent forHTTPHeaderField:@"User-Agent"]; + else if (gBrowserUserAgent) + [request setValue:gBrowserUserAgent forHTTPHeaderField:@"User-Agent"]; #endif // TARGET_OS_IPHONE - if (!m_basicAuthUser.empty()) - { - NSData * loginAndPassword = [[NSString stringWithUTF8String:(m_basicAuthUser + ":" + m_basicAuthPassword).c_str()] dataUsingEncoding:NSUTF8StringEncoding]; -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" - // base64Encoding selector below was deprecated in iOS 7+, but we still need it to support 5.1+ versions. - [request setValue:[NSString stringWithFormat:@"Basic %@", [loginAndPassword base64Encoding]] forHTTPHeaderField:@"Authorization"]; -#pragma clang diagnostic pop - } + if (!m_basicAuthUser.empty()) + { + NSData * loginAndPassword = [[NSString stringWithUTF8String:(m_basicAuthUser + ":" + m_basicAuthPassword).c_str()] dataUsingEncoding:NSUTF8StringEncoding]; + [request setValue:[NSString stringWithFormat:@"Basic %@", [loginAndPassword base64EncodedStringWithOptions:0]] forHTTPHeaderField:@"Authorization"]; + } - if (!m_bodyData.empty()) - { - request.HTTPBody = [NSData dataWithBytes:m_bodyData.data() length:m_bodyData.size()]; - if (m_debugMode) - LOG(LINFO, ("Uploading buffer of size", m_bodyData.size(), "bytes")); - } - else if (!m_bodyFile.empty()) - { - NSError * err = nil; - NSString * path = [NSString stringWithUTF8String:m_bodyFile.c_str()]; - const unsigned long long file_size = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&err].fileSize; - if (err) - { - m_errorCode = static_cast(err.code); - if (m_debugMode) - LOG(LERROR, ("Error: ", m_errorCode, [err.localizedDescription UTF8String])); - - return false; - } - request.HTTPBodyStream = [NSInputStream inputStreamWithFileAtPath:path]; - [request setValue:[NSString stringWithFormat:@"%llu", file_size] forHTTPHeaderField:@"Content-Length"]; - if (m_debugMode) - LOG(LINFO, ("Uploading file", m_bodyFile, file_size, "bytes")); - } - - NSHTTPURLResponse * response = nil; - NSError * err = nil; - NSData * url_data = [NSURLConnection sendSynchronousRequest:request returningResponse:&response error:&err]; - - if (response) - { - m_errorCode = static_cast(response.statusCode); - m_urlReceived = [response.URL.absoluteString UTF8String]; - - NSString * content = [response.allHeaderFields objectForKey:@"Content-Type"]; - if (content) - m_contentTypeReceived = std::move([content UTF8String]); - - NSString * encoding = [response.allHeaderFields objectForKey:@"Content-Encoding"]; - if (encoding) - m_contentEncodingReceived = std::move([encoding UTF8String]); - - // Apple merges all Set-Cookie fields into one NSDictionary key delimited by commas. - NSString * cookies = [response.allHeaderFields objectForKey:@"Set-Cookie"]; - if (cookies) - m_serverCookies = normalize_server_cookies(std::move([cookies UTF8String])); - - if (url_data) - { - if (m_receivedFile.empty()) - m_serverResponse.assign(reinterpret_cast(url_data.bytes), url_data.length); - else - [url_data writeToFile:[NSString stringWithUTF8String:m_receivedFile.c_str()] atomically:YES]; - - } - return true; - } - // Request has failed if we are here. - // MacOSX/iOS-specific workaround for HTTP 401 error bug. - // @see bit.ly/1TrHlcS for more details. - if (err.code == NSURLErrorUserCancelledAuthentication) - { - m_errorCode = 401; - return true; - } - - m_errorCode = static_cast(err.code); + if (!m_bodyData.empty()) + { + request.HTTPBody = [NSData dataWithBytes:m_bodyData.data() length:m_bodyData.size()]; if (m_debugMode) - LOG(LERROR, ("Error: ", m_errorCode, ':', [err.localizedDescription UTF8String], "while connecting to", m_urlRequested)); + LOG(LINFO, ("Uploading buffer of size", m_bodyData.size(), "bytes")); + } + else if (!m_inputFile.empty()) + { + NSError * err = nil; + NSString * path = [NSString stringWithUTF8String:m_inputFile.c_str()]; + const unsigned long long file_size = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&err].fileSize; + if (err) + { + m_errorCode = static_cast(err.code); + if (m_debugMode) + LOG(LERROR, ("Error: ", m_errorCode, [err.localizedDescription UTF8String])); - return false; - } // @autoreleasepool + return false; + } + request.HTTPBodyStream = [NSInputStream inputStreamWithFileAtPath:path]; + [request setValue:[NSString stringWithFormat:@"%llu", file_size] forHTTPHeaderField:@"Content-Length"]; + if (m_debugMode) + LOG(LINFO, ("Uploading file", m_inputFile, file_size, "bytes")); + } + + NSHTTPURLResponse * response = nil; + NSError * err = nil; + NSData * url_data = [NSURLConnection sendSynchronousRequest:request returningResponse:&response error:&err]; + + if (response) + { + m_errorCode = static_cast(response.statusCode); + m_urlReceived = [response.URL.absoluteString UTF8String]; + + NSString * content = [response.allHeaderFields objectForKey:@"Content-Type"]; + if (content) + m_contentTypeReceived = std::move([content UTF8String]); + + NSString * encoding = [response.allHeaderFields objectForKey:@"Content-Encoding"]; + if (encoding) + m_contentEncodingReceived = std::move([encoding UTF8String]); + + // Apple merges all Set-Cookie fields into one NSDictionary key delimited by commas. + NSString * cookies = [response.allHeaderFields objectForKey:@"Set-Cookie"]; + if (cookies) + m_serverCookies = NormalizeServerCookies(std::move([cookies UTF8String])); + + if (url_data) + { + if (m_outputFile.empty()) + m_serverResponse.assign(reinterpret_cast(url_data.bytes), url_data.length); + else + [url_data writeToFile:[NSString stringWithUTF8String:m_outputFile.c_str()] atomically:YES]; + + } + return true; + } + // Request has failed if we are here. + // MacOSX/iOS-specific workaround for HTTP 401 error bug. + // @see bit.ly/1TrHlcS for more details. + if (err.code == NSURLErrorUserCancelledAuthentication) + { + m_errorCode = 401; + return true; + } + + m_errorCode = static_cast(err.code); + if (m_debugMode) + LOG(LERROR, ("Error: ", m_errorCode, ':', [err.localizedDescription UTF8String], "while connecting to", m_urlRequested)); + + return false; } } // namespace platform diff --git a/platform/http_client_curl.cpp b/platform/http_client_curl.cpp index bdf116a3b1..5f2665e403 100644 --- a/platform/http_client_curl.cpp +++ b/platform/http_client_curl.cpp @@ -53,15 +53,15 @@ DECLARE_EXCEPTION(PipeCallError, RootException); struct ScopedRemoveFile { ScopedRemoveFile() = default; - explicit ScopedRemoveFile(string const & fileName) : m_file(fileName) {} + explicit ScopedRemoveFile(string const & fileName) : m_fileName(fileName) {} ~ScopedRemoveFile() { - if (!m_file.empty()) - std::remove(m_file.c_str()); + if (!m_fileName.empty()) + std::remove(m_fileName.c_str()); } - std::string m_file; + std::string m_fileName; }; static string ReadFileAsString(string const & filePath) @@ -111,9 +111,9 @@ string GetTmpFileName() return GetPlatform().TmpPathForFile(ss.str()); } -typedef vector> HeadersT; +typedef vector> Headers; -HeadersT ParseHeaders(string const & raw) +Headers ParseHeaders(string const & raw) { istringstream stream(raw); HeadersT headers; @@ -130,6 +130,19 @@ HeadersT ParseHeaders(string const & raw) } return headers; } + +bool WriteToFile(string const & fileName, string const & data) +{ + ofstream ofs(fileName); + if(!ofs.is_open()) + { + LOG(LERROR, ("Failed to write into a temporary file.")); + return false; + } + + ofs << data; + return true; +} } // namespace // Used as a test stub for basic HTTP client implementation. // Make sure that you have curl installed in the PATH. @@ -145,7 +158,7 @@ bool HttpClient::RunHttpRequest() ScopedRemoveFile body_deleter; ScopedRemoveFile received_file_deleter; - string cmd = "curl -s -w '%{http_code}' -X " + m_httpMethod + " -D '" + headers_deleter.m_file + "' "; + string cmd = "curl -s -w '%{http_code}' -X " + m_httpMethod + " -D '" + headers_deleter.m_fileName + "' "; if (!m_contentType.empty()) cmd += "-H 'Content-Type: " + m_contentType + "' "; @@ -161,27 +174,25 @@ bool HttpClient::RunHttpRequest() if (!m_bodyData.empty()) { - body_deleter.m_file = GetTmpFileName(); + body_deleter.m_fileName = GetTmpFileName(); // POST body through tmp file to avoid breaking command line. - if (!(ofstream(body_deleter.m_file) << m_bodyData).good()) - { - LOG(LERROR, ("Failed to write into a temporary file.")); + if (!WriteToFile(body_deleter.m_fileName, m_bodyData)) return false; - } + // TODO(AlexZ): Correctly clean up this internal var to avoid client confusion. - m_bodyFile = body_deleter.m_file; + m_inputFile = body_deleter.m_fileName; } // Content-Length is added automatically by curl. - if (!m_bodyFile.empty()) - cmd += "--data-binary '@" + m_bodyFile + "' "; + if (!m_inputFile.empty()) + cmd += "--data-binary '@" + m_inputFile + "' "; // Use temporary file to receive data from server. // If user has specified file name to save data, it is not temporary and is not deleted automatically. - string rfile = m_receivedFile; + string rfile = m_outputFile; if (rfile.empty()) { rfile = GetTmpFileName(); - received_file_deleter.m_file = rfile; + received_file_deleter.m_fileName = rfile; } cmd += "-o " + rfile + strings::to_string(" ") + "'" + m_urlRequested + "'"; @@ -202,7 +213,7 @@ bool HttpClient::RunHttpRequest() return false; } - HeadersT const headers = ParseHeaders(ReadFileAsString(headers_deleter.m_file)); + HeadersT const headers = ParseHeaders(ReadFileAsString(headers_deleter.m_fileName)); for (auto const & header : headers) { if (header.first == "Set-Cookie") @@ -222,14 +233,14 @@ bool HttpClient::RunHttpRequest() m_urlReceived = header.second; } } - m_serverCookies = normalize_server_cookies(move(m_serverCookies)); + m_serverCookies = NormalizeServerCookies(move(m_serverCookies)); if (m_urlReceived.empty()) { m_urlReceived = m_urlRequested; // Load body contents in final request only (skip redirects). // Sometimes server can reply with empty body, and it's ok. - if (m_receivedFile.empty()) + if (m_outputFile.empty()) m_serverResponse = ReadFileAsString(rfile); } else @@ -240,7 +251,7 @@ bool HttpClient::RunHttpRequest() LOG(LINFO, ("HTTP redirect", m_errorCode, "to", m_urlReceived)); HttpClient redirect(m_urlReceived); - redirect.set_cookies(combined_cookies()); + redirect.SetCookies(CombinedCookies()); if (!redirect.RunHttpRequest()) { @@ -248,8 +259,8 @@ bool HttpClient::RunHttpRequest() return false; } - m_errorCode = redirect.error_code(); - m_urlReceived = redirect.url_received(); + m_errorCode = redirect.ErrorCode(); + m_urlReceived = redirect.UrlReceived(); m_serverCookies = move(redirect.m_serverCookies); m_serverResponse = move(redirect.m_serverResponse); m_contentTypeReceived = move(redirect.m_contentTypeReceived); diff --git a/platform/platform.pro b/platform/platform.pro index b623316b02..66f5e60527 100644 --- a/platform/platform.pro +++ b/platform/platform.pro @@ -92,6 +92,7 @@ SOURCES += \ country_file.cpp \ file_logging.cpp \ get_text_by_id.cpp \ + http_client.cpp \ http_request.cpp \ local_country_file.cpp \ local_country_file_utils.cpp \ diff --git a/platform/platform_mac.mm b/platform/platform_mac.mm index 1d509cf925..062e06b64b 100644 --- a/platform/platform_mac.mm +++ b/platform/platform_mac.mm @@ -19,8 +19,6 @@ Platform::Platform() { - @autoreleasepool - { // get resources directory path string const resourcesPath = [[[NSBundle mainBundle] resourcePath] UTF8String]; string const bundlePath = [[[NSBundle mainBundle] bundlePath] UTF8String]; @@ -78,7 +76,6 @@ Platform::Platform() LOG(LDEBUG, ("Writable Directory:", m_writableDir)); LOG(LDEBUG, ("Tmp Directory:", m_tmpDir)); LOG(LDEBUG, ("Settings Directory:", m_settingsDir)); - } // @autoreleasepool } string Platform::UniqueClientId() const diff --git a/routing/online_cross_fetcher.cpp b/routing/online_cross_fetcher.cpp index 27e6c36f0e..f114b985f1 100644 --- a/routing/online_cross_fetcher.cpp +++ b/routing/online_cross_fetcher.cpp @@ -62,9 +62,9 @@ OnlineCrossFetcher::OnlineCrossFetcher(string const & serverURL, ms::LatLon cons void OnlineCrossFetcher::Do() { m_mwmPoints.clear(); - if (m_request.RunHttpRequest() && m_request.error_code() == 200 && !m_request.was_redirected()) - ParseResponse(m_request.server_response(), m_mwmPoints); + if (m_request.RunHttpRequest() && m_request.ErrorCode() == 200 && !m_request.WasRedirected()) + ParseResponse(m_request.ServerResponse(), m_mwmPoints); else - LOG(LWARNING, ("Can't get OSRM server response. Code: ", m_request.error_code())); + LOG(LWARNING, ("Can't get OSRM server response. Code: ", m_request.ErrorCode())); } } // namespace routing diff --git a/xcode/platform/platform.xcodeproj/project.pbxproj b/xcode/platform/platform.xcodeproj/project.pbxproj index 0d9bad508f..e9895b9fb2 100644 --- a/xcode/platform/platform.xcodeproj/project.pbxproj +++ b/xcode/platform/platform.xcodeproj/project.pbxproj @@ -9,6 +9,7 @@ /* Begin PBXBuildFile section */ 3D30587D1D8320E4004AC712 /* http_client.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3D30587B1D8320E4004AC712 /* http_client.hpp */; }; 3D30587F1D880910004AC712 /* http_client_apple.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3D30587E1D880910004AC712 /* http_client_apple.mm */; }; + 3D97F64B1D9C05E800380945 /* http_client.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D97F64A1D9C05E800380945 /* http_client.cpp */; }; 56EB1EDC1C6B6E6C0022D831 /* file_logging.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 56EB1ED81C6B6E6C0022D831 /* file_logging.cpp */; }; 56EB1EDD1C6B6E6C0022D831 /* file_logging.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 56EB1ED91C6B6E6C0022D831 /* file_logging.hpp */; }; 56EB1EDE1C6B6E6C0022D831 /* mwm_traits.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 56EB1EDA1C6B6E6C0022D831 /* mwm_traits.cpp */; }; @@ -99,6 +100,7 @@ /* Begin PBXFileReference section */ 3D30587B1D8320E4004AC712 /* http_client.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = http_client.hpp; sourceTree = ""; }; 3D30587E1D880910004AC712 /* http_client_apple.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = http_client_apple.mm; sourceTree = ""; }; + 3D97F64A1D9C05E800380945 /* http_client.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_client.cpp; sourceTree = ""; }; 56EB1ED81C6B6E6C0022D831 /* file_logging.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = file_logging.cpp; sourceTree = ""; }; 56EB1ED91C6B6E6C0022D831 /* file_logging.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = file_logging.hpp; sourceTree = ""; }; 56EB1EDA1C6B6E6C0022D831 /* mwm_traits.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = mwm_traits.cpp; sourceTree = ""; }; @@ -300,6 +302,7 @@ 6753437A1A3F5CF500A0A8C3 /* platform */ = { isa = PBXGroup; children = ( + 3D97F64A1D9C05E800380945 /* http_client.cpp */, 3D30587E1D880910004AC712 /* http_client_apple.mm */, 3D30587B1D8320E4004AC712 /* http_client.hpp */, 56EB1ED81C6B6E6C0022D831 /* file_logging.cpp */, @@ -559,6 +562,7 @@ 3D30587F1D880910004AC712 /* http_client_apple.mm in Sources */, 67247FFD1C60BD6500EDE56A /* writable_dir_changer.cpp in Sources */, 6741250C1B4C00CC00A3E828 /* local_country_file_utils.cpp in Sources */, + 3D97F64B1D9C05E800380945 /* http_client.cpp in Sources */, 67AB92EA1B7B3E9100AB5194 /* get_text_by_id.cpp in Sources */, 671C62061AE9014C00076BD0 /* measurement_utils.cpp in Sources */, 675343B61A3F5D5A00A0A8C3 /* http_request.cpp in Sources */,