Fix implementations for short/odd code lengths (#668)

* Add test cases for short, padded code lengths * update Java for short, odd code lengths * add validity tests for 2 and 3 length codes * add decoding tests for codes with length 2 & 3 * add invalid three digit code to tests * update java implementation for 2-digit codes * prevent illegal code lengths being generated * prevent illegal code lengths * replace number with const * replace number with var * replace number with var * replace number with var, formatting fixes * replace number with var * replace number with var, formatting fixes * add code length and odd-ness checks * fix padded code validity check * format with rustfmt
google · Dec 27, 2024 · c92c6f3 · c92c6f3
1 parent 4d83aed
commit c92c6f3
Show file tree

Hide file tree

Showing 19 changed files with 113 additions and 15 deletions.
diff --git a/c/src/olc.c b/c/src/olc.c
@@ -88,6 +88,12 @@ int OLC_Encode(const OLC_LatLon* location, size_t length, char* code,
   if (length > kMaximumDigitCount) {
     length = kMaximumDigitCount;
   }
+  if (length < kMinimumDigitCount) {
+    length = kMinimumDigitCount;
+  }
+  if (length < kPairCodeLength && length % 2 == 1) {
+    length = length + 1;
+  }
   // Adjust latitude and longitude so they fall into positive ranges.
   double latitude = adjust_latitude(location->lat, length);
   double longitude = normalize_longitude(location->lon);

diff --git a/c/src/olc_private.h b/c/src/olc_private.h
@@ -21,6 +21,8 @@ static const char kPaddingCharacter = '0';
 static const char kAlphabet[] = "23456789CFGHJMPQRVWX";
 // Number of digits in the alphabet.
 static const size_t kEncodingBase = OLC_kEncodingBase;
+// The min number of digits returned in a plus code.
+static const size_t kMinimumDigitCount = 2;
 // The max number of digits returned in a plus code. Roughly 1 x 0.5 cm.
 static const size_t kMaximumDigitCount = 15;
 // The number of code characters that are lat/lng pairs.

diff --git a/cpp/openlocationcode.cc b/cpp/openlocationcode.cc
@@ -17,6 +17,7 @@ const char kAlphabet[] = "23456789CFGHJMPQRVWX";
 const size_t kEncodingBase = 20;
 // The max number of digits returned in a plus code. Roughly 1 x 0.5 cm.
 const size_t kMaximumDigitCount = 15;
+const size_t kMinimumDigitCount = 2;
 const size_t kPairCodeLength = 10;
 const size_t kGridCodeLength = kMaximumDigitCount - kPairCodeLength;
 const size_t kGridColumns = 4;
@@ -122,6 +123,11 @@ std::string clean_code_chars(const std::string &code) {
 std::string Encode(const LatLng &location, size_t code_length) {
   // Limit the maximum number of digits in the code.
   code_length = std::min(code_length, internal::kMaximumDigitCount);
+  // Ensure the length is valid.
+  code_length = std::max(code_length, internal::kMinimumDigitCount);
+  if (code_length < internal::kPairCodeLength && code_length % 2 == 1) {
+    code_length = code_length + 1;
+  }
   // Adjust latitude and longitude so that they are normalized/clipped.
   double latitude = adjust_latitude(location.latitude, code_length);
   double longitude = normalize_longitude(location.longitude);

diff --git a/dart/lib/src/open_location_code.dart b/dart/lib/src/open_location_code.dart
@@ -39,6 +39,9 @@ const latitudeMax = 90;
 /// The maximum value for longitude in degrees.
 const longitudeMax = 180;
 
+// The min number of digits in a plus code.
+const minDigitCount = 2;
+
 // The max number of digits to process in a plus code.
 const maxDigitCount = 15;
 
@@ -251,7 +254,7 @@ bool isFull(String code) {
 /// * [codeLength]: The number of significant digits in the output code, not
 /// including any separator characters.
 String encode(num latitude, num longitude, {int codeLength = pairCodeLength}) {
-  if (codeLength < 2 || (codeLength < pairCodeLength && codeLength.isOdd)) {
+  if (codeLength < minDigitCount || (codeLength < pairCodeLength && codeLength.isOdd)) {
     throw ArgumentError('Invalid Open Location Code length: $codeLength');
   }
   codeLength = min(maxDigitCount, codeLength);

diff --git a/go/olc.go b/go/olc.go
@@ -143,7 +143,7 @@ func Check(code string) error {
 		if firstSep < sepPos {
 			return errors.New("short codes cannot have padding")
 		}
-		if len(code)-firstPad-1%2 == 1 {
+		if firstPad%2 == 1 {
 			return errors.New("odd number of padding chars")
 		}
 	}

diff --git a/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java b/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java
@@ -57,6 +57,9 @@ public final class OpenLocationCode {
   // The number of characters to place before the separator.
   private static final int SEPARATOR_POSITION = 8;
 
+  // The minimum number of digits in a plus code.
+  public static final int MIN_DIGIT_COUNT = 2;
+
   // The max number of digits to process in a plus code.
   public static final int MAX_DIGIT_COUNT = 15;
 
@@ -188,7 +191,7 @@ public OpenLocationCode(double latitude, double longitude, int codeLength) {
     // Limit the maximum number of digits in the code.
     codeLength = Math.min(codeLength, MAX_DIGIT_COUNT);
     // Check that the code length requested is valid.
-    if (codeLength < PAIR_CODE_LENGTH && codeLength % 2 == 1 || codeLength < 4) {
+    if (codeLength < PAIR_CODE_LENGTH && codeLength % 2 == 1 || codeLength < MIN_DIGIT_COUNT) {
       throw new IllegalArgumentException("Illegal code length " + codeLength);
     }
     // Ensure that latitude and longitude are valid.

diff --git a/js/closure/openlocationcode.js b/js/closure/openlocationcode.js
@@ -101,6 +101,11 @@ var LATITUDE_MAX = 90;
  */
 var LONGITUDE_MAX = 180;
 
+/**
+ * Minimum length of a code.
+ */
+var MIN_CODE_LEN = 2;
+
 /**
  * Maximum length of a code.
  */
@@ -352,7 +357,7 @@ function encode(latitude, longitude, optLength) {
   if (typeof optLength == 'undefined') {
     optLength = PAIR_CODE_LENGTH;
   }
-  if (optLength < 2 ||
+  if (optLength < MIN_CODE_LEN ||
       (optLength < PAIR_CODE_LENGTH && optLength % 2 == 1)) {
     throw new Error(
         'IllegalArgumentException: Invalid Open Location Code length');

diff --git a/js/src/openlocationcode.js b/js/src/openlocationcode.js
@@ -108,6 +108,9 @@
   // The maximum value for longitude in degrees.
   var LONGITUDE_MAX_ = 180;
 
+  // The min number of digits in a plus code.
+  var MIN_DIGIT_COUNT_ = 2;
+
   // The max number of digits to process in a plus code.
   var MAX_DIGIT_COUNT_ = 15;
 
@@ -317,7 +320,7 @@
     if (isNaN(latitude) || isNaN(longitude) || isNaN(codeLength)) {
       throw new Error('ValueError: Parameters are not numbers');
     }
-    if (codeLength < 2 ||
+    if (codeLength < MIN_DIGIT_COUNT_ ||
         (codeLength < PAIR_CODE_LENGTH_ && codeLength % 2 == 1)) {
       throw new Error('IllegalArgumentException: Invalid Open Location Code length');
     }

diff --git a/plpgsql/pluscode_functions.sql b/plpgsql/pluscode_functions.sql
@@ -327,6 +327,7 @@ DECLARE
     ENCODING_BASE_ int := char_length(CODE_ALPHABET_);
     LATITUDE_MAX_ int := 90;
     LONGITUDE_MAX_ int := 180;
+    MIN_DIGIT_COUNT_ int := 2;
     MAX_DIGIT_COUNT_ int := 15;
     PAIR_CODE_LENGTH_ int := 10;
     PAIR_PRECISION_ decimal := power(ENCODING_BASE_, 3);
@@ -343,7 +344,7 @@ DECLARE
     ndx smallint;
     i_ smallint;
 BEGIN
-    IF ((codeLength < 2) OR ((codeLength < PAIR_CODE_LENGTH_) AND (codeLength % 2 = 1))) THEN
+    IF ((codeLength < MIN_DIGIT_COUNT_) OR ((codeLength < PAIR_CODE_LENGTH_) AND (codeLength % 2 = 1))) THEN
         RAISE EXCEPTION 'Invalid Open Location Code length - %', codeLength
         USING HINT = 'The Open Location Code length must be 2, 4, 6, 8, 10, 11, 12, 13, 14, or 15.';
     END IF;

diff --git a/python/openlocationcode/openlocationcode.py b/python/openlocationcode/openlocationcode.py
@@ -81,6 +81,9 @@
 # The maximum value for longitude in degrees.
 LONGITUDE_MAX_ = 180
 
+# The min number of digits to process in a plus code.
+MIN_DIGIT_COUNT_ = 2
+
 # The max number of digits to process in a plus code.
 MAX_DIGIT_COUNT_ = 15
 
@@ -241,8 +244,8 @@ def encode(latitude, longitude, codeLength=PAIR_CODE_LENGTH_):
       codeLength: The number of significant digits in the output code, not
           including any separator characters.
     """
-    if codeLength < 2 or (codeLength < PAIR_CODE_LENGTH_ and
-                          codeLength % 2 == 1):
+    if codeLength < MIN_DIGIT_COUNT_ or (codeLength < PAIR_CODE_LENGTH_ and
+                                         codeLength % 2 == 1):
         raise ValueError('Invalid Open Location Code length - ' +
                          str(codeLength))
     codeLength = min(codeLength, MAX_DIGIT_COUNT_)
@@ -371,7 +374,6 @@ def decode(code):
                     min(len(code), MAX_DIGIT_COUNT_))
 
 
-
 def recoverNearest(code, referenceLatitude, referenceLongitude):
     """
      Recover the nearest matching code to a specified location.
@@ -528,6 +530,7 @@ class CodeArea(object):
        code_length: The number of significant characters that were in the code.
            This excludes the separator.
     """
+
     def __init__(self, latitudeLo, longitudeLo, latitudeHi, longitudeHi,
                  codeLength):
         self.latitudeLo = latitudeLo

diff --git a/ruby/lib/plus_codes.rb b/ruby/lib/plus_codes.rb
@@ -16,6 +16,9 @@ module PlusCodes
   # The max number of characters can be placed before the separator.
   SEPARATOR_POSITION = 8
 
+  # Minimum number of digits to process in a plus code.
+  MIN_CODE_LENGTH = 2
+
   # Maximum number of digits to process in a plus code.
   MAX_CODE_LENGTH = 15
 

diff --git a/ruby/lib/plus_codes/open_location_code.rb b/ruby/lib/plus_codes/open_location_code.rb
@@ -270,7 +270,8 @@ def valid_character?(code)
     end
 
     def invalid_length?(code_length)
-      code_length < 2 || (code_length < PAIR_CODE_LENGTH && code_length.odd?)
+      code_length < MIN_CODE_LENGTH ||
+        (code_length < PAIR_CODE_LENGTH && code_length.odd?)
     end
 
     def padded(code)

diff --git a/rust/src/consts.rs b/rust/src/consts.rs
@@ -23,6 +23,9 @@ pub const LATITUDE_MAX: f64 = 90f64;
 // The maximum value for longitude in degrees.
 pub const LONGITUDE_MAX: f64 = 180f64;
 
+// Minimum number of digits to process for plus codes.
+pub const MIN_CODE_LENGTH: usize = 2;
+
 // Maximum number of digits to process for plus codes.
 pub const MAX_CODE_LENGTH: usize = 15;
 

diff --git a/rust/src/interface.rs b/rust/src/interface.rs
@@ -6,8 +6,8 @@ use codearea::CodeArea;
 use consts::{
     CODE_ALPHABET, ENCODING_BASE, GRID_CODE_LENGTH, GRID_COLUMNS, GRID_ROWS, LATITUDE_MAX,
     LAT_INTEGER_MULTIPLIER, LNG_INTEGER_MULTIPLIER, LONGITUDE_MAX, MAX_CODE_LENGTH,
-    MIN_TRIMMABLE_CODE_LEN, PADDING_CHAR, PADDING_CHAR_STR, PAIR_CODE_LENGTH, PAIR_RESOLUTIONS,
-    SEPARATOR, SEPARATOR_POSITION,
+    MIN_CODE_LENGTH, MIN_TRIMMABLE_CODE_LEN, PADDING_CHAR, PADDING_CHAR_STR, PAIR_CODE_LENGTH,
+    PAIR_RESOLUTIONS, SEPARATOR, SEPARATOR_POSITION,
 };
 
 use private::{
@@ -106,7 +106,7 @@ pub fn encode(pt: Point<f64>, code_length: usize) -> String {
     let mut lat = clip_latitude(pt.lat());
     let lng = normalize_longitude(pt.lng());
 
-    let trimmed_code_length = cmp::min(code_length, MAX_CODE_LENGTH);
+    let trimmed_code_length = cmp::min(cmp::max(code_length, MIN_CODE_LENGTH), MAX_CODE_LENGTH);
 
     // Latitude 90 needs to be adjusted to be just less, so the returned code
     // can also be decoded.

diff --git a/rust/tests/all_test.rs b/rust/tests/all_test.rs
@@ -22,7 +22,6 @@ use csv_reader::CSVReader;
 ///
 /// We could probably take it a little further, and assert that tested was >= # tests in the file
 /// (allowing tests to be added, but assuming # tests will never be reduced).
-
 #[test]
 fn is_valid_test() {
     let mut tested = 0;

diff --git a/test_data/decoding.csv b/test_data/decoding.csv
@@ -1,9 +1,13 @@
+################################################################################
+#
 # Test decoding Open Location Codes.
 #
 # Provides test cases for decoding valid codes.
 #
 # Format:
 #   code,length,latLo,lngLo,latHi,lngHi
+#
+################################################################################
 7FG49Q00+,6,20.35,2.75,20.4,2.8
 7FG49QCJ+2V,10,20.37,2.782125,20.370125,2.78225
 7FG49QCJ+2VX,11,20.3701,2.78221875,20.370125,2.78225
@@ -20,8 +24,17 @@ CFX30000+,4,89,1,90,2
 62H20000+,4,1,-180,2,-179
 62H30000+,4,1,-179,2,-178
 CFX3X2X2+X2,10,89.9998750,1,90,1.0001250
+84000000+,2,30,-140,50,-120
+################################################################################
+#
 # Test non-precise latitude/longitude value
+#
+################################################################################
 6FH56C22+22,10,1.2000000000000028,3.4000000000000057,1.2001249999999999,3.4001250000000027
+################################################################################
+#
 # Validate that digits after the first 15 are ignored when decoding
+#
+################################################################################
 849VGJQF+VX7QR3J,15,37.5396691200,-122.3750698242,37.5396691600,-122.3750697021
 849VGJQF+VX7QR3J7QR3J,15,37.5396691200,-122.3750698242,37.5396691600,-122.3750697021
diff --git a/test_data/encoding.csv b/test_data/encoding.csv
@@ -1,9 +1,13 @@
+################################################################################
+#
 # Test encoding Open Location Codes.
 #
 # Provides test cases for encoding latitude and longitude to codes.
 #
 # Format:
 #   latitude,longitude,length,expected code (empty if the input should cause an error)
+#
+################################################################################
 20.375,2.775,6,7FG49Q00+
 20.3700625,2.7821875,10,7FG49QCJ+2V
 20.3701125,2.782234375,11,7FG49QCJ+2VX
@@ -123,18 +127,28 @@
 -18.32,96.397,10,5MHRM9JW+2R
 -30.3,76.5,11,4JXRPG22+222
 50.342,-112.534,15,95298FR8+RC22222
+################################################################################
 #
 # There is no exact IEEE754 representation of 80.01 (or the negative), so test
 # on either side.
 #
+################################################################################
 80.0100000001,58.57,15,CHGW2H6C+2222222
 80.0099999999,58.57,15,CHGW2H5C+X2RRRRR
 -80.0099999999,58.57,15,2HFWXHRC+2222222
 -80.0100000001,58.57,15,2HFWXHQC+X2RRRRR
+################################################################################
 #
 # Add a few other examples.
 #
+################################################################################
 47.000000080000000,8.00022229,15,8FVC2222+235235C
 68.3500147997595,113.625636875353,15,9PWM9J2G+272FWJV
 38.1176000887231,165.441989844555,15,8VC74C9R+2QX445C
 -28.1217794010122,-154.066811473758,15,5337VWHM+77PR2GR
+################################################################################
+#
+# Test short length.
+#
+################################################################################
+37.539669125,-122.375069724,2,84000000+
diff --git a/test_data/validityTests.csv b/test_data/validityTests.csv
@@ -1,17 +1,33 @@
+################################################################################
+#
 # Test data for validity tests.
 # Format of each line is:
 #   code,isValid,isShort,isFull
+#
+################################################################################
+#
 # Valid full codes:
+#
+################################################################################
 8FWC2345+G6,true,false,true
 8FWC2345+G6G,true,false,true
 8fwc2345+,true,false,true
 8FWCX400+,true,false,true
+84000000+,true,false,true
+################################################################################
+#
 # Valid short codes:
+#
+################################################################################
 WC2345+G6g,true,true,false
 2345+G6,true,true,false
 45+G6,true,true,false
 +G6,true,true,false
+################################################################################
+#
 # Invalid codes
+#
+################################################################################
 G+,false,false,false
 +,false,false,false
 8FWC2345+G,false,false,false
@@ -23,8 +39,13 @@ G+,false,false,false
 WC2300+G6g,false,false,false
 WC2345+G,false,false,false
 WC2300+,false,false,false
+84900000+,false,false,false
+################################################################################
+#
 # Validate that codes at and exceeding 15 digits are still valid when all their
 # digits are valid, and invalid when not.
+#
+################################################################################
 849VGJQF+VX7QR3J,true,false,true
 849VGJQF+VX7QR3U,false,false,false
 849VGJQF+VX7QR3JW,true,false,true

diff --git a/visualbasic/OpenLocationCode.bas b/visualbasic/OpenLocationCode.bas
@@ -90,6 +90,12 @@ Private Const LATITUDE_MAX_ As Double = 90
 ' The maximum value for longitude in degrees.
 Private Const LONGITUDE_MAX_ As Double = 180
 
+' Minimum number of digits in a code.
+Private Const MIN_DIGIT_COUNT_ = 2;
+
+' Maximum number of digits in a code.
+Private Const MAX_DIGIT_COUNT_ = 15;
+
 ' Maximum code length using lat/lng pair encoding. The area of such a
 ' code is approximately 13x13 meters (at the equator), and should be suitable
 ' for identifying buildings. This excludes prefix and separator characters.
@@ -212,7 +218,13 @@ Public Function OLCEncode(ByVal latitude As Double, ByVal longitude As Double, O
   If codeLength = 0 Then
     codeLength = CODE_PRECISION_NORMAL
   End If
-  If codeLength < 2 Then
+  If codeLength < MIN_DIGIT_COUNT_ Then
+    Err.raise vbObjectError + 513, "OLCEncodeWithLength", "Invalid code length"
+  End If
+  If codeLength > MAX_DIGIT_COUNT_ Then
+    Err.raise vbObjectError + 513, "OLCEncodeWithLength", "Invalid code length"
+  End If
+  If codeLength < PAIR_CODE_LENGTH_ And codeLength \ 2 = 1 Then
     Err.raise vbObjectError + 513, "OLCEncodeWithLength", "Invalid code length"
   End If
   Dim lat, lng As Double