From 9c645f14c2e02538c5c10c42b5b7970d75801dff Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Thu, 23 Sep 2021 20:03:41 +0300 Subject: [PATCH] Review fixes. Signed-off-by: Viktor Govako --- .../maps/widget/placepage/PlacePageView.java | 13 +-- coding/url.cpp | 10 ++- coding/url.hpp | 1 + indexer/editable_map_object.cpp | 89 ++++++++----------- indexer/editable_map_object.hpp | 2 +- 5 files changed, 53 insertions(+), 62 deletions(-) diff --git a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java index 8aa2ac95a3..ea3120717d 100644 --- a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java +++ b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java @@ -1313,19 +1313,19 @@ public class PlacePageView extends NestedScrollViewClickFixed break; case R.id.ll__place_facebook: final String facebookPage = mMapObject.getMetadata(Metadata.MetadataType.FMD_FACEBOOK_PAGE); - items.add("https://m.facebook.com/"+facebookPage); + items.add("https://m.facebook.com/" + facebookPage); break; case R.id.ll__place_instagram: final String instagramPage = mMapObject.getMetadata(Metadata.MetadataType.FMD_INSTAGRAM_PAGE); - items.add("https://instagram.com/"+instagramPage); + items.add("https://instagram.com/" + instagramPage); break; case R.id.ll__place_twitter: final String twitterPage = mMapObject.getMetadata(Metadata.MetadataType.FMD_TWITTER_PAGE); - items.add("https://mobile.twitter.com/"+twitterPage); + items.add("https://mobile.twitter.com/" + twitterPage); break; case R.id.ll__place_vk: final String vkPage = mMapObject.getMetadata(Metadata.MetadataType.FMD_VK_PAGE); - items.add("https://vk.com/"+vkPage); + items.add("https://vk.com/" + vkPage); break; case R.id.ll__place_email: items.add(mTvEmail.getText().toString()); @@ -1340,11 +1340,6 @@ public class PlacePageView extends NestedScrollViewClickFixed items.add(mTvOperator.getText().toString()); break; case R.id.ll__place_wiki: - if (mMapObject == null) - { - LOGGER.e(TAG, "A long click tap on wiki cannot be handled, mMapObject is null!"); - break; - } items.add(mMapObject.getMetadata(Metadata.MetadataType.FMD_WIKIPEDIA)); break; } diff --git a/coding/url.cpp b/coding/url.cpp index 0972d61193..60e889376c 100644 --- a/coding/url.cpp +++ b/coding/url.cpp @@ -192,6 +192,12 @@ Url::Url(std::string const & url) } } +Url Url::FromString(std::string const & url) +{ + bool const hasProtocol = strings::StartsWith(url, "http://") || strings::StartsWith(url, "https://"); + return Url(hasProtocol ? url : "https://" + url); +} + bool Url::Parse(std::string const & url) { // Get url scheme. @@ -265,10 +271,10 @@ string Url::GetWebDomain() const string Url::GetWebPath() const { - // Return everything after domain name + // Return everything after the domain name. auto const found = m_path.find('/'); if (found != string::npos && m_path.size() > found + 1) - return m_path.substr(found+1); + return m_path.substr(found + 1); return {}; } diff --git a/coding/url.hpp b/coding/url.hpp index 06cd979d36..623a68d39d 100644 --- a/coding/url.hpp +++ b/coding/url.hpp @@ -29,6 +29,7 @@ public: using Callback = std::function; explicit Url(std::string const & url); + static Url FromString(std::string const & url); std::string const & GetScheme() const { return m_scheme; } std::string const & GetPath() const { return m_path; } diff --git a/indexer/editable_map_object.cpp b/indexer/editable_map_object.cpp index ba7e9b3fac..76608f991a 100644 --- a/indexer/editable_map_object.cpp +++ b/indexer/editable_map_object.cpp @@ -771,23 +771,17 @@ bool EditableMapObject::ValidateFacebookPage(string const & page) { if (page.empty()) return true; - // Check that facebookPage contains valid username. + // See rules: https://www.facebook.com/help/105399436216001 - if (regex_match(page, regex(R"(^@?[a-zA-Z\d.\-]{5,}$)"))) + static auto const s_fbRegex = regex(R"(^@?[a-zA-Z\d.\-]{5,}$)"); + if (regex_match(page, s_fbRegex)) return true; + if (ValidateWebsite(page)) { - string facebookPageUrl = page; - // Check if HTTP protocol is present - if (!strings::StartsWith(page, "http://") && !strings::StartsWith(page, "https://")) - facebookPageUrl = "https://" + page; - - const url::Url url = url::Url(facebookPageUrl); - const string &domain = strings::MakeLowerCase(url.GetWebDomain()); - // Check Facebook domain name - return strings::EndsWith(domain, "facebook.com") || strings::EndsWith(domain, "fb.com") - || strings::EndsWith(domain, "fb.me") || strings::EndsWith(domain, "facebook.de") - || strings::EndsWith(domain, "facebook.fr"); + string const domain = strings::MakeLowerCase(url::Url::FromString(page).GetWebDomain()); + return (strings::StartsWith(domain, "facebook.") || strings::StartsWith(domain, "fb.") || + domain.find(".facebook.") != string::npos || domain.find(".fb.") != string::npos); } return false; @@ -799,20 +793,14 @@ bool EditableMapObject::ValidateInstagramPage(string const & page) if (page.empty()) return true; - // Check that `page` contains valid username. // Rules took here: https://blog.jstassen.com/2016/03/code-regex-for-instagram-username-and-hashtags/ - if (regex_match(page, regex(R"(^@?[A-Za-z0-9_][A-Za-z0-9_.]{0,28}[A-Za-z0-9_]$)"))) + static auto const s_instaRegex = regex(R"(^@?[A-Za-z0-9_][A-Za-z0-9_.]{0,28}[A-Za-z0-9_]$)"); + if (regex_match(page, s_instaRegex)) return true; + if (ValidateWebsite(page)) { - string instagramPageUrl = page; - // Check if HTTP protocol is present - if (!strings::StartsWith(page, "http://") && !strings::StartsWith(page, "https://")) - instagramPageUrl = "https://" + page; - - const url::Url url = url::Url(instagramPageUrl); - const string &domain = strings::MakeLowerCase(url.GetWebDomain()); - // Check Facebook domain name + string const domain = strings::MakeLowerCase(url::Url::FromString(page).GetWebDomain()); return domain == "instagram.com" || strings::EndsWith(domain, ".instagram.com"); } @@ -824,28 +812,22 @@ bool EditableMapObject::ValidateTwitterPage(string const & page) { if (page.empty()) return true; + if (ValidateWebsite(page)) { - string twitterPageUrl = page; - // Check if HTTP protocol is present - if (!strings::StartsWith(page, "http://") && !strings::StartsWith(page, "https://")) - twitterPageUrl = "https://" + page; - - const url::Url url = url::Url(twitterPageUrl); - const string &domain = strings::MakeLowerCase(url.GetWebDomain()); - // Check Facebook domain name + string const domain = strings::MakeLowerCase(url::Url::FromString(page).GetWebDomain()); return domain == "twitter.com" || strings::EndsWith(domain, ".twitter.com"); } else { - // Check that page contains valid username. // Rules took here: https://stackoverflow.com/q/11361044 - return regex_match(page, regex(R"(^@?[A-Za-z0-9_]{1,15}$)")); + static auto const s_twitterRegex = regex(R"(^@?[A-Za-z0-9_]{1,15}$)"); + return regex_match(page, s_twitterRegex); } } //static -bool EditableMapObject::ValidateVkPage(string const & page) +bool EditableMapObject::ValidateVkPage(string page) { if (page.empty()) return true; @@ -859,25 +841,26 @@ bool EditableMapObject::ValidateVkPage(string const & page) * - contains a period with less than four symbols after it starting with a letter. */ - string vkPageClean = page; - if (vkPageClean.front() == '@') - vkPageClean = vkPageClean.substr(1); + if (page.size() < 5) + return false; - if (vkPageClean.front() == '_' && vkPageClean.back() == '_') return false; - if (regex_match(vkPageClean, regex(R"(^\d\d\d.+$)"))) return false; - if (regex_match(vkPageClean, regex(R"(^[A-Za-z0-9_.]{5,32}$)"))) return true; + if (page.front() == '@') + page = page.substr(1); + if (page.front() == '_' && page.back() == '_') + return false; + + static auto const s_badVkRegex = regex(R"(^\d\d\d.+$)"); + if (regex_match(page, s_badVkRegex)) + return false; + + static auto const s_goodVkRegex = regex(R"(^[A-Za-z0-9_.]{5,32}$)"); + if (regex_match(page, s_goodVkRegex)) + return true; } if (ValidateWebsite(page)) { - string vkPageUrl = page; - // Check if HTTP protocol is present - if (!strings::StartsWith(page, "http://") && !strings::StartsWith(page, "https://")) - vkPageUrl = "https://" + page; - - const url::Url url = url::Url(vkPageUrl); - const string &domain = strings::MakeLowerCase(url.GetWebDomain()); - // Check Facebook domain name + string const domain = strings::MakeLowerCase(url::Url::FromString(page).GetWebDomain()); return domain == "vk.com" || strings::EndsWith(domain, ".vk.com") || domain == "vkontakte.ru" || strings::EndsWith(domain, ".vkontakte.ru"); } @@ -892,7 +875,10 @@ bool EditableMapObject::ValidateEmail(string const & email) return true; if (strings::IsASCIIString(email)) - return regex_match(email, regex(R"([^@\s]+@[a-zA-Z0-9-]+(\.[a-zA-Z0-9-]+)+$)")); + { + static auto const s_emailRegex = regex(R"([^@\s]+@[a-zA-Z0-9-]+(\.[a-zA-Z0-9-]+)+$)"); + return regex_match(email, s_emailRegex); + } if ('@' == email.front() || '@' == email.back()) return false; @@ -945,7 +931,10 @@ bool EditableMapObject::ValidateName(string const & name) return true; if (strings::IsASCIIString(name)) - return regex_match(name, regex(R"(^[ A-Za-z0-9.,?!@#$%&()\-\+:;"'`]+$)")); + { + static auto const s_nameRegex = regex(R"(^[ A-Za-z0-9.,?!@#$%&()\-\+:;"'`]+$)"); + return regex_match(name, s_nameRegex); + } std::wstring_convert, char32_t> converter; diff --git a/indexer/editable_map_object.hpp b/indexer/editable_map_object.hpp index d4e551c408..36ef2e0c14 100644 --- a/indexer/editable_map_object.hpp +++ b/indexer/editable_map_object.hpp @@ -175,7 +175,7 @@ public: static bool ValidateFacebookPage(std::string const & facebookPage); static bool ValidateInstagramPage(std::string const & page); static bool ValidateTwitterPage(std::string const & page); - static bool ValidateVkPage(std::string const & page); + static bool ValidateVkPage(std::string page); static bool ValidateEmail(std::string const & email); static bool ValidateLevel(std::string const & level); static bool ValidateName(std::string const & name);