From aec67de447f6151dbaffb58528fcb69256e0c230 Mon Sep 17 00:00:00 2001 From: Will Beason Date: Sat, 7 Mar 2026 10:05:54 -0600 Subject: [PATCH 1/2] Add validity tests to cover edge cases This improves Go coverage for optimizations I want to be sure are safe. --- go/olc.go | 3 --- go/olc_test.go | 30 ++++++++++++++++++++++++++++++ test_data/encoding.csv | 1 + test_data/validityTests.csv | 12 ++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/go/olc.go b/go/olc.go index cda9ca96..c3321e34 100644 --- a/go/olc.go +++ b/go/olc.go @@ -167,9 +167,6 @@ func CheckFull(code string) error { if firstLat := strings.IndexByte(Alphabet, upper(code[0])) * int(encBase); firstLat >= latMax*2 { return errors.New("latitude outside range") } - if len(code) == 1 { - return nil - } if firstLong := strings.IndexByte(Alphabet, upper(code[1])) * int(encBase); firstLong >= lngMax*2 { return errors.New("longitude outside range") } diff --git a/go/olc_test.go b/go/olc_test.go index 407fb8a0..19241b05 100644 --- a/go/olc_test.go +++ b/go/olc_test.go @@ -131,6 +131,36 @@ func TestCheck(t *testing.T) { } } +func TestCheckShort(t *testing.T) { + for i, elt := range validity { + err := CheckShort(elt.code) + got := err == nil + if got != elt.isShort { + t.Errorf("%d. %q validity is %t (err=%v), wanted %t.", i, elt.code, got, err, elt.isShort) + } + } +} + +func TestCheckFull(t *testing.T) { + for i, elt := range validity { + err := CheckFull(elt.code) + got := err == nil + if got != elt.isFull { + t.Errorf("%d. %q validity is %t (err=%v), wanted %t.", i, elt.code, got, err, elt.isFull) + } + } +} + +func TestDecodeValidity(t *testing.T) { + for i, elt := range validity { + _, err := Decode(elt.code) + got := err == nil + if got != elt.isFull { + t.Errorf("%d. %q validity is %t (err=%v), wanted %t.", i, elt.code, got, err, elt.isValid) + } + } +} + func TestEncodeDegrees(t *testing.T) { const allowedErrRate float64 = 0.05 var badCodes int diff --git a/test_data/encoding.csv b/test_data/encoding.csv index 534299cf..4cf28c56 100644 --- a/test_data/encoding.csv +++ b/test_data/encoding.csv @@ -24,6 +24,7 @@ 20.5,2.5,2762500000,1495040000,4,7FG40000+ -89.9999375,-179.9999375,1562,512,10,22222222+22 0.5,179.5,2262500000,2945024000,4,6VGX0000+ +0.5,179.5,2262500000,2945024000,3,6VGX0000+ 1,1,2275000000,1482752000,11,6FH32222+222 ################################################################################ # diff --git a/test_data/validityTests.csv b/test_data/validityTests.csv index 984be50f..a7fd8042 100644 --- a/test_data/validityTests.csv +++ b/test_data/validityTests.csv @@ -14,6 +14,8 @@ 8fwc2345+,true,false,true 8FWCX400+,true,false,true 84000000+,true,false,true +C2000000+,true,false,true +2V000000+,true,false,true ################################################################################ # # Valid short codes: @@ -29,7 +31,9 @@ WC2345+G6g,true,true,false # ################################################################################ G+,false,false,false +G,false,false,false +,false,false,false +45CC+0+,false,false,false 8FWC2345+G,false,false,false 8FWC2_45+G6,false,false,false 8FWC2η45+G6,false,false,false @@ -40,6 +44,14 @@ WC2300+G6g,false,false,false WC2345+G,false,false,false WC2300+,false,false,false 84900000+,false,false,false +# Cannot start with padding +049VGJQF+VX7Q,false,false,false +# Missing separator +849VGJQFVX7Q,false,false,false +# Latitude outside range +F2000000+,true,false,false +# Longitude outside range +2W000000+,true,false,false ################################################################################ # # Validate that codes at and exceeding 15 digits are still valid when all their From 2cab34af8b14465ed50162cf62e5adc5aa9ead76 Mon Sep 17 00:00:00 2001 From: Will Beason Date: Sat, 7 Mar 2026 10:05:54 -0600 Subject: [PATCH 2/2] Add validity tests to cover edge cases I added these tests to improve Go coverage for optimizations I want to be sure are safe. In adding these, I found two cases where the libraries of various language handle validation logic inconsistently. In cases of inconsistency, I defaulted to the behavior used by most language in the repository. 1. Automatically expanding requested short code length if odd. Most of the language implementations automatically add 1 to the length of requested codes if the length indicates it is in the pair part of the code. 2. Validate latitude and longitude bounds within validation logic for full codes. Most of the language implementations only check the first two characters for latitude/longitude bounds when within the validation logic for full codes, not when generally checking if codes are valid. This does introduce a strange case where a code is "valid", but is neither a valid short nor a valid long code. As this was the preexisting behavior, I have defaulted to this. --- cpp/openlocationcode.cc | 14 +++++----- .../openlocationcode/OpenLocationCode.java | 28 +++++++++---------- js/closure/openlocationcode.js | 9 +++--- python/openlocationcode/openlocationcode.py | 6 ++-- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/cpp/openlocationcode.cc b/cpp/openlocationcode.cc index d128e423..4318605a 100644 --- a/cpp/openlocationcode.cc +++ b/cpp/openlocationcode.cc @@ -78,6 +78,10 @@ std::string encodeIntegers(int64_t lat_val, int64_t lng_val, // Add the separator character. code[internal::kSeparatorPosition] = internal::kSeparator; + // 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); // Compute the grid part of the code if necessary. if (code_length > internal::kPairCodeLength) { for (size_t i = internal::kGridCodeLength; i >= 1; i--) { @@ -89,6 +93,9 @@ std::string encodeIntegers(int64_t lat_val, int64_t lng_val, lng_val /= internal::kGridColumns; } } else { + if (code_length % 2 == 1) { + code_length++; + } lat_val /= pow(internal::kGridRows, internal::kGridCodeLength); lng_val /= pow(internal::kGridColumns, internal::kGridCodeLength); } @@ -193,13 +200,6 @@ std::string clean_code_chars(const std::string &code) { } // anonymous namespace 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; - } // Convert latitude and longitude into integer values. int64_t lat_val = internal::latitudeToInteger(location.latitude); int64_t lng_val = internal::longitudeToInteger(location.longitude); diff --git a/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java b/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java index 37b36636..de07d487 100644 --- a/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java +++ b/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java @@ -205,8 +205,12 @@ public OpenLocationCode(double latitude, double longitude, int codeLength) { static String encodeIntegers(long lat, long lng, int codeLength) { // Limit the maximum number of digits in the code. codeLength = Math.min(codeLength, MAX_DIGIT_COUNT); + // Automatically increase code length if odd and below pair code length. + if ((codeLength < PAIR_CODE_LENGTH) && codeLength % 2 == 1) { + codeLength += 1; + } // Check that the code length requested is valid. - if (codeLength < PAIR_CODE_LENGTH && codeLength % 2 == 1 || codeLength < MIN_DIGIT_COUNT) { + if (codeLength < MIN_DIGIT_COUNT) { throw new IllegalArgumentException("Illegal code length " + codeLength); } @@ -359,6 +363,15 @@ public static CodeArea decode(String code) throws IllegalArgumentException { * @return True if it is a full code. */ public boolean isFull() { + // First latitude character can only have first 9 values. + if (CODE_ALPHABET.indexOf(code.charAt(0)) > 8) { + return false; + } + + // First longitude character can only have first 18 values. + if (CODE_ALPHABET.indexOf(code.charAt(1)) > 17) { + return false; + } return code.indexOf(SEPARATOR) == SEPARATOR_POSITION; } @@ -569,19 +582,6 @@ public static boolean isValidCode(String code) { return false; } - // Check first two characters: only some values from the alphabet are permitted. - if (separatorPosition == SEPARATOR_POSITION) { - // First latitude character can only have first 9 values. - if (CODE_ALPHABET.indexOf(code.charAt(0)) > 8) { - return false; - } - - // First longitude character can only have first 18 values. - if (CODE_ALPHABET.indexOf(code.charAt(1)) > 17) { - return false; - } - } - // Check the characters before the separator. boolean paddingStarted = false; for (int i = 0; i < separatorPosition; i++) { diff --git a/js/closure/openlocationcode.js b/js/closure/openlocationcode.js index eea7bcc6..d7943210 100644 --- a/js/closure/openlocationcode.js +++ b/js/closure/openlocationcode.js @@ -420,10 +420,11 @@ function _encodeIntegers(latInt, lngInt, codeLength) { if (typeof codeLength == 'undefined') { codeLength = PAIR_CODE_LENGTH; } - if ( - codeLength < MIN_CODE_LEN || - (codeLength < PAIR_CODE_LENGTH && codeLength % 2 == 1) - ) { + // Increment length if short and not even. + if (codeLength < PAIR_CODE_LENGTH && codeLength % 2 === 1) { + codeLength++; + } + if (codeLength < MIN_CODE_LEN) { throw new Error('IllegalArgumentException: Invalid Plus Code length'); } codeLength = Math.min(codeLength, MAX_CODE_LEN); diff --git a/python/openlocationcode/openlocationcode.py b/python/openlocationcode/openlocationcode.py index b29a9d70..efdbdd9f 100644 --- a/python/openlocationcode/openlocationcode.py +++ b/python/openlocationcode/openlocationcode.py @@ -287,8 +287,10 @@ def encodeIntegers(latVal, lngVal, codeLength): This function is exposed for testing purposes and should not be called directly. """ - if codeLength < MIN_DIGIT_COUNT_ or (codeLength < PAIR_CODE_LENGTH_ and - codeLength % 2 == 1): + if codeLength < PAIR_CODE_LENGTH_ and codeLength % 2 == 1: + # Automatically increment short lengths to be even. + codeLength+=1 + if codeLength < MIN_DIGIT_COUNT_: raise ValueError('Invalid Open Location Code length - ' + str(codeLength)) codeLength = min(codeLength, MAX_DIGIT_COUNT_)