From 6c8c5bf486757adf67ee756d4a419d0ce53480d2 Mon Sep 17 00:00:00 2001 From: Hemang Manhas Date: Sun, 9 Feb 2025 18:41:05 +0530 Subject: [PATCH 1/4] [API] Allows Parsing and sharing coordinates in links - Replaced coordinate URL instead of om:// format - Updated parser to accept coordinate URL's - Wrote basic unit tests for the parser - added parsing optional zoom functionality - Handled invalid cases : - Out of range Lat,Lon - Special characters'?' &'#' - Trailing and leading slashes - used sscanf for cleaner code - used std :: string_view for efficient memory usage - Updated Test cases & Passed successfully Signed-off-by: Hemang Manhas --- .../app/organicmaps/util/SharingUtils.java | 12 ++-- ge0/ge0_tests/parser_tests.cpp | 65 +++++++++++++++++++ ge0/parser.cpp | 52 +++++++++++++++ 3 files changed, 125 insertions(+), 4 deletions(-) diff --git a/android/app/src/main/java/app/organicmaps/util/SharingUtils.java b/android/app/src/main/java/app/organicmaps/util/SharingUtils.java index 1d1a68db90..c7922be591 100644 --- a/android/app/src/main/java/app/organicmaps/util/SharingUtils.java +++ b/android/app/src/main/java/app/organicmaps/util/SharingUtils.java @@ -122,9 +122,10 @@ public class SharingUtils final String geoUrl = Framework.nativeGetGe0Url(loc.getLatitude(), loc.getLongitude(), Framework .nativeGetDrawScale(), ""); + final String coordUrl = String.format("https://omaps.app/%.5f,%.5f", loc.getLatitude(), loc.getLongitude()); final String httpUrl = Framework.getHttpGe0Url(loc.getLatitude(), loc.getLongitude(), Framework .nativeGetDrawScale(), ""); - final String text = context.getString(R.string.my_position_share_sms, geoUrl, httpUrl); + final String text = context.getString(R.string.my_position_share_sms, httpUrl , coordUrl); intent.putExtra(Intent.EXTRA_TEXT, text); context.startActivity(Intent.createChooser(intent, context.getString(R.string.share))); @@ -142,10 +143,11 @@ public class SharingUtils final String geoUrl = Framework.nativeGetGe0Url(object.getLat(), object.getLon(), object.getScale(), object.getName()); + final String coordUrl = String.format("https://omaps.app/%.5f,%.5f", object.getLat(), object.getLon()); final String httpUrl = Framework.getHttpGe0Url(object.getLat(), object.getLon(), object.getScale(), object.getName()); final String address = TextUtils.isEmpty(object.getAddress()) ? object.getName() : object.getAddress(); - final String text = context.getString(R.string.my_position_share_email, address, geoUrl, httpUrl); + final String text = context.getString(R.string.my_position_share_email, address,httpUrl ,coordUrl ); intent.putExtra(Intent.EXTRA_TEXT, text); context.startActivity(Intent.createChooser(intent, context.getString(R.string.share))); @@ -161,6 +163,8 @@ public class SharingUtils final String geoUrl = Framework.nativeGetGe0Url(bookmark.getLat(), bookmark.getLon(), bookmark.getScale(), bookmark.getName()); + final String coordUrl = String.format("https://omaps.app/%.5f,%.5f", bookmark.getLat(), bookmark.getLon()); + final String httpUrl = Framework.getHttpGe0Url(bookmark.getLat(), bookmark.getLon(), bookmark.getScale(), bookmark.getName()); StringBuilder text = new StringBuilder(); @@ -171,9 +175,9 @@ public class SharingUtils text.append(bookmark.getAddress()); } text.append(UiUtils.NEW_STRING_DELIMITER); - text.append(geoUrl); - text.append(UiUtils.NEW_STRING_DELIMITER); text.append(httpUrl); + text.append(UiUtils.NEW_STRING_DELIMITER); + text.append(coordUrl); intent.putExtra(Intent.EXTRA_TEXT, text.toString()); context.startActivity(Intent.createChooser(intent, context.getString(R.string.share))); diff --git a/ge0/ge0_tests/parser_tests.cpp b/ge0/ge0_tests/parser_tests.cpp index ce82c31531..6865f5f60f 100644 --- a/ge0/ge0_tests/parser_tests.cpp +++ b/ge0/ge0_tests/parser_tests.cpp @@ -312,4 +312,69 @@ UNIT_TEST(OtherPrefixes) TestSuccess("https://omaps.app/Byqqqqqqqq", 45, 0, 4.25, ""); TestFailure("https://omaps.app/Byqqqqqqq"); } +UNIT_TEST(PlainCoordinateUrl_Valid) +{ + + TestSuccess("https://omaps.app/0,0", 0.0, 0.0, 18.0, ""); + TestSuccess("https://omaps.app/-10,10", -10.0, 10.0, 18.0, ""); + TestSuccess("https://omaps.app/10,0.32", 10.0, 0.32, 18.0, ""); + TestSuccess("https://omaps.app/.123,-.456", 0.123, -0.456, 18.0, ""); + + TestSuccess("https://omaps.app/0.00000,0.00000", 0.0, 0.0, 18.0, ""); + TestSuccess("https://omaps.app/45.00000,-90.00000", 45.0, -90.0, 18.0, ""); + TestSuccess("https://omaps.app/-45.12345,179.99999", -45.12345, 179.99999, 18.0, ""); + TestSuccess("https://omaps.app/89.99999,-179.99999", 89.99999, -179.99999, 18.0, ""); + TestSuccess("https://omaps.app/-89.99999,179.99999", -89.99999, 179.99999, 18.0, ""); + + TestSuccess("https://omaps.app/12.34567,76.54321/Place", 12.34567, 76.54321, 18.0, "Place"); + TestSuccess("https://omaps.app/12.34567,76.54321/Place%20Name", 12.34567, 76.54321, 18.0, "Place_Name"); + TestSuccess("https://omaps.app/12.34567,76.54321/My_Poi", 12.34567, 76.54321, 18.0, "My Poi"); + + TestSuccess("https://omaps.app/13.02227,77.76043/", 13.02227, 77.76043, 18.0, ""); + + TestSuccess("https://omaps.app/13.02227,77.76043/Place////", 13.02227, 77.76043, 18.0, "Place"); + TestSuccess("https://omaps.app/13.02227,77.76043////Place////", 13.02227, 77.76043, 18.0, "Place"); + + // Special Character + TestSuccess("https://omaps.app/13.02227,77.76043?foo=bar", 13.02227, 77.76043, 18.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043#section", 13.02227, 77.76043, 18.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043/Place?foo=bar#section", 13.02227, 77.76043, 18.0, "Place"); + + TestSuccess("https://omaps.app/12.34567,76.54321,10.5", 12.34567, 76.54321, 10.5, ""); + TestSuccess("https://omaps.app/12.34567,76.54321,15.75/Place", 12.34567, 76.54321, 15.75, "Place"); + TestSuccess("https://omaps.app/13.02227,77.76043,18.0", 13.02227, 77.76043, 18.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043,20/AnotherPlace", 13.02227, 77.76043, 20.0, "AnotherPlace"); + TestSuccess("https://omaps.app/13.02227,77.76043,17.5/Place///?foo=bar#section", 13.02227, 77.76043, 17.5, "Place"); +} + +UNIT_TEST(PlainCoordinateUrl_Invalid) +{ + TestFailure("https://omaps.app/1302227 77.76043"); + TestFailure("https://omaps.app/1302227-77.76043"); + + TestFailure("https://omaps.app/abc,77.76043"); + TestFailure("https://omaps.app/13.02227,xyz"); + TestFailure("https://omaps.app/13.02227,Infinity"); + + TestFailure("https://omaps.app/13.02227,77.76043,"); + TestFailure("https://omaps.app/13.02227,77.76043,abc"); + TestFailure("https://omaps.app/13.02227,77.76043,100"); + TestFailure("https://omaps.app/13.02227,77.76043,0.5"); + TestFailure("https://omaps.app/13.02227,77.76043,22.5"); + + TestFailure("https://omaps.app/13.02227,77.76043,100,extra"); + TestFailure("https://omaps.app/13.02227,,77.76043"); + TestFailure("https://omaps.app/,"); + TestFailure("https://omaps.app/,/Name"); + TestFailure("https://omaps.app//Name"); + + TestFailure("https://omaps.app/13.02227,77.76043foo"); + TestFailure("https://omaps.app/foo13.02227,77.76043"); + + TestFailure("https://omaps.app/91.00000,0.00000"); + TestFailure("https://omaps.app/-91.00000,0.00000"); + TestFailure("https://omaps.app/0.00000,181.00000"); + TestFailure("https://omaps.app/0.00000,-181.00000"); +} + } // namespace ge0 diff --git a/ge0/parser.cpp b/ge0/parser.cpp index 569c58a38b..8daef78872 100644 --- a/ge0/parser.cpp +++ b/ge0/parser.cpp @@ -50,6 +50,58 @@ bool Ge0Parser::Parse(std::string const & url, Result & result) bool Ge0Parser::ParseAfterPrefix(std::string const & url, size_t from, Result & result) { + std::string_view remaining(url.data() + from, url.size() - from); + size_t posSep = remaining.find_first_of("?#"); + if (posSep != std::string_view::npos) + remaining = remaining.substr(0, posSep); + if (remaining.empty()) + return false; + if (remaining.find(',') != std::string_view::npos) + { + std::string_view coords; + std::string_view name; + size_t slashPos = remaining.find('/'); + if (slashPos == std::string_view::npos) + { + coords = remaining; + } + else + { + coords = remaining.substr(0, slashPos); + name = remaining.substr(slashPos + 1); + while (!name.empty() && name.front() == '/') + name.remove_prefix(1); + while (!name.empty() && name.back() == '/') + name.remove_suffix(1); + } + double lat, lon, zoom; + int pos = 0; + int count = sscanf(coords.data(), " %lf , %lf %n", &lat, &lon, &pos); + if (count == 2 && pos == (int)coords.size()) + { + zoom = 18.0; + } + else + { + pos = 0; + count = sscanf(coords.data(), " %lf , %lf , %lf %n", &lat, &lon, &zoom, &pos); + if (count != 3 || pos != (int)coords.size()) + return false; + } + if (lat < -90.0 || lat > 90.0 || lon < -180.0 || lon > 180.0) + return false; + if (!std::isfinite(zoom) || zoom < 1.0 || zoom > 22.0) + return false; + + result.m_lat = lat; + result.m_lon = lon; + result.m_zoomLevel = zoom; + + if (!name.empty()) + result.m_name = DecodeName(std::string(name)); + return true; + } + size_t const kEncodedZoomAndCoordinatesLength = 10; if (url.size() < from + kEncodedZoomAndCoordinatesLength) return false; -- 2.45.3 From 8e5ea99643b5f88676897a5aa177a7bd74cb06b3 Mon Sep 17 00:00:00 2001 From: Hemang Manhas Date: Tue, 11 Feb 2025 15:48:49 +0530 Subject: [PATCH 2/4] [API] Makes sharing Coordinate URL modular and reusable -tested links Signed-off-by: Hemang Manhas --- .../app/src/main/cpp/app/organicmaps/Framework.cpp | 14 ++++++++++++++ .../src/main/java/app/organicmaps/Framework.java | 7 +++++++ .../java/app/organicmaps/util/SharingUtils.java | 14 ++++++++------ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/android/app/src/main/cpp/app/organicmaps/Framework.cpp b/android/app/src/main/cpp/app/organicmaps/Framework.cpp index 905a6b36fe..445ce4880f 100644 --- a/android/app/src/main/cpp/app/organicmaps/Framework.cpp +++ b/android/app/src/main/cpp/app/organicmaps/Framework.cpp @@ -980,6 +980,20 @@ Java_app_organicmaps_Framework_nativeGetGe0Url(JNIEnv * env, jclass, jdouble lat return jni::ToJavaString(env, url); } +JNIEXPORT jstring JNICALL +Java_app_organicmaps_Framework_nativeGetCoordUrl(JNIEnv * env, jclass, jdouble lat, jdouble lon, jdouble zoomLevel, jstring name) +{ + ::Framework * fr = frm(); + double const scale = (zoomLevel > 0 ? zoomLevel : fr->GetDrawScale()); + std::string nameStr = jni::ToNativeString(env, name); + char buf[256]; + if (!nameStr.empty()) + snprintf(buf, sizeof(buf), "https://omaps.app/%.5f,%.5f,%.0f/%s", lat, lon, scale, nameStr.c_str()); + else + snprintf(buf, sizeof(buf), "https://omaps.app/%.5f,%.5f,%.0f", lat, lon, scale); + return jni::ToJavaString(env, buf); +} + JNIEXPORT jstring JNICALL Java_app_organicmaps_Framework_nativeGetGeoUri(JNIEnv * env, jclass, jdouble lat, jdouble lon, jdouble zoomLevel, jstring name) { diff --git a/android/app/src/main/java/app/organicmaps/Framework.java b/android/app/src/main/java/app/organicmaps/Framework.java index 349431f208..8b61ce055e 100644 --- a/android/app/src/main/java/app/organicmaps/Framework.java +++ b/android/app/src/main/java/app/organicmaps/Framework.java @@ -149,6 +149,10 @@ public class Framework return nativeGetGe0Url(lat, lon, zoomLevel, name).replaceFirst( Constants.Url.SHORT_SHARE_PREFIX, Constants.Url.HTTP_SHARE_PREFIX); } + public static String getCoordUrl(double lat, double lon, double zoomLevel, String name) + { + return nativeGetCoordUrl(lat, lon, zoomLevel, name); + } /** * Generates Bitmap with route altitude image chart taking into account current map style. @@ -196,6 +200,9 @@ public class Framework public static native String nativeFormatSpeed(double speed); public static native String nativeGetGe0Url(double lat, double lon, double zoomLevel, String name); + + public static native String nativeGetCoordUrl(double lat, double lon, double zoomLevel, String name); + public static native String nativeGetGeoUri(double lat, double lon, double zoomLevel, String name); public static native String nativeGetAddress(double lat, double lon); diff --git a/android/app/src/main/java/app/organicmaps/util/SharingUtils.java b/android/app/src/main/java/app/organicmaps/util/SharingUtils.java index c7922be591..687437e318 100644 --- a/android/app/src/main/java/app/organicmaps/util/SharingUtils.java +++ b/android/app/src/main/java/app/organicmaps/util/SharingUtils.java @@ -122,10 +122,11 @@ public class SharingUtils final String geoUrl = Framework.nativeGetGe0Url(loc.getLatitude(), loc.getLongitude(), Framework .nativeGetDrawScale(), ""); - final String coordUrl = String.format("https://omaps.app/%.5f,%.5f", loc.getLatitude(), loc.getLongitude()); + final String coordUrl = Framework.getCoordUrl(loc.getLatitude(), loc.getLongitude(), Framework + .nativeGetDrawScale(), ""); final String httpUrl = Framework.getHttpGe0Url(loc.getLatitude(), loc.getLongitude(), Framework .nativeGetDrawScale(), ""); - final String text = context.getString(R.string.my_position_share_sms, httpUrl , coordUrl); + final String text = context.getString(R.string.my_position_share_sms, httpUrl, coordUrl); intent.putExtra(Intent.EXTRA_TEXT, text); context.startActivity(Intent.createChooser(intent, context.getString(R.string.share))); @@ -143,11 +144,12 @@ public class SharingUtils final String geoUrl = Framework.nativeGetGe0Url(object.getLat(), object.getLon(), object.getScale(), object.getName()); - final String coordUrl = String.format("https://omaps.app/%.5f,%.5f", object.getLat(), object.getLon()); + final String coordUrl = Framework.getCoordUrl(object.getLat(), object.getLon(), + object.getScale(), object.getName()); final String httpUrl = Framework.getHttpGe0Url(object.getLat(), object.getLon(), object.getScale(), object.getName()); final String address = TextUtils.isEmpty(object.getAddress()) ? object.getName() : object.getAddress(); - final String text = context.getString(R.string.my_position_share_email, address,httpUrl ,coordUrl ); + final String text = context.getString(R.string.my_position_share_email, address, httpUrl, coordUrl); intent.putExtra(Intent.EXTRA_TEXT, text); context.startActivity(Intent.createChooser(intent, context.getString(R.string.share))); @@ -163,8 +165,8 @@ public class SharingUtils final String geoUrl = Framework.nativeGetGe0Url(bookmark.getLat(), bookmark.getLon(), bookmark.getScale(), bookmark.getName()); - final String coordUrl = String.format("https://omaps.app/%.5f,%.5f", bookmark.getLat(), bookmark.getLon()); - + final String coordUrl = Framework.getCoordUrl(bookmark.getLat(), bookmark.getLon(), + bookmark.getScale(), bookmark.getName()); final String httpUrl = Framework.getHttpGe0Url(bookmark.getLat(), bookmark.getLon(), bookmark.getScale(), bookmark.getName()); StringBuilder text = new StringBuilder(); -- 2.45.3 From f1f49946380a02b2b43896380fc5fb5d7be7aea7 Mon Sep 17 00:00:00 2001 From: hemanggs Date: Mon, 3 Mar 2025 17:18:03 +0530 Subject: [PATCH 3/4] [API] Made suggested changes Signed-off-by: hemanggs --- .../main/cpp/app/organicmaps/Framework.cpp | 10 ++-- ge0/ge0_tests/parser_tests.cpp | 54 ++++++++++--------- ge0/ge0_tests/url_generator_tests.cpp | 36 +++++++++++++ ge0/parser.cpp | 48 ++++++++++++++--- ge0/parser.hpp | 2 + ge0/url_generator.cpp | 18 +++++++ ge0/url_generator.hpp | 13 +++++ 7 files changed, 141 insertions(+), 40 deletions(-) diff --git a/android/app/src/main/cpp/app/organicmaps/Framework.cpp b/android/app/src/main/cpp/app/organicmaps/Framework.cpp index 445ce4880f..1e9de95634 100644 --- a/android/app/src/main/cpp/app/organicmaps/Framework.cpp +++ b/android/app/src/main/cpp/app/organicmaps/Framework.cpp @@ -986,12 +986,10 @@ Java_app_organicmaps_Framework_nativeGetCoordUrl(JNIEnv * env, jclass, jdouble l ::Framework * fr = frm(); double const scale = (zoomLevel > 0 ? zoomLevel : fr->GetDrawScale()); std::string nameStr = jni::ToNativeString(env, name); - char buf[256]; - if (!nameStr.empty()) - snprintf(buf, sizeof(buf), "https://omaps.app/%.5f,%.5f,%.0f/%s", lat, lon, scale, nameStr.c_str()); - else - snprintf(buf, sizeof(buf), "https://omaps.app/%.5f,%.5f,%.0f", lat, lon, scale); - return jni::ToJavaString(env, buf); + static const char kHttpSharePrefix[] = "https://omaps.app/"; + string const url = ge0::GenerateCoordUrl(lat, lon, scale, nameStr); + return jni::ToJavaString(env, url); + } JNIEXPORT jstring JNICALL diff --git a/ge0/ge0_tests/parser_tests.cpp b/ge0/ge0_tests/parser_tests.cpp index 6865f5f60f..285ca0d835 100644 --- a/ge0/ge0_tests/parser_tests.cpp +++ b/ge0/ge0_tests/parser_tests.cpp @@ -315,36 +315,45 @@ UNIT_TEST(OtherPrefixes) UNIT_TEST(PlainCoordinateUrl_Valid) { - TestSuccess("https://omaps.app/0,0", 0.0, 0.0, 18.0, ""); - TestSuccess("https://omaps.app/-10,10", -10.0, 10.0, 18.0, ""); - TestSuccess("https://omaps.app/10,0.32", 10.0, 0.32, 18.0, ""); - TestSuccess("https://omaps.app/.123,-.456", 0.123, -0.456, 18.0, ""); + TestSuccess("https://omaps.app/0,0", 0.0, 0.0, 17.0, ""); + TestSuccess("https://omaps.app/-10,10", -10.0, 10.0, 17.0, ""); + TestSuccess("https://omaps.app/10,0.32", 10.0, 0.32, 17.0, ""); + TestSuccess("https://omaps.app/.123,-.456", 0.123, -0.456, 17.0, ""); - TestSuccess("https://omaps.app/0.00000,0.00000", 0.0, 0.0, 18.0, ""); - TestSuccess("https://omaps.app/45.00000,-90.00000", 45.0, -90.0, 18.0, ""); - TestSuccess("https://omaps.app/-45.12345,179.99999", -45.12345, 179.99999, 18.0, ""); - TestSuccess("https://omaps.app/89.99999,-179.99999", 89.99999, -179.99999, 18.0, ""); - TestSuccess("https://omaps.app/-89.99999,179.99999", -89.99999, 179.99999, 18.0, ""); + TestSuccess("https://omaps.app/0.00000,0.00000", 0.0, 0.0, 17.0, ""); + TestSuccess("https://omaps.app/45.00000,-90.00000", 45.0, -90.0, 17.0, ""); + TestSuccess("https://omaps.app/-45.12345,179.99999", -45.12345, 179.99999, 17.0, ""); + TestSuccess("https://omaps.app/89.99999,-179.99999", 89.99999, -179.99999, 17.0, ""); + TestSuccess("https://omaps.app/-89.99999,179.99999", -89.99999, 179.99999, 17.0, ""); - TestSuccess("https://omaps.app/12.34567,76.54321/Place", 12.34567, 76.54321, 18.0, "Place"); - TestSuccess("https://omaps.app/12.34567,76.54321/Place%20Name", 12.34567, 76.54321, 18.0, "Place_Name"); - TestSuccess("https://omaps.app/12.34567,76.54321/My_Poi", 12.34567, 76.54321, 18.0, "My Poi"); + TestSuccess("https://omaps.app/12.34567,76.54321/Place", 12.34567, 76.54321, 17.0, "Place"); + TestSuccess("https://omaps.app/12.34567,76.54321/Place%20Name", 12.34567, 76.54321, 17.0, "Place_Name"); + TestSuccess("https://omaps.app/12.34567,76.54321/Place Name", 12.34567, 76.54321, 17.0, "Place_Name"); // Whitespace + TestSuccess("https://omaps.app/12.34567,76.54321/My_Poi", 12.34567, 76.54321, 17.0, "My Poi"); - TestSuccess("https://omaps.app/13.02227,77.76043/", 13.02227, 77.76043, 18.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043/", 13.02227, 77.76043, 17.0, ""); - TestSuccess("https://omaps.app/13.02227,77.76043/Place////", 13.02227, 77.76043, 18.0, "Place"); - TestSuccess("https://omaps.app/13.02227,77.76043////Place////", 13.02227, 77.76043, 18.0, "Place"); + TestSuccess("https://omaps.app/13.02227,77.76043/Place////", 13.02227, 77.76043, 17.0, "Place"); + TestSuccess("https://omaps.app/13.02227,77.76043////Place////", 13.02227, 77.76043, 17.0, "Place"); // Special Character - TestSuccess("https://omaps.app/13.02227,77.76043?foo=bar", 13.02227, 77.76043, 18.0, ""); - TestSuccess("https://omaps.app/13.02227,77.76043#section", 13.02227, 77.76043, 18.0, ""); - TestSuccess("https://omaps.app/13.02227,77.76043/Place?foo=bar#section", 13.02227, 77.76043, 18.0, "Place"); + TestSuccess("https://omaps.app/13.02227,77.76043?foo=bar", 13.02227, 77.76043, 17.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043#section", 13.02227, 77.76043, 17.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043/Place?foo=bar#section", 13.02227, 77.76043, 17.0, "Place"); TestSuccess("https://omaps.app/12.34567,76.54321,10.5", 12.34567, 76.54321, 10.5, ""); TestSuccess("https://omaps.app/12.34567,76.54321,15.75/Place", 12.34567, 76.54321, 15.75, "Place"); TestSuccess("https://omaps.app/13.02227,77.76043,18.0", 13.02227, 77.76043, 18.0, ""); TestSuccess("https://omaps.app/13.02227,77.76043,20/AnotherPlace", 13.02227, 77.76043, 20.0, "AnotherPlace"); TestSuccess("https://omaps.app/13.02227,77.76043,17.5/Place///?foo=bar#section", 13.02227, 77.76043, 17.5, "Place"); + + // Out of range clamping + TestSuccess("https://omaps.app/13.02227,77.76043,100", 13.02227, 77.76043, 20.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043,0.5", 13.02227, 77.76043, 1.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043,22.5", 13.02227, 77.76043, 20.0, ""); + + TestSuccess("https://omaps.app/13.02227,77.76043,abc", 13.02227, 77.76043, 17.0, ""); + } UNIT_TEST(PlainCoordinateUrl_Invalid) @@ -355,14 +364,7 @@ UNIT_TEST(PlainCoordinateUrl_Invalid) TestFailure("https://omaps.app/abc,77.76043"); TestFailure("https://omaps.app/13.02227,xyz"); TestFailure("https://omaps.app/13.02227,Infinity"); - - TestFailure("https://omaps.app/13.02227,77.76043,"); - TestFailure("https://omaps.app/13.02227,77.76043,abc"); - TestFailure("https://omaps.app/13.02227,77.76043,100"); - TestFailure("https://omaps.app/13.02227,77.76043,0.5"); - TestFailure("https://omaps.app/13.02227,77.76043,22.5"); - - TestFailure("https://omaps.app/13.02227,77.76043,100,extra"); + TestFailure("https://omaps.app/13.02227,77.76043,100,extra"); TestFailure("https://omaps.app/13.02227,,77.76043"); TestFailure("https://omaps.app/,"); TestFailure("https://omaps.app/,/Name"); diff --git a/ge0/ge0_tests/url_generator_tests.cpp b/ge0/ge0_tests/url_generator_tests.cpp index 2cc6af79c9..831727ea8f 100644 --- a/ge0/ge0_tests/url_generator_tests.cpp +++ b/ge0/ge0_tests/url_generator_tests.cpp @@ -342,6 +342,42 @@ UNIT_TEST(GenerateShortShowMapUrl_UnicodeMixedWithOtherChars) TEST_EQUAL("om://8wAAAAAAAA/Back%20in_\xe2\x98\x84%21\xd1\x8e\xd0\xbc", res, ()); } +UNIT_TEST(GenerateCoordUrl_SmokeTest) +{ + string res = GenerateCoordUrl(0, 0, 19, "Name"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Name", res, ()); +} + +UNIT_TEST(GenerateCoordUrl_NameIsEmpty) +{ + string res = GenerateCoordUrl(0, 0, 19, ""); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19", res, ()); +} + +UNIT_TEST(GenerateCoordUrl_SpaceIsReplacedWithUnderscore) +{ + string res = GenerateCoordUrl(0, 0, 19, "Hello World"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello_World", res, ()); +} + +UNIT_TEST(GenerateCoordUrl_UnderscoreIsReplacedWithSpaceEncoded) +{ + string res = GenerateCoordUrl(0, 0, 19, "Hello_World"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello%20World", res, ()); +} + +UNIT_TEST(GenerateCoordUrl_ControlCharsAreEscaped) +{ + string res = GenerateCoordUrl(0, 0, 19, "Hello\tWorld\n"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello%09World%0A", res, ()); +} + +UNIT_TEST(GenerateCoordUrl_FractionalZoom) +{ + string res = GenerateCoordUrl(10, 20, 8.25, "Name"); + TEST_EQUAL("https://omaps.app/10.00000,20.00000,8/Name", res, ()); +} + UNIT_TEST(GenerateGeoUri_SmokeTest) { string res = GenerateGeoUri(33.8904075, 35.5066454, 16.5, "Falafel M. Sahyoun"); diff --git a/ge0/parser.cpp b/ge0/parser.cpp index 8daef78872..266a84e71b 100644 --- a/ge0/parser.cpp +++ b/ge0/parser.cpp @@ -12,6 +12,7 @@ #include #include +#include namespace ge0 { @@ -79,20 +80,51 @@ bool Ge0Parser::ParseAfterPrefix(std::string const & url, size_t from, Result & int count = sscanf(coords.data(), " %lf , %lf %n", &lat, &lon, &pos); if (count == 2 && pos == (int)coords.size()) { - zoom = 18.0; + zoom = scales::GetUpperComfortScale(); } else { - pos = 0; - count = sscanf(coords.data(), " %lf , %lf , %lf %n", &lat, &lon, &zoom, &pos); - if (count != 3 || pos != (int)coords.size()) + int commaCount = std::count(coords.begin(), coords.end(), ','); + if (commaCount == 1) + { + pos = 0; + count = sscanf(coords.data(), " %lf , %lf %n", &lat, &lon, &pos); + if (count != 2 || pos != (int)coords.size()) + return false; + zoom = scales::GetUpperComfortScale(); + } + else if (commaCount == 2) + { + pos = 0; + count = sscanf(coords.data(), " %lf , %lf , %lf %n", &lat, &lon, &zoom, &pos); + if (count == 3 && pos == (int)coords.size()) + { + // Parsed zoom successfully. + } + else + { + pos = 0; + count = sscanf(coords.data(), " %lf , %lf %n", &lat, &lon, &pos); + if (count == 2) + zoom = scales::GetUpperComfortScale(); + else + return false; + } + } + else + { return false; + } } - if (lat < -90.0 || lat > 90.0 || lon < -180.0 || lon > 180.0) + + if (lat < -kLatAbsLimit || lat > kLatAbsLimit || lon < -kLonAbsLimit || lon > kLonAbsLimit) return false; - if (!std::isfinite(zoom) || zoom < 1.0 || zoom > 22.0) - return false; - + if (!std::isfinite(zoom)) + zoom = scales::GetUpperStyleScale(); + if (zoom < 1.0) + zoom = 1.0; + if (zoom > scales::GetUpperStyleScale()) + zoom = scales::GetUpperStyleScale(); result.m_lat = lat; result.m_lon = lon; result.m_zoomLevel = zoom; diff --git a/ge0/parser.hpp b/ge0/parser.hpp index 7c0dbc2414..09fab6593c 100644 --- a/ge0/parser.hpp +++ b/ge0/parser.hpp @@ -30,6 +30,8 @@ public: bool ParseAfterPrefix(std::string const & url, size_t from, Result & result); protected: + static constexpr double kLatAbsLimit = 90.0; + static constexpr double kLonAbsLimit = 180.0; uint8_t DecodeBase64Char(char const c); static double DecodeZoom(uint8_t const zoomByte); bool DecodeLatLon(std::string const & s, double & lat, double & lon); diff --git a/ge0/url_generator.cpp b/ge0/url_generator.cpp index afbf052220..df05977c74 100644 --- a/ge0/url_generator.cpp +++ b/ge0/url_generator.cpp @@ -103,6 +103,24 @@ std::string GenerateShortShowMapUrl(double lat, double lon, double zoom, std::st return urlSample; } +std::string GenerateCoordUrl(double lat, double lon, double zoom, std::string const & name) +{ + static const std::string kHttpSharePrefix = "https://omaps.app/"; + std::string url = kHttpSharePrefix; + + char buf[64]; + snprintf(buf, sizeof(buf), "%.5f,%.5f,%.0f", lat, lon, zoom); + url.append(buf); + + if (!name.empty()) + { + url.push_back('/'); + url.append(UrlEncodeString(TransformName(name))); + } + + return url; +} + std::string GenerateGeoUri(double lat, double lon, double zoom, std::string const & name) { std::ostringstream oss; diff --git a/ge0/url_generator.hpp b/ge0/url_generator.hpp index 2b0047cec1..e576b2d625 100644 --- a/ge0/url_generator.hpp +++ b/ge0/url_generator.hpp @@ -19,6 +19,19 @@ inline static int const kMaxCoordBits = kMaxPointBytes * 3; // om://ZCoordba64/Name std::string GenerateShortShowMapUrl(double lat, double lon, double zoomLevel, std::string const & name); +// Generates a coordinate URL. +// +// URL format: +// +// +-------------------------------- lat +// | +-------------------- lon +// | | +---- zoom +// | | | +-- url-encoded name +// | | | | +// | | | | +// https://omaps.app/54.683486138,25.289361259,14/ +std::string GenerateCoordUrl(double lat, double lon, double zoom, std::string const & name); + // Generates a geo: uri. // // - https://datatracker.ietf.org/doc/html/rfc5870 -- 2.45.3 From b3d5a6a23d99b805087b0ca00df445574b7b745b Mon Sep 17 00:00:00 2001 From: hemanggs Date: Tue, 4 Mar 2025 09:25:33 +0530 Subject: [PATCH 4/4] [API] Suggested changes Signed-off-by: hemanggs --- .../main/cpp/app/organicmaps/Framework.cpp | 5 +- .../main/java/app/organicmaps/Framework.java | 4 -- .../app/organicmaps/util/SharingUtils.java | 16 ++--- ge0/ge0_tests/parser_tests.cpp | 3 + ge0/ge0_tests/url_generator_tests.cpp | 58 ++++++++----------- ge0/url_generator.cpp | 10 ++-- 6 files changed, 40 insertions(+), 56 deletions(-) diff --git a/android/app/src/main/cpp/app/organicmaps/Framework.cpp b/android/app/src/main/cpp/app/organicmaps/Framework.cpp index 1e9de95634..e64c6678ab 100644 --- a/android/app/src/main/cpp/app/organicmaps/Framework.cpp +++ b/android/app/src/main/cpp/app/organicmaps/Framework.cpp @@ -985,9 +985,8 @@ Java_app_organicmaps_Framework_nativeGetCoordUrl(JNIEnv * env, jclass, jdouble l { ::Framework * fr = frm(); double const scale = (zoomLevel > 0 ? zoomLevel : fr->GetDrawScale()); - std::string nameStr = jni::ToNativeString(env, name); - static const char kHttpSharePrefix[] = "https://omaps.app/"; - string const url = ge0::GenerateCoordUrl(lat, lon, scale, nameStr); + std::string const nameStr = jni::ToNativeString(env, name); + std::string const url = ge0::GenerateCoordUrl(lat, lon, scale, nameStr); return jni::ToJavaString(env, url); } diff --git a/android/app/src/main/java/app/organicmaps/Framework.java b/android/app/src/main/java/app/organicmaps/Framework.java index 8b61ce055e..b92422adfa 100644 --- a/android/app/src/main/java/app/organicmaps/Framework.java +++ b/android/app/src/main/java/app/organicmaps/Framework.java @@ -149,10 +149,6 @@ public class Framework return nativeGetGe0Url(lat, lon, zoomLevel, name).replaceFirst( Constants.Url.SHORT_SHARE_PREFIX, Constants.Url.HTTP_SHARE_PREFIX); } - public static String getCoordUrl(double lat, double lon, double zoomLevel, String name) - { - return nativeGetCoordUrl(lat, lon, zoomLevel, name); - } /** * Generates Bitmap with route altitude image chart taking into account current map style. diff --git a/android/app/src/main/java/app/organicmaps/util/SharingUtils.java b/android/app/src/main/java/app/organicmaps/util/SharingUtils.java index 687437e318..5ec3505165 100644 --- a/android/app/src/main/java/app/organicmaps/util/SharingUtils.java +++ b/android/app/src/main/java/app/organicmaps/util/SharingUtils.java @@ -120,9 +120,7 @@ public class SharingUtils final String subject = context.getString(R.string.share); intent.putExtra(Intent.EXTRA_SUBJECT, subject); - final String geoUrl = Framework.nativeGetGe0Url(loc.getLatitude(), loc.getLongitude(), Framework - .nativeGetDrawScale(), ""); - final String coordUrl = Framework.getCoordUrl(loc.getLatitude(), loc.getLongitude(), Framework + final String coordUrl = Framework.nativeGetCoordUrl(loc.getLatitude(), loc.getLongitude(), Framework .nativeGetDrawScale(), ""); final String httpUrl = Framework.getHttpGe0Url(loc.getLatitude(), loc.getLongitude(), Framework .nativeGetDrawScale(), ""); @@ -142,10 +140,8 @@ public class SharingUtils context.getString(R.string.bookmark_share_email_subject); intent.putExtra(Intent.EXTRA_SUBJECT, subject); - final String geoUrl = Framework.nativeGetGe0Url(object.getLat(), object.getLon(), - object.getScale(), object.getName()); - final String coordUrl = Framework.getCoordUrl(object.getLat(), object.getLon(), - object.getScale(), object.getName()); + final String coordUrl = Framework.nativeGetCoordUrl(object.getLat(), object.getLon(), + object.getScale(), object.getName()); final String httpUrl = Framework.getHttpGe0Url(object.getLat(), object.getLon(), object.getScale(), object.getName()); final String address = TextUtils.isEmpty(object.getAddress()) ? object.getName() : object.getAddress(); @@ -163,10 +159,8 @@ public class SharingUtils final String subject = context.getString(R.string.bookmark_share_email_subject); intent.putExtra(Intent.EXTRA_SUBJECT, subject); - final String geoUrl = Framework.nativeGetGe0Url(bookmark.getLat(), bookmark.getLon(), - bookmark.getScale(), bookmark.getName()); - final String coordUrl = Framework.getCoordUrl(bookmark.getLat(), bookmark.getLon(), - bookmark.getScale(), bookmark.getName()); + final String coordUrl = Framework.nativeGetCoordUrl(bookmark.getLat(), bookmark.getLon(), + bookmark.getScale(), bookmark.getName()); final String httpUrl = Framework.getHttpGe0Url(bookmark.getLat(), bookmark.getLon(), bookmark.getScale(), bookmark.getName()); StringBuilder text = new StringBuilder(); diff --git a/ge0/ge0_tests/parser_tests.cpp b/ge0/ge0_tests/parser_tests.cpp index 285ca0d835..308f8cabb8 100644 --- a/ge0/ge0_tests/parser_tests.cpp +++ b/ge0/ge0_tests/parser_tests.cpp @@ -344,12 +344,15 @@ UNIT_TEST(PlainCoordinateUrl_Valid) TestSuccess("https://omaps.app/12.34567,76.54321,10.5", 12.34567, 76.54321, 10.5, ""); TestSuccess("https://omaps.app/12.34567,76.54321,15.75/Place", 12.34567, 76.54321, 15.75, "Place"); TestSuccess("https://omaps.app/13.02227,77.76043,18.0", 13.02227, 77.76043, 18.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043,18.0#", 13.02227, 77.76043, 18.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043,18.0?", 13.02227, 77.76043, 18.0, ""); TestSuccess("https://omaps.app/13.02227,77.76043,20/AnotherPlace", 13.02227, 77.76043, 20.0, "AnotherPlace"); TestSuccess("https://omaps.app/13.02227,77.76043,17.5/Place///?foo=bar#section", 13.02227, 77.76043, 17.5, "Place"); // Out of range clamping TestSuccess("https://omaps.app/13.02227,77.76043,100", 13.02227, 77.76043, 20.0, ""); TestSuccess("https://omaps.app/13.02227,77.76043,0.5", 13.02227, 77.76043, 1.0, ""); + TestSuccess("https://omaps.app/13.02227,77.76043,-0.5", 13.02227, 77.76043, 1.0, ""); TestSuccess("https://omaps.app/13.02227,77.76043,22.5", 13.02227, 77.76043, 20.0, ""); TestSuccess("https://omaps.app/13.02227,77.76043,abc", 13.02227, 77.76043, 17.0, ""); diff --git a/ge0/ge0_tests/url_generator_tests.cpp b/ge0/ge0_tests/url_generator_tests.cpp index 831727ea8f..959eeff707 100644 --- a/ge0/ge0_tests/url_generator_tests.cpp +++ b/ge0/ge0_tests/url_generator_tests.cpp @@ -342,40 +342,32 @@ UNIT_TEST(GenerateShortShowMapUrl_UnicodeMixedWithOtherChars) TEST_EQUAL("om://8wAAAAAAAA/Back%20in_\xe2\x98\x84%21\xd1\x8e\xd0\xbc", res, ()); } -UNIT_TEST(GenerateCoordUrl_SmokeTest) +UNIT_TEST(PlainCoordinateUrl_Valid_Generator) { - string res = GenerateCoordUrl(0, 0, 19, "Name"); - TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Name", res, ()); -} - -UNIT_TEST(GenerateCoordUrl_NameIsEmpty) -{ - string res = GenerateCoordUrl(0, 0, 19, ""); - TEST_EQUAL("https://omaps.app/0.00000,0.00000,19", res, ()); -} - -UNIT_TEST(GenerateCoordUrl_SpaceIsReplacedWithUnderscore) -{ - string res = GenerateCoordUrl(0, 0, 19, "Hello World"); - TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello_World", res, ()); -} - -UNIT_TEST(GenerateCoordUrl_UnderscoreIsReplacedWithSpaceEncoded) -{ - string res = GenerateCoordUrl(0, 0, 19, "Hello_World"); - TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello%20World", res, ()); -} - -UNIT_TEST(GenerateCoordUrl_ControlCharsAreEscaped) -{ - string res = GenerateCoordUrl(0, 0, 19, "Hello\tWorld\n"); - TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello%09World%0A", res, ()); -} - -UNIT_TEST(GenerateCoordUrl_FractionalZoom) -{ - string res = GenerateCoordUrl(10, 20, 8.25, "Name"); - TEST_EQUAL("https://omaps.app/10.00000,20.00000,8/Name", res, ()); + { + auto const url = GenerateCoordUrl(0, 0, 19, "Name"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Name", url, ()); + } + { + auto const url = GenerateCoordUrl(0, 0, 19, ""); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19", url, ()); + } + { + auto const url = GenerateCoordUrl(0, 0, 19, "Hello World"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello_World", url, ()); + } + { + auto const url = GenerateCoordUrl(0, 0, 19, "Hello_World"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello%20World", url, ()); + } + { + auto const url = GenerateCoordUrl(0, 0, 19, "Hello\tWorld\n"); + TEST_EQUAL("https://omaps.app/0.00000,0.00000,19/Hello%09World%0A", url, ()); + } + { + auto const url = GenerateCoordUrl(10, 20, 8.25, "Name"); + TEST_EQUAL("https://omaps.app/10.00000,20.00000,8/Name", url, ()); + } } UNIT_TEST(GenerateGeoUri_SmokeTest) diff --git a/ge0/url_generator.cpp b/ge0/url_generator.cpp index df05977c74..dc518e7dcc 100644 --- a/ge0/url_generator.cpp +++ b/ge0/url_generator.cpp @@ -105,17 +105,17 @@ std::string GenerateShortShowMapUrl(double lat, double lon, double zoom, std::st std::string GenerateCoordUrl(double lat, double lon, double zoom, std::string const & name) { - static const std::string kHttpSharePrefix = "https://omaps.app/"; - std::string url = kHttpSharePrefix; + std::string url = "https://omaps.app/"; char buf[64]; - snprintf(buf, sizeof(buf), "%.5f,%.5f,%.0f", lat, lon, zoom); + auto const writtenChars = snprintf(buf, sizeof(buf), "%.5f,%.5f,%.0f", lat, lon, zoom); + CHECK(writtenChars > 0 && writtenChars < static_cast(sizeof(buf)), ("Buffer is too small", writtenChars, lat, lon, zoom)); url.append(buf); if (!name.empty()) { - url.push_back('/'); - url.append(UrlEncodeString(TransformName(name))); + url.push_back('/'); + url.append(UrlEncodeString(TransformName(name))); } return url; -- 2.45.3