From 56a9739e04171c3e6e2f11b06619c03ab961463f Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Mon, 6 Aug 2018 14:30:59 -0700 Subject: [PATCH 1/8] Add new codearea_test --- cpp/BUILD | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cpp/BUILD b/cpp/BUILD index d31b6b52..9bff03e8 100644 --- a/cpp/BUILD +++ b/cpp/BUILD @@ -44,6 +44,21 @@ cc_test( ], ) +cc_test( + name = "codearea_test", + size = "small", + srcs = ["codearea_test.cc"], + copts = [ + "-pthread", + "-Iexternal/gtest/include", + ], + linkopts = ["-pthread"], + deps = [ + ":codearea", + "@gtest//:main", + ], +) + cc_binary( name = "openlocationcode_example", srcs = [ From b5883774d36d33a1128e903d99a3d79d6981c404 Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Mon, 6 Aug 2018 14:33:45 -0700 Subject: [PATCH 2/8] Add new CodeArea methods. --- cpp/codearea.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cpp/codearea.h b/cpp/codearea.h index 795e3ca9..e081cd7b 100644 --- a/cpp/codearea.h +++ b/cpp/codearea.h @@ -20,6 +20,10 @@ class CodeArea { double GetLongitudeHi() const; size_t GetCodeLength() const; LatLng GetCenter() const; + // Returns whether or not this area was a valid decode result. + bool IsValid() const; + // Returns an invalid CodeArea object. + static const CodeArea& InvalidCodeArea(); private: double latitude_lo_; From da64929c3cf4ec6329b0a19ae6bfa1a8a4c24ff9 Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Mon, 6 Aug 2018 14:35:11 -0700 Subject: [PATCH 3/8] Implement new CodeArea methods. --- cpp/codearea.cc | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/cpp/codearea.cc b/cpp/codearea.cc index 0ae88b20..529b4d2b 100644 --- a/cpp/codearea.cc +++ b/cpp/codearea.cc @@ -4,8 +4,11 @@ namespace openlocationcode { +namespace { const double kLatitudeMaxDegrees = 90; const double kLongitudeMaxDegrees = 180; +const CodeArea kInvalidCodeArea(0.0, 0.0, 0.0, 0.0, 0); +} // anonymous namespace CodeArea::CodeArea(double latitude_lo, double longitude_lo, double latitude_hi, double longitude_hi, size_t code_length) { @@ -44,4 +47,15 @@ LatLng CodeArea::GetCenter() const { return center; } -} // namespace openlocationcode +bool CodeArea::IsValid() const { + return code_length_ > 0 && + latitude_lo_ < latitude_hi_ && longitude_lo_ < longitude_hi_ && + latitude_lo_ >= -90.0 && latitude_hi_ <= 90.0 && + longitude_lo_ >= -180.0 && longitude_hi_ <= 180.0; +} + +const CodeArea& CodeArea::InvalidCodeArea() { + return kInvalidCodeArea; +} + +} // namespace openlocationcode From 531dc1b5ec2a454408862ca6e434113b98682f7b Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Mon, 6 Aug 2018 14:36:59 -0700 Subject: [PATCH 4/8] Implement new CodeArea unit tests. --- cpp/codearea_test.cc | 59 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 cpp/codearea_test.cc diff --git a/cpp/codearea_test.cc b/cpp/codearea_test.cc new file mode 100644 index 00000000..cc0b8290 --- /dev/null +++ b/cpp/codearea_test.cc @@ -0,0 +1,59 @@ +#include "codearea.h" +#include "gtest/gtest.h" + +namespace openlocationcode { +namespace { + +TEST(CodeAreaChecks, Accessors) { + const CodeArea area(1.0, 2.0, 3.0, 4.0, 6); + // Check accessor methods return what we expect. + EXPECT_EQ(area.GetLatitudeLo(), 1.0); + EXPECT_EQ(area.GetLongitudeLo(), 2.0); + EXPECT_EQ(area.GetLatitudeHi(), 3.0); + EXPECT_EQ(area.GetLongitudeHi(), 4.0); + EXPECT_EQ(area.GetCodeLength(), 6); +} + +TEST(CodeAreaChecks, GetCenter) { + // Simple case. + const CodeArea area1(0.0, 0.0, 1.0, 2.0, 8); + EXPECT_EQ(area1.GetCenter().latitude, 0.5); + EXPECT_EQ(area1.GetCenter().longitude, 1.0); + // Negative latitudes & longitudes. + const CodeArea area2(-10.0, -30.0, -24.0, -84.0, 4); + EXPECT_EQ(area2.GetCenter().latitude, -17.0); + EXPECT_EQ(area2.GetCenter().longitude, -57.0); + // Latitude & longitude ranges crossing zero. + const CodeArea area3(-30.0, -17.0, 5.0, 21.0, 4); + EXPECT_EQ(area3.GetCenter().latitude, -12.5); + EXPECT_EQ(area3.GetCenter().longitude, 2.0); + // Zero-sized area (not strictly valid, but center is still well-defined). + const CodeArea area4(-65.0, 117.0, -65.0, 117.0, 2); + EXPECT_EQ(area4.GetCenter().latitude, -65.0); + EXPECT_EQ(area4.GetCenter().longitude, 117.0); +} + +TEST(CodeAreaChecks, IsValid) { + // All zeroes: invalid. + EXPECT_FALSE(CodeArea(0.0, 0.0, 0.0, 0.0, 0).IsValid()); + // Whole-world area: valid. + EXPECT_TRUE(CodeArea(-90.0, -180.0, 90.0, 180.0, 1).IsValid()); + // Typical area: valid. + EXPECT_TRUE(CodeArea(-1.0, -1.0, 1.0, 1.0, 10).IsValid()); + // Zero code length: invalid. + EXPECT_FALSE(CodeArea(-1.0, -1.0, 1.0, 1.0, 0).IsValid()); + // Low latitude >= high latitude: invalid. + EXPECT_FALSE(CodeArea(1.0, -1.0, 1.0, 1.0, 10).IsValid()); + EXPECT_FALSE(CodeArea(2.0, -1.0, 1.0, 1.0, 10).IsValid()); + // Low longitude >= high longitude: invalid. + EXPECT_FALSE(CodeArea(-1.0, 1.0, 1.0, 1.0, 10).IsValid()); + EXPECT_FALSE(CodeArea(-1.0, 2.0, 1.0, 1.0, 10).IsValid()); +} + +TEST(CodeAreaChecks, InvalidCodeArea) { + // CodeArea::InvalideCodeArea() must return an invalid code area, obviously. + EXPECT_FALSE(CodeArea::InvalidCodeArea().IsValid()); +} + +} // namespace +} // namespace openlocationcode From 1514918586e8aa7cf7d005a17bf20ce611084a6a Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Mon, 6 Aug 2018 14:43:41 -0700 Subject: [PATCH 5/8] Add minimal validity checking to Decode(). --- cpp/openlocationcode.cc | 64 +++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/cpp/openlocationcode.cc b/cpp/openlocationcode.cc index 8a81cacc..43686fb8 100644 --- a/cpp/openlocationcode.cc +++ b/cpp/openlocationcode.cc @@ -209,31 +209,28 @@ CodeArea Decode(const std::string &code) { clean_code.find(internal::kPaddingCharacter)); } double resolution_degrees = internal::kEncodingBase; - double latitude = 0.0; - double longitude = 0.0; - double latitude_high = 0.0; - double longitude_high = 0.0; // Up to the first 10 characters are encoded in pairs. Subsequent characters // represent grid squares. + double coords[2] = {0.0, 0.0}; + double highs[2]; for (size_t i = 0; i < std::min(clean_code.size(), internal::kPairCodeLength); - i += 2, resolution_degrees /= internal::kEncodingBase) { - // The character at i represents latitude. Retrieve it and convert to - // degrees (positive range). - double value = get_alphabet_position(clean_code[i]); - value *= resolution_degrees; - latitude += value; - latitude_high = latitude + resolution_degrees; - // Checks if there are no more characters. - if (i == std::min(clean_code.size(), internal::kPairCodeLength)) { - break; + ++i) { + // Retrieve a digit, convert it to degrees and add it to a coordinate. + const int digit = get_alphabet_position(clean_code[i]); + if (digit < 0) { + return CodeArea::InvalidCodeArea(); + } + const size_t j = i & 1; + coords[j] += static_cast(digit) * resolution_degrees; + highs[j] = coords[j] + resolution_degrees; + if (j != 0) { + resolution_degrees /= internal::kEncodingBase; } - // The character at i + 1 represents longitude. Retrieve it and convert to - // degrees (positive range). - value = get_alphabet_position(clean_code[i + 1]); - value *= resolution_degrees; - longitude += value; - longitude_high = longitude + resolution_degrees; } + double latitude = coords[0]; + double longitude = coords[1]; + double latitude_high = highs[0]; + double longitude_high = highs[1]; if (clean_code.size() > internal::kPairCodeLength) { // Now do any grid square characters. // Adjust the resolution back a step because we need the resolution of the @@ -246,9 +243,12 @@ CodeArea Decode(const std::string &code) { for (size_t i = internal::kPairCodeLength; i < std::min(internal::kMaximumDigitCount, clean_code.size()); i++) { // Get the value of the character at i and convert it to the degree value. - size_t value = get_alphabet_position(clean_code[i]); - size_t row = value / internal::kGridColumns; - size_t col = value % internal::kGridColumns; + const int digit = get_alphabet_position(clean_code[i]); + if (digit < 0) { + return CodeArea::InvalidCodeArea(); + } + const size_t row = digit / internal::kGridColumns; + const size_t col = digit % internal::kGridColumns; // Lat and lng grid sizes shouldn't underflow due to maximum code length // enforcement, but a hypothetical underflow won't cause fatal errors // here. @@ -256,9 +256,9 @@ CodeArea Decode(const std::string &code) { longitude_resolution /= internal::kGridColumns; latitude += row * latitude_resolution; longitude += col * longitude_resolution; - latitude_high = latitude + latitude_resolution; - longitude_high = longitude + longitude_resolution; } + latitude_high = latitude + latitude_resolution; + longitude_high = longitude + longitude_resolution; } return CodeArea(latitude - internal::kLatitudeMaxDegrees, longitude - internal::kLongitudeMaxDegrees, @@ -328,13 +328,15 @@ std::string RecoverNearest(const std::string &short_code, // within -90 to 90 degrees. double center_lat = code_rect.GetCenter().latitude; double center_lng = code_rect.GetCenter().longitude; - if (latitude + half_res < center_lat && center_lat - resolution > -internal::kLatitudeMaxDegrees) { - // If the proposed code is more than half a cell north of the reference location, - // it's too far, and the best match will be one cell south. + if (latitude + half_res < center_lat && + center_lat - resolution > -internal::kLatitudeMaxDegrees) { + // If the proposed code is more than half a cell north of the reference + // location, it's too far, and the best match will be one cell south. center_lat -= resolution; - } else if (latitude - half_res > center_lat && center_lat + resolution < internal::kLatitudeMaxDegrees) { - // If the proposed code is more than half a cell south of the reference location, - // it's too far, and the best match will be one cell north. + } else if (latitude - half_res > center_lat && + center_lat + resolution < internal::kLatitudeMaxDegrees) { + // If the proposed code is more than half a cell south of the reference + // location, it's too far, and the best match will be one cell north. center_lat += resolution; } // How many degrees longitude is the code from the reference? From 811358d719241ff30cc807b8672cc0b5e0322ece Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Mon, 6 Aug 2018 14:49:13 -0700 Subject: [PATCH 6/8] Add DecodingChecks:DecodeOutOfRange subtest. --- cpp/openlocationcode_test.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cpp/openlocationcode_test.cc b/cpp/openlocationcode_test.cc index 5582342e..680a2a52 100644 --- a/cpp/openlocationcode_test.cc +++ b/cpp/openlocationcode_test.cc @@ -122,6 +122,7 @@ TEST_P(EncodingChecks, Encode) { EXPECT_EQ(test_data.code, actual_code); // Now decode the code and check we get the correct coordinates. CodeArea actual_rect = Decode(test_data.code); + EXPECT_TRUE(actual_rect.IsValid()); EXPECT_NEAR(expected_rect.GetCenter().latitude, actual_rect.GetCenter().latitude, 1e-10); EXPECT_NEAR(expected_rect.GetCenter().longitude, @@ -131,6 +132,15 @@ TEST_P(EncodingChecks, Encode) { INSTANTIATE_TEST_CASE_P(OLC_Tests, EncodingChecks, ::testing::ValuesIn(GetEncodingDataFromCsv())); +TEST(DecodingChecks, DecodeOutOfRange) { + // Leading longitude digit "W" is out of range. + EXPECT_FALSE(Decode(std::string("9W4Q0000+")).IsValid()); + // Leading latitude digit "F" is out of range. + EXPECT_FALSE(Decode(std::string("F2+")).IsValid()); + // Invalid digits in post-separator portion. + EXPECT_FALSE(Decode(std::string("7Q7Q7Q7Q+5Z")).IsValid()); +} + struct ValidityTestData { std::string code; bool is_valid; From c043ee5f2f79cb008b73b58f37971941df9fd90d Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Thu, 8 Sep 2022 15:05:23 -0700 Subject: [PATCH 7/8] Fix invalid code check after PR conflict resolution. --- cpp/openlocationcode.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cpp/openlocationcode.cc b/cpp/openlocationcode.cc index 098accc3..607ab6a5 100644 --- a/cpp/openlocationcode.cc +++ b/cpp/openlocationcode.cc @@ -208,8 +208,13 @@ CodeArea Decode(const std::string &code) { // Define the place value for the most significant pair. int pv = pow(internal::kEncodingBase, internal::kPairCodeLength / 2 - 1); for (size_t i = 0; i < digits - 1; i += 2) { - normal_lat += get_alphabet_position(clean_code[i]) * pv; - normal_lng += get_alphabet_position(clean_code[i + 1]) * pv; + const int lat_dval = get_alphabet_position(clean_code[i]); + const int lng_dval = get_alphabet_position(clean_code[i + 1]); + if (lat_dval < 0 || lng_dval < 0) { + return CodeArea::InvalidCodeArea(); + } + normal_lat += lat_dval * pv; + normal_lng += lng_dval * pv; if (i < digits - 2) { pv /= internal::kEncodingBase; } From 63dcc27c06e0da06475b5fb0ac7570fd7f668c0f Mon Sep 17 00:00:00 2001 From: Viet-Tam Luu Date: Thu, 8 Sep 2022 15:15:51 -0700 Subject: [PATCH 8/8] Apply clang-format to codearea.cc to pass clang-format check. --- cpp/codearea.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cpp/codearea.cc b/cpp/codearea.cc index 8edced71..f40320b4 100644 --- a/cpp/codearea.cc +++ b/cpp/codearea.cc @@ -40,14 +40,12 @@ LatLng CodeArea::GetCenter() const { } bool CodeArea::IsValid() const { - return code_length_ > 0 && - latitude_lo_ < latitude_hi_ && longitude_lo_ < longitude_hi_ && - latitude_lo_ >= -90.0 && latitude_hi_ <= 90.0 && - longitude_lo_ >= -180.0 && longitude_hi_ <= 180.0; + return code_length_ > 0 && latitude_lo_ < latitude_hi_ && + longitude_lo_ < longitude_hi_ && latitude_lo_ >= -90.0 && + latitude_hi_ <= 90.0 && longitude_lo_ >= -180.0 && + longitude_hi_ <= 180.0; } -const CodeArea& CodeArea::InvalidCodeArea() { - return kInvalidCodeArea; -} +const CodeArea& CodeArea::InvalidCodeArea() { return kInvalidCodeArea; } } // namespace openlocationcode